[PATCH 3/7] firmware: Add RNMI/RNME handler infrastructure

[Anup Patel]

On Fri, Apr 10, 2026 at 12:50 AM Evgeny Voevodin
<evvoevod at tenstorrent.com> wrote:
>
> On Wed, Apr 9, 2026 at 8:21 AM Anup Patel <anup at brainfault.org> wrote:
> > On Wed, Mar 11, 2026 at 12:03 AM Evgeny Voevodin
> > <evvoevod at tenstorrent.com> wrote:
> > >
> > > +.macro TRAP_CALL_C_RNME_ROUTINE
> > > +       /* Call C routine */
> > > +       add     a0, sp, zero
> > > +       call    sbi_rnme_handler
> > > +.endm
> > > +
> >
> > s/TRAP_CALL_C_RNME_ROUTINE/TRAP_CALL_C_RNMI_ROUTINE/
>
> This macro is used in the RNME (exception) path to call
> sbi_rnme_handler(), so the current name is intentional.
> See below for why RNME needs a separate path.
>
> > > +_rnmi_handler:
> >
> > s/_rnmi_handler/_trap_rnmi_handler/
>
> Will fix. I'll also rename _rnme_handler to _trap_rnme_handler
> for consistency.
>
> > > +_rnme_handler:
> > > [...]
> > > +       /* mret - return from exception */
> > > +       mret
> > > +
> >
> > Drop this _rnme_handler because C handler can differentiate
> > between NME and NMI based on the MSB of mncause CSR.
>
> Per the Smrnmi extension specification, when an
> exception occurs with NMIE=0:
>
>   "the actions taken are the same as if the exception had
>    occurred while mnstatus.NMIE were set, except that the
>    program counter is set to the RNMI exception trap handler
>    address (rather than the address specified by mtvec)"
>
> This means RNME takes the normal M-mode trap action: it writes
> mepc, mcause, mstatus -- the regular M-mode CSRs, not MN* CSRs.
> mncause is not written by the RNME trap action (it only
> writes mepc, mcause, mstatus), so it retains whatever value
> it had before -- either from a preceding RNMI (MSB=1) or its
> unspecified reset value. In neither case does it reflect the
> RNME exception, so checking mncause MSB cannot reliably
> distinguish RNMI from RNME.
>
> mncause is only written with MSB=0 for exceptions when Smdbltrp
> is also implemented (from the mncause CSR description: "exception
> within M-mode that results in a double trap as specified in the
> Smdbltrp extension").
> With Smdbltrp, both paths go to NMIVEC and use MN* CSRs, so a
> single handler would work. Without it, they differ fundamentally:
>
>   RNMI: MNSCRATCH, MNEPC/MNSTATUS/MNCAUSE, mnret
>   RNME: MSCRATCH,  MEPC/MSTATUS/MCAUSE,    mret
>
> A single assembly entry point cannot serve both because the very
> first instruction must choose which scratch CSR to swap (MNSCRATCH
> vs MSCRATCH), and there is no CSR that can distinguish the two
> cases at that point.

Clearly, for RNME, the existing _trap_handler() can be re-used
because _trap_handler() depends on same set of CSRs and mret
instruction.

I agree that code path RNME should be different from RNMI but
that does not mean we need three separate low-level handlers.

Further, the Smrnmi spec also states the following:
"For example, some implementations might use the address specified
in mtvec as the RNMI exception trap handler."

This means on platforms where mtvec points to the RNMI exception
trap handler address, we will have to anyway re-use _trap_handler()
and sbi_trap_handler() for RNMI exception handling.

>
> I'd like to keep both handlers for correctness with pure Smrnmi.
> Smdbltrp support (and the single-handler optimization) could be
> added in a follow-up series. Would that work?
>
> > > +       int (*rnme_handler)(struct sbi_trap_context *tcntx);
> > > +
> > > +       int (*rnmi_handler)(struct sbi_trap_context *tcntx);
> >
> > Drop these platform callbacks from this patch. We will be
> > extending sbi_irqchip to support NMIs. For NME, we will
> > can simply crash.
>
> Agreed. I'll remove the platform callbacks. For RNMI, I'll
> route through sbi_irqchip once that support is in place. For
> RNME, I'll simplify to just print diagnostics and hang.
>
> > > +struct sbi_trap_context *sbi_rnme_handler(struct sbi_trap_context *tcntx);
> > > +
> > > +struct sbi_trap_context *sbi_rnmi_handler(struct sbi_trap_context *tcntx);
> > > +
> >
> > Only one sbi_trap_rnmi_handler() needed here which will be
> > called for both NMI and NME.
>
> As described above, since the assembly entry points must be
> separate (different CSR sets), I'll keep two C functions:
>
>   - sbi_trap_rnmi_handler(): handles NMI interrupts, will
>     route through sbi_irqchip
>   - sbi_trap_rnme_handler(): handles exceptions with NMIE=0,
>     prints diagnostics and hangs
>
> Both will be renamed with the trap_ prefix as you suggested.
>

Regards,
Anup