[hw,dma] DMA fixes after merge to master

[Razer6]

This PR adds two fixes needed after merging the DMA to master:

1. It changes the SHA mode None to SHA_None, which was changed in the related upstream package
2. It removes the devmode interface, which was removed from the upstream register tooling.

[Razer6]

Smoke is still failing with:

    * Test dma_generic_smoke has 1 failures.
        * 0.dma_generic_smoke.41808117927340199003857090625009877405905829949318719279971918770493320013570\
          Line 108, in log /work/rschilling/opentitan/scratch/dma-fixes/dma-sim-xcelium/0.dma_generic_smoke/latest/run.log

                UVM_FATAL @  130003034ps: (dv_base_reg.sv:354) [dma_reg_block.range_regwen] lock register invalid access RW
                UVM_INFO @  130003034ps: (uvm_report_catcher.svh:705) [UVM/REPORT/CATCHER]
                --- UVM Report catcher Summary ---
                

[Razer6]

The issue is most probably because #20446 is missing on master.

[rswarbrick]

This looks good to me!

[Razer6]

Unfortunately, the last commit didn't change anything. @alees24 Could you help me here?

[Razer6]

@rswarbrick That is the DMA I was talking about regarding the regwen.

[rswarbrick]

Looking at the line that's failing, I think this comes from an assertion in dv_base_reg. See lock_lockable_flds at dv_base_reg.sv:330.

It seems to be showing the REGWEN field as being RW. But that's confusing to me: it looks like it's:

    { name: "RANGE_REGWEN"
      desc: "Used to lock the DMA enabled memory range configuration registers."
      swaccess: "rw0c"
      hwaccess: "hro"
...

Maybe worth adding some tracing to figure out how the regwen field is considered RW. Maybe we're not propagating the access permissions correctly from a register to the fields inside it?

[andreaskurth]

RTL change LGTM

[johngt]

@martin-velay - @andreaskurth has suggested that you take a quick look at this too.

[martin-velay]

The DV & RTL seems OK. For the FPV in tpl format I have no idea.

And I think you should also update opentitan/hw/ip/dma/dv/doc/tb.svg to remove the references to devmode

[rswarbrick]

Hi @Razer6! This looks good to me but I realise that we don't have CI coverage for this block yet. Are you happy for the change to be merged now or would you prefer to tweak it first?

[Razer6]

@rswarbrick I'm happy to merge that though smoke still fails with the error from above.

[rswarbrick]

I've just taken a bit of a look and the problem is being caused by the fact that the regwen field of range_regwen appears in the auto-generated dma_ral_pkg.sv with access set to "RW". The code in dv_base_reg.sv always expects regwen fields to be "RO" or "W0C" (this is the test that's failing).

I was a bit surprised to see that this worked in the integrated-dev branch. If you do a bit of tracing through reggen, you find that the access flags actually come from access.py and this is different between the two branches. The difference comes from commit 0b90b90.

I'll do a bit more digging, but this should hopefully explain why there is a mismatch.

[rswarbrick]

Ahah! The logic in the DV code is from this:

  // This function is used for wen_reg to lock its lockable flds by changing the lockable flds'
  // access policy. For register write via csr_wr(), this function is included in post_write().
  // For register write via tl_access(), user will need to call this function manually.
  virtual function void lock_lockable_flds(uvm_reg_data_t val, uvm_predict_e kind);
    if (is_wen_reg()) begin
      `uvm_info(`gfn, $sformatf("lock_lockable_flds %d val", val), UVM_LOW);
      foreach (m_fields[i]) begin
        dv_base_reg_field fld;
        `downcast(fld, m_fields[i])
        if (fld.is_wen_fld()) begin
          uvm_reg_data_t field_val = val & fld.get_field_mask();
          string field_access = fld.get_access();
          case (field_access)
            // discussed in issue #1922: enable register is standarized to W0C or RO (if HW has
            // write access).
            "W0C": begin
              // This is the regular behavior with W0C access policy enabled (i.e., only
              // clearing is possible).
              if (kind == UVM_PREDICT_WRITE && field_val == 1'b0) begin
                fld.set_lockable_flds_access(1);
              // In this case we are using direct prediction where the access policy is not
              // applied. I.e., a regwen bit that has been set to 0 can be set to 1 again.
              end else if (kind == UVM_PREDICT_DIRECT) begin
                fld.set_lockable_flds_access((~field_val) & fld.get_field_mask());
              end
            end
            "RO": ; // if RO, it's updated by design, need to predict in scb
            default:`uvm_fatal(`gfn, $sformatf("lock register invalid access %s", field_access))
          endcase
        end
      end
    end
  endfunction

This isn't particularly easy to read, but I think the point is that every field in a regwen is expected to either be W0C or RO`. The important thing is that neither of these values allows software to directly re-enable writes.

Looking at the Python code, the dv_rights function is supposed to return the access rights for a field in a form that UVM will understand. On integrated_dev, this is basically the name of the enum value except that r0w1c is mapped to "w1c" (presumably UVM doesn't know about r0w1c as an access pattern).

On master, the commit I pointed to was reasonable. The point is that a mubi value can't be considered (e.g.) R0C because UVM will be thinking about clearing each bit, rather than the logical meaning of the mubi.

I think the correct fix is to sort out the dv_base_reg logic and I'll start looking at that now.

[rswarbrick]

Well, that was harder than I expected! I think the change in #24912 should be what we need.

[Razer6]

I added a commit on top to update the TB figure. I removed the reference to devmode, removed the duplicated reference to tl_agent_dma_ctn_cfg and added a reference to tl_agent_dma_sys_cfg

[vogelpi]

I've reviewed and approved @rswarbrick 's fix. We should probably merge that one first and then this PR here.

[Razer6]

Sounds good. I see the fix got merged. Let's merge this once CI is happy to unblock the other DMA PRs.

[rswarbrick]

We've got past the only changes that might be detectable by CI (all that remains is FPGA runs, which this can't break). Looks good!