openhwgroup / cva6

The CORE-V CVA6 is an Application class 6-stage RISC-V CPU capable of booting Linux
https://docs.openhwgroup.org/projects/cva6-user-manual/
Other
2.23k stars 675 forks source link

Fix the 65x CSR document #2275

Closed JeanRochCoulon closed 2 months ago

JeanRochCoulon commented 3 months ago

Several feedbacks from 65x CSR document https://docs.openhwgroup.org/projects/cva6-user-manual/04_cv32a65x/design/source/CSRs.html

zchamski commented 3 months ago

Taking the points in order:

  1. exception information need to be added: when exception will be generated ?

    • This requires a careful analysis of the RV ISA spec (upstream and annotated). Since riscv-configdoes not support per-CSR exception information, the requested "exception information" will most likely take the form of an additional paragraph in the introduction of the CSR design doc (@zchamski)
  2. Upper PMP CSR are ROCST zero

    • @JeanRochCoulon; I guess this applies to PMPADDRn registers with n = 8-15? The definitions of these registers are ROCST zero in the riscv-config spec but they are later folded into a generic definition in the CSR factorizer. I will have a look at the code and ask @AbdessamiiOukalrazqou for help if needed.
  3. PMPCfg have several bits stuck at zero

    • will be updated in the riscv-config input spec (@zchamski) and handled in the CSR factorizer (@zchamski with support from @AbdessamiiOukalrazqou). In the rv-config input spec there are two tasks:
      • the bits stuck at zero in PMPCONFIG0/PMPCONFIG1 need to be made explicit by masking
      • the definitions of PMPCFG2 and PMPCFG3 need to be made ROCST zero.
  4. MCONFIGPTR does not exist

    • @JeanRochCoulon: Please clarify. This read-only CSR is present in the annotated spec for CV32A65X, in the riscv-config input spec, in the rendered CSR design doc and in the RTL.
  5. replace MHPMCOUNTER[]H by MHPMCOUNTERH[]

    • This needs further discussion as the H suffix is placed after the number. Maybe the interval notation ("firsteg - lastreg") and properly typeset names such as MHPMCOUNTERnH will be a better solution (but RST is limited in this respect.)
  6. in CSR list, replace []_ by []

    • @AbdessamiiOukalrazqou, can you take a look at this? I confirm that hyperlinks do not function for indexed register names.
  7. In legal value, replace "0 - 1" by "0x0 - 0x1"

    • this is a value rendering problem, will be fixed by @zchamski
JeanRochCoulon commented 3 months ago

I agree about MCONFIGPTR, this CSR exists. And I am ok to keep MHPMCOUNTERH as it is Today.

JeanRochCoulon commented 2 months ago

This issue has been fixed in CSR specification, thanks @AbdessamiiOukalrazqou