search

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
245 stars 49 forks source link

smclicshv psuedo-code assumes hart has IALIGN=16 and needs to be better documented #411

Closed james-ball-qualcomm closed 1 month ago

james-ball-qualcomm commented 1 month ago

In "smclicshv Changes to xtvec CSR Mode for CLIC" section, there is pseudo-code showing the least-significant bit of the vector table entry is forced to zero when the hart fetches the entry from the table (the "&~1" in the pseudo-code). image

I have two related issues related to this:

  1. I presume this should show "&~1" in implementations that have IALIGN=16 and "&~3" in implementations that have IALIGN=32.
  2. We should document outside the pseudo-code that the 1 or 2 least-significant bits of the table entry are forced to zero (depending on IALIGN). We should document those bits as WPRI which means that they are reserved and software must write them with zero.
jb-brelot-nxp commented 1 month ago

Hi James,

I propose to add a note that an implementation "can decide to align PC" accordingly to IALIGN, but I would not recommend any forcing.

From my side if SW is adding a unaligned address in the Vector Table it is a SW error, as it is branching to an unaligned address. And this will then be covered by unprivileged spec chapter 1.5

Masking bottom bit from my side could lead to execute some instructions badly executed without seeing the initial issue..

james-ball-qualcomm commented 1 month ago

The existing pseudo-code already shows forcing the LSB to 0 (with the implicit assumption of IALIGN=16). If you don't force the 1 LSB (IALIGN=16) or 2 LSBs (IALIGN=32) to zero then what happens if a programmer puts non-zero values there? Presumably it would cause a misaligned instruction address exception since it is a hardware state machine and not a jump instruction.

So, I recommend that the pseudo-code and the text both make it clear that implementations must ignore the bottom 1 or 2 LSBs (depending on IALIGN).

james-ball-qualcomm commented 1 month ago

Maybe we can talk about this at the next Fast Interrupt TG meeting since I'm not sure I fully understand your comments and perspective?

james-ball-qualcomm commented 1 month ago

Can we close this now that the related PR has been accepted and merged into the main branch?

jb-brelot-nxp commented 1 month ago

PR done and merged. can be close