Add DCSR and DPC CSRs

[neverlandiz]

This PR addresses Issue #570 and adds the remaining missing debug CSRs

• DCSR
• DPC

[neverlandiz]

For core debug registers like DCSR and DPC, it’s stated that “these registers are only accessible from Debug Mode” (Section 4.9, debug specs). What would be the correct priv_mode for those CSRs? Debug mode isn’t one of the options in the schema (only M, S, U, VS).

[ThinkOpenly]

For core debug registers like DCSR and DPC, it’s stated that “these registers are only accessible from Debug Mode” (Section 4.9, debug specs). What would be the correct priv_mode for those CSRs? Debug mode isn’t one of the options in the schema (only M, S, U, VS).

Hmm. The Priv spec 1.3 "Debug Mode" says:

Debug mode (D-mode) can be considered an additional privilege mode, with even more access than M-mode.

That sounds like a full Mode (capital M). It probably needs to be treated as such, adding a new mode in schemas/csr_schema.json, and an entry in arch/isa/globals.isa, but there don't appear to be any CSR bits to identify "D-mode" (I couldn't find any). So, which new enum value to choose is an open question. Maybe @dhower-qc has an idea?

Some additional information: arch/isa/globals.isa has:

# encoded as defined in the privilege spec
enum PrivilegeMode {
  M  0b011
  S  0b001
  HS 0b001 # alias for S when H extension is used
  U  0b000
  VS 0b101
  VU 0b100
}

These 3 bits appear to be the concatenation of MPV and MPP, or at least they line up that way in the Priv spec, 18.4.1 "Machine Status Registers (mstatus and mstatush)", table 32:

It may be that you could just pick a random new enum value (prepend a 4th bit, like 0b1000?), but I don't know for sure. (You could certainly try!)

[dhower-qc]

Yea, we do need to add D-mode in globals.isa, and yes, there are cases where we are relying on the encoding of PrivilegeMode being the table Paul added above. For example:

riscv-unified-db/arch/isa/globals.isa

Lines 628 to 634 in 00f1540

	$pc = {CSR[stvec].BASE, 2'b00};
	CSR[scause].INT = 1'b0;
	CSR[scause].CODE = $bits(code);
	CSR[hstatus].GVA = 1;
	CSR[hstatus].SPV = 1; # guest page faults always come from a virtual mode
	CSR[hstatus].SPVP = $bits(from_mode)[0];
	CSR[mstatus].SPP = $bits(from_mode)[0];

As such, we should encode D as 0b1011 since it is M (0b0011) with extra permission.

[ThinkOpenly]

One of the regression tests is failing:

/home/runner/work/riscv-unified-db/riscv-unified-db/lib/arch_obj_models/csr.rb:215:in `length': Unexpected length field for dpc (RuntimeError)

Might need a hint from @dhower-qc here, but adding DXLEN support might also include:

1. Adding a value for it in cfgs/example_rv64_with_overlay.yaml
2. Updating arch/csr/schema.adoc
3. Updating arch/README.adoc
4. Maybe updating lib/arch_obj_models/csr.rb (This is the spot which might be causing the failure.)

[dhower-qc]

@ThinkOpenly

The regressions are failing because of the various places we've assumed that MXLEN is the fundamental XLEN of the machine. We either need to fix that, or...

I wonder if DXLEN can ever be > than MXLEN? The spec states that DXLEN is:

Debug XLEN, which is the widest XLEN a hart supports, ignoring the current value of mxl in misa.

That seems to be accounting for the fact that misa.MXL used to be mutable. But the spec was amended to make that impossible.

As such, I have trouble imagining any use for a machine where MXLEN == 32 and DXLEN == 64. And it's not worth implementing a DXLEN > MXLEN for the old spec where misa.MXL could change, since the assumption is there are zero implementations out there that do it (it was never well-enough defined anyway).

My vote is to drop DXLEN, and get clarification from the debug spec that DXLEN is always identical to MXLEN.

[ThinkOpenly]

My vote is to drop DXLEN, and get clarification from the debug spec that DXLEN is always identical to MXLEN.

Per Paul Donahue (Ventana, author of the Debug spec):

I believe that DXLEN is always the same as MXLEN. I thought that we fixed the definition when mxl became read-only but it looks like that was overlooked.

[neverlandiz]

Thanks for the feedback! I'll remove the DXLEN code and use MXLEN for dpc instead.

[dhower-qc]

Per Paul Donahue (Ventana, author of the Debug spec):

I believe that DXLEN is always the same as MXLEN. I thought that we fixed the definition when mxl became read-only but it looks like that was overlooked.

Excellent. That's going save us a few headaches.

[ThinkOpenly]

@neverlandiz, if you are willing to make a few more changes:

• ratification date in arch/ext/Sdext.yaml could be set to 2025-02
• all of STEPIE, STOPCOUNT, and STOPTIME need new configuration parameters (in arch/ext/Sdext.yaml) because they can be hardwared to 0, 1, or read-write, just like MPRVEN.
• they also need their own type() and reset_value attributes.

If you are sufficiently tired of this PR and want to see it merged and move on with your life, we can merge this and open an issue, or we can take on our own commits here.

What's your preference? (No worries either way, honestly.)

[neverlandiz]

@ThinkOpenly I'd be happy to make the changes! I plan on finishing the PRs that I have opened.

[ThinkOpenly]

So, now that #797 has been merged, we need a major rebase:

• arch/ext/Sdext.yaml -> spec/std/isa/ext/Sdext.yaml
• arch/csr/* -> spec/std/isa/csr/
• arch/isa/globals/isa -> spec/std/isa/isa/globals.isa (that's a lot of "isa"!)
• schemas/csr_schema.json -> spec/schemas/csr_schema.json
• lib/cfg_arch.rb -> tools/ruby-gems/udb/lib/udb/cfg_arch.rb
• lib/arch_obj_models/csr.rb -> tools/ruby-gems/udb/lib/udb/obj/csr.rb

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[ThinkOpenly]

@dhower-qc can we merge this? We got a request to work on #215, but this PR covers a lot of that work.