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

Non-naturally aligned access to the trap vector table #290

Closed Silabs-ArjanB closed 1 year ago

Silabs-ArjanB commented 1 year ago

When performing a hardware vectored interrupt entry the access to the trap vector table will always be a naturally aligned access as the trap vector table access is defined with:

pc := M[TBASE + XLEN/8 * exccode)] & ~1

TBASE = xtvt[XLEN-1:6]<<6 # Trap vector table base is at least 64-byte aligned.

However, when performing an xret instruction while xcause.inhv is set, the trap vector table access is not guaranteed to be a naturally aligned access as it is defined as:

pc := M[xepc]

Of course if xepc has not been touched by software, its content would have remained naturally aligned, but we cannot make that assumption when implementing this hardware.

Could the xret instruction therefore be defined as follows instead?

if (xcause.inhv) //xcause here refers to the cause CSR of the previous privilege mode pc := M[xepc & ~(XLEN/8 - 1)] //xepc here refers to the current privilege mode else pc := xepc

kasanovic commented 1 year ago

The xret should cause a trap if not naturally aligned. In some systems, requiring that the low bits are ignored in this case would require additional special-case logic. Also, trapping will help catch bugs, so I think the current behavior is the correct choice.

Issue https://github.com/openhwgroup/cv32e40x/issues/704 is a hardware design issue in a particular core implementation - the same issue shows up whenever an instruction prefetcher steps into data sections. In this case, the prefetcher could be told not to prefetch around a table lookup.

kasanovic commented 1 year ago

Discussed in TG meeting 1/17/2023. Closing.

Silabs-ArjanB commented 1 year ago

Hello @kasanovic

Thank you for discussing this issue in the TG meeting. The following is however still not fully clear:

The xret should cause a trap if not naturally aligned.

Is it allowed that 'naturally aligned' means 32 bit aligned if an xret is executed with xcause.inhv == 1 even though a hart supports the C extension?

Should the exception code be 0 (instruction address misaligned) when executing an xret with xcause.inhv while mepc is not word aligned?

kasanovic commented 1 year ago

"Naturally aligned" depends on the possible instruction sizes. If compressed instructions are supported, then only the low bit must be zero. If compressed instructions are not supported, then both low bits must be zero. Yes, instruction address misaligned exception should be reported.

Silabs-ArjanB commented 1 year ago

Hello @kasanovic ,

I don't think this proposal is appropriate for harts that support compressed instructions.

Let me try to explain the issue better:

So clearly the actual content of the trap vector table is at least word aligned (actually it will be XLEN/8 aligned) (also if a hart supports Compressed instructions).

For ease of discussion let's now assume that XLEN=32 and the hart supports Compressed instructions.

If the hart perform a xret with xcause.inhv == 1, then I think it would be very weird if we require the alignment trap to be based on the 'natural alignment' implied by the Compressed instructions (as suggested in above answer). The actual trap vector table content in this case is still word aligned (and not halfword aligned), and therefore I think that we should let xret with xcause.inhv == 1 cause a trap if the initial fetch address has its lower two bits as 2'b10 (even if the hart also supports Compressed instructions) (the final target address of course follows the regular natural alignment rules). If we would not do this then, upon executing a xret with xcause.inhv == 1 the hart would have to construct a 32-bit pointer from two word aligned table entries (which it can do of course, but the result would never be a valid pointer).

So in short I think we should do the following:

kasanovic commented 1 year ago

You're correct. In this case with xcause.inhv set, there should only be an alignment exception raised if the xepc was not naturally XLEN/8-byte aligned as the table entries must be XLEN/8 aligned.

kasanovic commented 1 year ago

We'll create a pull to define the misalignment trap in this way.

tovine commented 1 year ago

Is this requirement only applicable if xcause.inhv is set, or always? EDIT: sorry, just got a bit confused first..

To summarize and see if I got this right:

kasanovic commented 1 year ago

Yes, that's right.

kasanovic commented 1 year ago

In TG meeting, we decided it was cleaner and simpler for hardware to simply force the table alignment by clearing the low bits before access the table. This removes the need to check for trap and matches other treatment of xepc values.