[PATCH 3/3] firmware: fw_base.S: Add common NMI trap handler

[Anup Patel]

On Mon, Mar 2, 2026 at 9:26 AM Nylon Chen <nylon.chen at sifive.com> wrote:
>
> Radim Krcmar <rkrcmar at qti.qualcomm.com> 於 2026年2月2日週一 下午10:49寫道：
> >
> > 2026-01-29T00:13:43-08:00, Nylon Chen <nylon.chen at sifive.com>:
> > > Add rnmi_handler assembly wrapper and sbi_rnmi_trap_handler default
> > > C handler. The default handler prints diagnostics and hangs the hart.
> > >
> > > Co-developed-by: Zong Li <zong.li at sifive.com>
> > > Signed-off-by: Zong Li <zong.li at sifive.com>
> > > Suggested-by: Nick Hu <nick.hu at sifive.com>
> > > Suggested-by: Samuel Holland <samuel.holland at sifive.com>
> > > Signed-off-by: Nylon Chen <nylon.chen at sifive.com>
> > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang at sifive.com>
> > > ---
> > > diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> > > @@ -643,6 +643,70 @@ memcmp:
> > > +     .section .entry, "ax", %progbits
> > > +     .align 4
> > > +     .globl sbi_rnmi_vector
> > > +sbi_rnmi_vector:
> > > +     /* Swap SP with MNSCRATCH */
> > > +     csrrw   sp, CSR_MNSCRATCH, sp
> > > +
> > > +     /* Allocate space for full trap registers structure */
> > > +     addi    sp, sp, -(SBI_TRAP_REGS_SIZE)
> >
> > [2/3] did:
> >
> >         rnmi_context_offset = sbi_scratch_alloc_offset(SBI_TRAP_REGS_SIZE);
> >         rnmi_ctx = sbi_scratch_offset_ptr(scratch, rnmi_context_offset);
> >         rnmi_sp  = (unsigned long)rnmi_ctx + SBI_TRAP_REGS_SIZE;
> >         csr_write(CSR_MNSCRATCH, rnmi_sp);
> >
> > We can avoid the add/subtract dance, since it produces rnmi_ctx.
> > A bigger issue is that it leaves 0 bytes for the stack of the callback.
> >
> > Is there a reason not to reuse the existing M-mode stack?
> >
> > Thanks.
> Hi Radim
>
> Thank you for the review.
>
> Regarding your question "Is there a reason not to reuse the existing
> M-mode stack?":
>
> We cannot reuse the M-mode stack because RNMI can interrupt M-mode code itself
> If RNMI reuses the M-mode stack, it would corrupt the interrupted
> M-mode function's stack frame.

If you look at the existing M-mode low-level trap handler it already
handles nested traps to support double-traps.

>
> You're absolutely right about the stack space issue.
>
> The current code allocates only SBI_TRAP_REGS_SIZE (280 bytes),
> leaving 0 bytes for the C handler.
>
> I'll fix this by allocating additional stack space:
>
> #define RNMI_STACK_SIZE  2048
> #define RNMI_CONTEXT_SIZE  (SBI_TRAP_REGS_SIZE + RNMI_STACK_SIZE)
> rnmi_context_offset = sbi_scratch_alloc_offset(RNMI_CONTEXT_SIZE);
> rnmi_sp = (unsigned long)rnmi_ctx + RNMI_CONTEXT_SIZE;
>
> This gives the C handler 2048 bytes of stack space while keeping the
> register save area separate.
>
> Does this approach sound reasonable to you?

Please re-use existing M-mode stack.

Regards,
Anup