[rv_dm,lint] Waive remaining Verilator lint warnings

[rswarbrick]

The first commit is #21736: merge that first, but review the second commit here.

For the second commit, the message is:

This should waive the final lint warnings that come from Verilator
when building rv_dm. To see the current status, run:

  util/dvsim/dvsim.py hw/top_earlgrey/lint/top_earlgrey_lint_cfgs.hjson \
    --tool verilator --select-cfgs=rv_dm

Before this change, you got the following warnings:

%Warning-WIDTHTRUNC: ../src/pulp-platform_riscv-dbg_0.1_0/pulp_riscv_dbg/src/dm_mem.sv:87:41: Operator VAR 'RomBaseAddr' expects 12 bits on the Initial value, but Initial value's VARREF 'HaltAddress' generates 64 bits.
                                                                                            : ... note: In instance 'rv_dm.u_dm_top.i_dm_mem'
   87 |   localparam logic [DbgAddressBits-1:0] RomBaseAddr   = dm::HaltAddress;
      |                                         ^~~~~~~~~~~
                     ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=5.020
                     ... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message.
%Warning-WIDTHTRUNC: ../src/pulp-platform_riscv-dbg_0.1_0/pulp_riscv_dbg/src/dm_mem.sv:90:41: Operator VAR 'RomEndAddr' expects 12 bits on the Initial value, but Initial value's ADD generates 64 bits.
                                                                                            : ... note: In instance 'rv_dm.u_dm_top.i_dm_mem'
   90 |   localparam logic [DbgAddressBits-1:0] RomEndAddr    = dm::HaltAddress + 'h7FF;
      |                                         ^~~~~~~~~~
%Warning-WIDTHEXPAND: ../src/pulp-platform_riscv-dbg_0.1_0/pulp_riscv_dbg/src/dm_mem.sv:92:59: Operator DIV expects 32 bits on the LHS, but LHS's VARREF 'ProgBufSize' generates 5 bits.
                                                                                             : ... note: In instance 'rv_dm.u_dm_top.i_dm_mem'
   92 |   localparam int unsigned ProgBuf64Size = dm::ProgBufSize / 2;

• The first two warnings come from the same problem: HaltAddress is
  declared in dm_pkg.sv to be 64-bit variables. In dm_mem.sv,
  DbgAddressBits is a localparam equal to 12.

• The third warning comes from the fact that we want to divide (with
  floor) and do it with "/2" instead of ">> 1". Verilator sees this as
  a binary operator where the second argument (2) is interpreted as a
  32-bit value.

[rswarbrick]

Force-push is to use "WIDTH" instead of "WIDTHTRUNC" and "WIDTHEXPAND" in the waivers. I think that these might only work for new enough versions of Verilator (which we don't depend on in our CI runners)

[andreaskurth]

We discussed this here, right?

This only holds as long as dm::HaltAddress in dm_pkg.sv doesn't get changed. It feels wrong to add this waiver, which is decoupled from dm_pkg.sv. Wouldn't it be better to fix the RTL, so that only the 12 LSB of dm::HaltAddress are used to compute RomBaseAddr and RomEndAddr? We could do that with a patch while we create an upstream PR and wait for it to get approved (or not, in which case we keep the patch).

Why waive this instead of patching the RTL?

[rswarbrick]

The honest answer: because the RTL is vendored! Would it make sense to waive this locally but also raise an issue in the pulp project (and we can then include the pulp issue ID in the waiver line)?

[andreaskurth]

I think this lint warning is not critical, so I don't put a veto on adding this waiver. I'd prefer getting the RTL fixed, though, but probably we have more important things to do right now.

So please feel free to go ahead with adding this wavier, but then I suggest creating a D3 issue to fix this in RTL and remove this waiver.

[rswarbrick]

Issue created: #22545.

[andreaskurth]

Automatic zero extension always leads to the correct result in this case, as far as I see. --> LGTM

[andreaskurth]

thx for getting rv_dm lint clean