arch: riscv: handle interrupt level for CLIC

[jimmyzhe]

RISC-V CLIC supports mintstatus.MIL (RO) and mcause.MPIL (RW) for interrupt levels.
When an interrupt happends, mintstatus.MIL (current interrupt level) is set to mcause.MPIL (previous interrupt level). Each ISR must execute MRET to restore mcause.MPIL back to mintstatus.MIL.

To handle nested interrupts with CLIC interrupt levels, save and restore mcause.MPIL in ISRs. Use RISCV_ALWAYS_SWITCH_THROUGH_ECALL to ensure ISRs do not exit without executing MRET.

[tovine]

When an interrupt happends, mintstatus.MIL (current interrupt level) is set to mcause.MPIL (previous interrupt level). Each ISR must execute MRET to restore mcause.MPIL back to mintstatus.MIL.

Let me see if my understanding is correct; this is the sequence you mean?

When an interrupt happens:
mcause.mpil = mintstatus.mil
And on mret:
mintstatus.mil = mcause.mpil

[tovine]

What's the reason for all this masking instead of just restoring the saved mcause value?

[jimmyzhe]

Yes, that's what I mean.

According to CLIC doc, when CLIC mode is enabled, mcause has new fields mcause.mpp and mcause.mpie that mirror mstatus.mpp and mstatus.mpie.
Restoring the entire mcause here pollutes the previous mstatus settings.

[tovine]

But mstatus is also restored later (line -689/+707), so it doesn't really matter if you "pollute" it?

Besides, if you end up in a situation where the thread context of mcause and mstatus are inconsistent then I'd say you have bigger problems, as the thread state would essentially be corrupt

[jimmyzhe]

Yes, mcause.mpp and mcause.mpie fields are always overwritten by the later storing to mstatus. The mirror fields in mstatus and mcause always remain the same in the stack frame because they are restored together.

However, the RISC-V PMP stack guard and userspace mechanism are based on mstatus.mpp. I am still figuring out if it is safe when mcause.mpp is restored before access to the stack (line -704/+705).

	lr t0, __struct_arch_esf_mepc_OFFSET(sp)
	lr t2, __struct_arch_esf_mstatus_OFFSET(sp)

[tovine]

However, the RISC-V PMP stack guard and userspace mechanism are based on mstatus.mpp

Are you sure this is correct? Sounds a bit strange that they would depends on the previous privilege mode (mpp) instead of the current one.
I don't think mstatus.mpp would take any effect until after executing mret to get back to whatever mode you were in...

[jimmyzhe]

Sorry, I misunderstood the userspace. Only the PMP stack guard is based on mstatus.mpp.
According to The RISC-V Instruction Set Manual: Volume II 3.1.6.3. Memory Privilege in mstatus Register, we can use mstatus.mprv and mstatus.mpp to perform load/store operations with MMU translation or PMP protection in M-mode.
The PMP stack guard has used this to detect thread and interrupt stack overflow since PR #44651.

[jimmyzhe]

I updated the restoring for mcause. It now saves and restores the entire mcause and avoids storing to memory between restoring mcause and mstatus.

[tovine]

Aah, I hadn't seen this one before - sorry about that, you were right about MPP being used in some cases. 🙂

When MPRV=1, load and store memory
addresses are translated and protected, and endianness is applied, as though the current privilege
mode were set to MPP.

I think your updated code looks much better though 👍

[masz-nordic]

This change breaks our internal CI with VPR cores.
Blocking until we have time to investigate.

[jimmyzhe]

I found that earlier Andes CLIC (N22 core) also fail because MRET only updates mintstatus when mcause.interrupt=1.
However, for the recent Andes CLIC (N225, D23 core), MRET always update mintstatus regardless of mcause.interrupt, which matches the latest CLIC spec (5.7.6 and 5.9.7).

When an interrupt causes a reschedule, an ISR may exit (MRET) with mcause.interrupt=0, and the earlier Andes CLIC doesn't update mintstatus, leading to masked interrupts after MRET.

I am not sure whether other CLIC implementations have similar condition.
@soburi fyi.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[jimmyzhe]

Rebased and updated for gd32vf103.

After I tested on longan_nano board, I found that the Nuclei ECLIC has the same behavior as the earlier Andes CLIC. Both require mcause.interrupt=1 to restore mintstatus when executing MRET.

In my opinion, this seems to be a SoC-specific behavior because this is not defined in the RISC-V CLIC spec, and the latest Andes CLIC doesn't have this limitation.

This limitation can be handled by SOC-specific context (CONFIG_RISCV_SOC_CONTEXT_SAVE) to set the mcause.interrupt to ensure the interrupt level is restored correctly after MRET.

I tested these changes on longan_nano with the following test cases:

• tests/kernel/common/kernel.common
• tests/kernel/interrupt/arch.interrupt
• tests/kernel/gen_isr_table/arch.interrupt.gen_isr_table.riscv_direct
• tests/kernel/gen_isr_table/arch.interrupt.gen_isr_table.riscv_no_direct

[jimmyzhe]

@masz-nordic, is there any updated information about the VPR cores?

[masz-nordic]

Unfortunately no. Perhaps a way to unblock you would be to make those changes depend on !defined(CONFIG_RISCV_CORE_NORDIC_VPR).

[ycsin]

Perhaps a way to unblock you would be to make those changes depend on !defined(CONFIG_RISCV_CORE_NORDIC_VPR).

Or maybe have something like CLIC_SUPPORT_NESTED_INTERRUPT, and select that accordingly? If it is in the specs, then maybe select that in RISCV_HAS_CLIC if !NRFX_CLIC

[marnold-b]

Or maybe have something like CLIC_SUPPORT_NESTED_INTERRUPT, and select that accordingly?

+1 for this solution, that makes it possible to disable it in a downstream soc as well.

[jimmyzhe]

@ycsin, thank you for your suggestion, the updated commit introduces CLIC_SUPPORT_INTERRUPT_PREEMPTION.

I used "interrupt preemption" because the VPR core may support nested ISR with IRQ offload (e.g. test_nested_irq_offload in tests/kernel/common), and I think 'interrupt preemption' is more appropriate for interrupt level handling , as described in the CLIC spec.

@masz-nordic, the VPR core is filtered by CLIC_SUPPORT_INTERRUPT_PREEMPTION. If this causes confusion about VPR core's hardware capability, the Kconfig name can be changed.