arch/risc-v: qemu: don't rewind interrupt stack sp on nested trap#19468
Open
hitHuang wants to merge 1 commit into
Open
arch/risc-v: qemu: don't rewind interrupt stack sp on nested trap#19468hitHuang wants to merge 1 commit into
hitHuang wants to merge 1 commit into
Conversation
qemu-rv's S-mode/non-SMP setintstack unconditionally reloaded sp to the top of the per-cpu interrupt stack. A trap taken while already running on that stack rewound sp back to the same address, so the nested trap's frame overwrote the still-live outer trap's frame. Port the bounds check already used by the canonical setintstack in riscv_macros.S: only move sp when it is outside the interrupt stack range. Other vendor chip.h files have the same unconditional-reload pattern and are left for a follow-up. Signed-off-by: liang.huang <liang.huang@houmo.ai>
hitHuang
requested review from
masayuki2009,
pussuw,
tmedicci and
xiaoxiang781216
as code owners
July 18, 2026 12:20
|
xiaoxiang781216
approved these changes
Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
qemu-rv's S-mode/non-SMP
setintstackmacro (arch/risc-v/src/qemu-rv/chip.h) unconditionally reloadedspto the top of the per-cpu interrupt stack on every trap entry. If a trap is taken while already running on the interrupt stack (e.g. a fault occurring inside an interrupt handler), this rewindsspback to the same fixed top-of-stack address, causing the nested trap's frame to be laid down on top of the still-live outer trap's frame and overwrite it.The canonical
setintstackinriscv_macros.Salready guards this reload with a bounds check: it only movesspwhenspis outside the interrupt stack range. This change ports that same logic to qemu-rv's per-cpuRISCV_PERCPU_IRQSTACKvariant, and factors the shared bounds-check sequence out into asetintstack_boundsmacro reused by both the SMP and S-mode/non-SMPsetintstackvariants inchip.h.Other vendor
chip.hfiles (litex, mpfs, and the non-SMPsetintstackvariants of jh7110/k230/sg2000) have the same unconditional-reload pattern and are left for a follow-up, since they cannot be exercised on QEMU.Why other vendor chips aren't fixed in this PR
Tracing the history of
setintstackon qemu-rv specifically: its S-mode variant was added in 2022 (3193aa3c97), as a plain unconditionalREGLOAD sp, RISCV_PERCPU_IRQSTACK(...)— the nested-trap bounds-check concept didn't exist yet. The bounds check itself was introduced later, in 2024, byb4174952843("arch/risc-v: support backtrace dump during IRQ"). That commit only touched two places: the canonical macro inriscv_macros.S, and qemu-rv's SMPsetintstackvariant that already existed inchip.hat the time. Its own commit message ("Tested with SMP, no SMP and no interrupt stack") shows the S-mode/non-SMP combination simply wasn't in the test matrix, so qemu-rv's S-mode variant was left with the 2022 unconditional-reload code until this PR.There is no shared/canonical
setintstackfor S-mode or SMP the way there is for the plain single-core M-mode case inriscv_macros.S— each chip vendor'schip.hreimplements both variants independently, copying the same style of code. Checking the other 9 vendorchip.hfiles (litex, mpfs, jh7110, k230, sg2000, eic7700x, bl808, k210, rp23xx-rv) shows every one of them still has the plain unconditional reload in both their SMP and S-mode variants — the 2024 bounds-check fix was never propagated to any of them, not just their S-mode paths. That's the actual shape of the current fragmentation: one single-file fix landed on qemu-rv only, and nothing else was ever touched. I don't have any of that hardware on hand to reproduce and verify a fix, so they're left untouched for now rather than changed without being able to test them.Impact
arch/risc-v/src/qemu-rv/chip.h, specifically the S-mode/non-SMPsetintstackmacro used whenCONFIG_ARCH_USE_S_MODE=y && !CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 15.Testing
Config:
CONFIG_BUILD_KERNEL=y, non-SMP,CONFIG_ARCH_INTERRUPTSTACK > 15.Boards: qemu
rv-virt:knsh(RV32) andrv-virt:knsh64(RV64).To reliably reproduce a fault occurring inside an interrupt handler (so the nested-trap path through
setintstackis exercised), the mtimer interrupt handler was temporarily instrumented to dereference a NULL pointer when a test flag is set via gdb:Test method: set breakpoints on
riscv_mtimer_interrupt()andriscv_exception()via gdb, and capturespat each breakpoint to check whether the nested trap's entry intoriscv_exception()clobbers the still-live outer (mtimer interrupt) frame.Before fix:
This shows that for the exception caused by the NULL dereference inside riscv_mtimer_interrupt(), sp upon entering riscv_exception() is again at the top of the interrupt stack.
After fix:
After the fix, sp at the riscv_exception() breakpoint now has a sane value: still on the interrupt stack, but no longer at the top.
Also regression-tested on knsh64 to confirm the SMP variant is unaffected.