riscv / riscv-fast-interrupt

Proposal for a RISC-V Core-Local Interrupt Controller (CLIC)
https://jira.riscv.org/browse/RVG-63
Creative Commons Attribution 4.0 International
247 stars 49 forks source link

Smclic section contains a lot of changes relevant only to S-mode #409

Closed christian-herber-nxp closed 1 month ago

christian-herber-nxp commented 2 months ago

The CLIC specification has two main extensions, Smclic and Ssclic. However, the Smclic specification mixes m-mode only and general statements. Examples:

The latter one even mixes references to x-CSRs and m-CSRs This is an attempt to reduce the amount of copy paste in the specification, which is good. The privileged specification goes full copy paste between supervisor and machine mode CSRs.

I think there is no good solution, but at least things need to be reviewed and corrected where needed. The real solution would be to make self-contained Smclic and Ssclic chapters.

jb-brelot-nxp commented 1 month ago

Need to check with Tech Writer if changing x to m is the RISC-V strategy .IN some others spec we are seeing the same practise.

jb-brelot-nxp commented 1 month ago

@james-ball-qualcomm : please check with tech writers.

james-ball-qualcomm commented 1 month ago

From email thread with tech writers:

I did a little research on this by looking at a recent copy of the priv ISA manual. I looked at the Smstateen/Ssstateen extensions, the Smcsrind/Sscsrind extensions, and the Hypervisor. None of them use syntax like x and instead explicitly say m and s (or vs/hs for Hypervisor). Note that I’m just using the less-than and greater-than signs to separate out the privilege mode prefix from the CSR name, they aren’t in the CSR names.

I only see the “x” prefix being used for CSR fields such as xIE and xPIE in the mstatus register.

So, my research suggests that the CLIC spec is an outlier and so we should remove the x in the smclic chapter and use m instead. I can make this change if approved.

christian-herber-nxp commented 1 month ago

Agree that this is the way to go. Appreciate if you can make the change.

james-ball-qualcomm commented 1 month ago

I've been working on the changes. I have a question. The CLIC registers accessed via the indirect CSR mechanism don't really seem to belong to a particular mode (M or S). Instead, it seems like they exist outside the scope of any particular mode but are accessed via different indirect CSR addresses for M-mode or S-mode. The only difference is that M-mode access sees all the information in the indirect access CSRs where as S-mode "any interrupt i that is not accessible to S-mode appears as hard-wired zeros in clicintip[i], clicintie[i], clicintattr[i], and clicintctl[i]."

Maybe we should move the definition of these indirect access CSRs back into their own chapter and then just have smclic and ssclic chapters talk about how to access them from their respective modes and how S-mode sees a subset of the info (the quoted text in the previous chapter)? If we agree to this, we should open a separate issue and do a separate PR.

james-ball-qualcomm commented 1 month ago

The software appendices have example code that assumes M-mode but then the text descriptions use the 'x' prefix CSR names. I'm changing them to use 'm' prefix CSR names but we should decide if we want that or not. If not, we should update the example code to use 'x' names too otherwise it is confusing.

james-ball-qualcomm commented 1 month ago

The smclicshv extension naming (i.e. sm*) suggests it only has M-mode features. Does it have S-mode features too? I ask because the smclicshv chapter uses the 'x' prefix notation instead of 'm'. What gives?

james-ball-qualcomm commented 1 month ago

I've made the fixes we identified in the clic weekly meeting. And I've created separate issues for issues I listed earlier here.