RISCV32 {__irq_wrapper} exception handling error under compressed instruction mode?

[zephyrbot]

Reported by Quanwen Du:

When RISCV32 is excuting compressed instruction which is 2bytes per instruction, and trigger an exception(not an ecall exception), the following code seems NOT to consider such cases in {arch\riscv32\core\isr.S}

lw t0, __NANO_ESF_mepc_OFFSET(sp)
addi t0, t0, 4
sw t0, __NANO_ESF_mepc_OFFSET(sp)

The code {addi t0, t0, 4} does NOT take compressed instruction into consideration. if the exception instruction is compressed instruction, shall be {addi t0, t0, 2}

(Imported from Jira ZEP-2393)

[zephyrbot]

by Jean-Paul Etienne:

Are you using the riscv toolchain coming from the zephyr-sdk or are you using an external riscv toolchain?
To handle compressed instruction, I'll have to add a extra config flag, like CONFIG_RISCV_RVC_INSTR, that will be disabled by default. I'll update the isr.S with the following:
#ifdef CONFIG_RISCV_RVC_INSTR
addi t0, t0, 2
#else
addi t0, t0, 4
#endif
For users that want to compile zephyr with compressed instructions enabled, they will have to enable it in their config. I prefer that this to be optional as we might have some RISCV SOCs that do not support compressed instructions.

when CONFIG_RISCV_RVC_INSTR is set, I can also automatically update the compilation flags SOCs supporting compressed instructions.

[zephyrbot]

by Quanwen Du:

Your solution may not work, because we have cases to mix RISC-V 32bits&16bits instructions into one image; actually RISCV does support such cases, right? So I think one extensible way is to check the instruction(which triggers exception) itself to know it is 16bits or 32bits, even 64/128bits. For example, if the 2 LSB bits of the instruction are NOT 11, means it is 16bits instruction, then it is safe to add 2. I am thinking this approach is more flexible, how do you think of this? You may refer to RISCV ISA definition to confirm this approach, but I assume it works.

[nashif]

@nategraff-sifive can you please check if this bug is still relevant?

[nategraff-sifive]

Will do 👍

[nategraff-sifive]

Okay, so the problem here is that the MEPC increment is located in the wrong part of the interrupt handling logic. The only time we want to jump over the instruction which triggered the trap is in the case of the ecall instruction, which in the case of Zephyr is used to trigger the scheduling event. Since the ecall instruction is always 4 bytes, even with compressed instructions, it's still proper to increment at a fixed 4 bytes.

I'll submit a pull request to relocate the MEPC increment to only happen when the exception is the result of an ecall, at which point this issue can be resolved.