riscv / riscv-j-extension

Working Draft of the RISC-V J Extension Specification
https://jira.riscv.org/browse/RVG-128
Creative Commons Attribution 4.0 International
158 stars 19 forks source link

ISA Extensions section clarifications #56

Closed JamesKenneyImperas closed 6 months ago

JamesKenneyImperas commented 7 months ago

Can you please clarify some of the information in ISA Extensions section?

  1. There is a statement All of these fields are read-only 0 on RV32 systems. I don't think the term RV32 system is precise enough. What is the behavior on a core with variable XLEN?

  2. Normally the visibility and behavior of fields in henvcfg is governed by the same fields in menvcfg. Is that also the case here? If so, can that be stated explicitly? See the Sstc extension specification for an example of how this is described. If it isn't the case, it would help to have that explicitly stated too.

Thanks.

(Edited to remove one question which was a misunderstanding on my part.)

pdonahue-ventana commented 7 months ago

There is a statement All of these fields are read-only 0 on RV32 systems. I don't think the term RV32 system is precise enough. What is the behavior on a core with variable XLEN?

Also, it should be hart rather than system. You can have heterogeneous systems.

martinmaas commented 7 months ago

The term "RV32 system" follows the way this term is used in the relevant parts of the privileged RISC-V spec, describing the physical underlying register width before any XLEN reconfiguration. E.g., Section 3.1.6.2 of the privileged spec:

For RV32 systems, the SXL and UXL fields do not exist, and SXLEN=32 and UXLEN=32.

The pointer masking standard (Section 2.7) describes the behavior in the presence of variable XLEN – if XLEN is set to 32 via UXL/SXL/MXL, the behavior is the same as on RV32.

I realize that "system" can have a range of meanings and the current definition should not preclude (e.g.,) SoCs that have different types of RISC-V processors in them. However, I think we should stay consistent with the definitions in the base ISA.

JamesKenneyImperas commented 7 months ago

Thanks Martin, I think that makes sense.

I think a couple of minor additional clarifications might help. The Pointer Masking Extensions section says:

Note that in RV32, the CSR bits introduced by pointer masking are still present, for compatibility between RV32 and larger systems with UXL/SXL/MXL set to 1.

  1. Does this mean that the PMM fields are visible with read-only potentially non-zero values in the relevant xenvcfgh registers in RV32 mode?
  2. Unless it's been recently changed, there isn't an senvcfgh register, so the statement above isn't true for the senvcfg.PMM field in the RV64 senvcfg CSR. Also, that CSR is shared between HS and VS modes, so the field will be cleared to zero on a transition from virtual mode with VSXLEN=32 to non-virtual mode with HSXLEN=64 (at least I think so - see issue https://github.com/riscv/riscv-isa-manual/issues/1173). This isn't necessarily a problem (the Hypervisor specification says that Hypervisor software is expected to manually swap the contents of these registers as needed) but I thought I'd point that out in case there is an expectation that it would be preserved in this scenario.

Thanks.

martinmaas commented 6 months ago

I added clarifications to the corresponding sections stating that setting UXL/SXL/MXL to 1 clears the corresponding pointer masking bits (commit 8088461). I believe this should address both of these issues (if you have additional concerns, please re-open this issue).