Skip to content

arch/risc-v: qemu: don't rewind interrupt stack sp on nested trap#19468

Merged
acassis merged 1 commit into
apache:masterfrom
hitHuang:fix/riscv_setintstack
Jul 19, 2026
Merged

arch/risc-v: qemu: don't rewind interrupt stack sp on nested trap#19468
acassis merged 1 commit into
apache:masterfrom
hitHuang:fix/riscv_setintstack

Conversation

@hitHuang

Copy link
Copy Markdown
Contributor

Summary

qemu-rv's S-mode/non-SMP setintstack macro (arch/risc-v/src/qemu-rv/chip.h) unconditionally reloaded sp to 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 rewinds sp back 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 setintstack in riscv_macros.S already guards this reload with a bounds check: it only moves sp when sp is outside the interrupt stack range. This change ports that same logic to qemu-rv's per-cpu RISCV_PERCPU_IRQSTACK variant, and factors the shared bounds-check sequence out into a setintstack_bounds macro reused by both the SMP and S-mode/non-SMP setintstack variants in chip.h.

Other vendor chip.h files (litex, mpfs, and the non-SMP setintstack variants 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 setintstack on qemu-rv specifically: its S-mode variant was added in 2022 (3193aa3c97), as a plain unconditional REGLOAD sp, RISCV_PERCPU_IRQSTACK(...) — the nested-trap bounds-check concept didn't exist yet. The bounds check itself was introduced later, in 2024, by b4174952843 ("arch/risc-v: support backtrace dump during IRQ"). That commit only touched two places: the canonical macro in riscv_macros.S, and qemu-rv's SMP setintstack variant that already existed in chip.h at 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 setintstack for S-mode or SMP the way there is for the plain single-core M-mode case in riscv_macros.S — each chip vendor's chip.h reimplements both variants independently, copying the same style of code. Checking the other 9 vendor chip.h files (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

  • Affects only arch/risc-v/src/qemu-rv/chip.h, specifically the S-mode/non-SMP setintstack macro used when CONFIG_ARCH_USE_S_MODE=y && !CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 15.
  • No Kconfig/API changes. No effect on builds without a dedicated interrupt stack, or on the SMP variant (already had the bounds check).
  • Fixes real stack-memory corruption on nested traps in this configuration; no behavior change for the non-nested-trap case.

Testing

Config: CONFIG_BUILD_KERNEL=y, non-SMP, CONFIG_ARCH_INTERRUPTSTACK > 15.

Boards: qemu rv-virt:knsh (RV32) and rv-virt:knsh64 (RV64).

To reliably reproduce a fault occurring inside an interrupt handler (so the nested-trap path through setintstack is exercised), the mtimer interrupt handler was temporarily instrumented to dereference a NULL pointer when a test flag is set via gdb:

huang@ubuntu:~/workspace/nuttx-fork/nuttx$ git diff arch/risc-v/src/common/riscv_mtimer.c
diff --git a/arch/risc-v/src/common/riscv_mtimer.c b/arch/risc-v/src/common/riscv_mtimer.c
index 3c29088667..7140d89f5a 100644
--- a/arch/risc-v/src/common/riscv_mtimer.c
+++ b/arch/risc-v/src/common/riscv_mtimer.c
@@ -26,9 +26,13 @@

 #include <nuttx/arch.h>
 #include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <sched.h>

 #include "riscv_mtimer.h"

+volatile int g_mtimer_test = 0;
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
@@ -132,6 +136,12 @@ static int riscv_mtimer_interrupt(int irq, void *context, void *arg)
 {
   struct riscv_mtimer_lowerhalf_s *priv = &g_riscv_mtime_lowerhalf;

+  if (g_mtimer_test == 1)
+    {
+      g_mtimer_test = 0;
+      *(volatile uint32_t *)0 = 0;
+    }
+
   riscv_mtimer_set_mtimecmp(priv, UINT64_MAX);
   oneshot_process_callback(&priv->lower);

Test method: set breakpoints on riscv_mtimer_interrupt() and riscv_exception() via gdb, and capture sp at each breakpoint to check whether the nested trap's entry into riscv_exception() clobbers the still-live outer (mtimer interrupt) frame.

Before fix:

0x00001000 in ?? ()
add symbol table from file "../../nuttx-fork/nuttx/nuttx"
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x80003ecc in ?? ()
(gdb) b riscv_exception
Breakpoint 1 at 0x8020c730: file common/riscv_exception.c, line 148.
(gdb) b riscv_mtimer_interrupt
Breakpoint 2 at 0x8020cc3a: file common/riscv_mtimer.c, line 139.
(gdb) c
Continuing.

Breakpoint 2, riscv_mtimer_interrupt (irq=21, context=0x80608564, arg=0x80600138 <g_riscv_mtime_lowerhalf>) at common/riscv_mtimer.c:139
139       if (g_mtimer_test == 1)
(gdb) info registers sp fp
sp             0x806067a8       0x806067a8 <g_intstackalloc+1960>
fp             0x806067b8       0x806067b8 <g_intstackalloc+1976>
(gdb) set variable g_mtimer_test=1
(gdb) c
Continuing.

Breakpoint 1, riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
148       uintreg_t cause = mcause & RISCV_IRQ_MASK;
(gdb) info registers sp fp
sp             0x806067a8       0x806067a8 <g_intstackalloc+1960>
fp             0x806067b8       0x806067b8 <g_intstackalloc+1976>
(gdb) bt
#0  riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
#1  0x80201fec in irq_dispatch (irq=irq@entry=7, context=<optimized out>) at irq/irq_dispatch.c:143
#2  0x8020162a in riscv_doirq (irq=7, regs=0x8060661c <g_intstackalloc+1564>) at common/riscv_doirq.c:112
#3  0x80201262 in riscv_dispatch_irq (vector=<optimized out>, regs=<optimized out>) at chip/qemu_rv_irq_dispatch.c:140
#4  0x802001d2 in handle_irq () at common/riscv_exception_common.S:238
#5  0x00000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

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:

0x00001000 in ?? ()
add symbol table from file "../../nuttx-fork/nuttx/nuttx"
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x8020cf7e in up_idle () at common/riscv_idle.c:77
77      }
(gdb) b riscv_exception
Breakpoint 1 at 0x8020c730: file common/riscv_exception.c, line 148.
(gdb) b riscv_mtimer_interrupt
Breakpoint 2 at 0x8020cc3a: file common/riscv_mtimer.c, line 139.
(gdb) c
Continuing.

Breakpoint 2, riscv_mtimer_interrupt (irq=21, context=0x80608564, arg=0x80600138 <g_riscv_mtime_lowerhalf>) at common/riscv_mtimer.c:139
139       if (g_mtimer_test == 1)
(gdb) info registers sp fp
sp             0x806067a8       0x806067a8 <g_intstackalloc+1960>
fp             0x806067b8       0x806067b8 <g_intstackalloc+1976>
(gdb) set variable g_mtimer_test=1
(gdb) c
Continuing.

Breakpoint 1, riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
148       uintreg_t cause = mcause & RISCV_IRQ_MASK;
(gdb) info registers sp fp
sp             0x806065c4       0x806065c4 <g_intstackalloc+1476>
fp             0x806065d4       0x806065d4 <g_intstackalloc+1492>
(gdb) bt
#0  riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
#1  0x80201fec in irq_dispatch (irq=irq@entry=7, context=<optimized out>) at irq/irq_dispatch.c:143
#2  0x8020162a in riscv_doirq (irq=7, regs=0x8060661c <g_intstackalloc+1564>) at common/riscv_doirq.c:112
#3  0x80201262 in riscv_dispatch_irq (vector=<optimized out>, regs=<optimized out>) at chip/qemu_rv_irq_dispatch.c:140
#4  0x802001e8 in handle_irq () at common/riscv_exception_common.S:238
#5  0x8020cc4c in riscv_mtimer_interrupt (irq=7, context=0x8060661c <g_intstackalloc+1564>, arg=0x0) at common/riscv_mtimer.c:141
Backtrace stopped: frame did not save the PC

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.

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>
@github-actions github-actions Bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: S The size of the change in this PR is small labels Jul 18, 2026
@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

hifive1-revb

@acassis
acassis merged commit 42d684b into apache:master Jul 19, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants