Re: [PATCH v2 3/5] firmware: Add RNMI handler infrastructure

On Sat, May 2, 2026 at 2:46 AM Evgeny Voevodin <[email protected]> wrote:
>
> Implement basic Resumable NMI (RNMI) handler support for the RISC-V
> Smrnmi extension.
>
> The new _trap_rnmi_handler assembly entry point saves context using the
> Smrnmi MN* CSRs (MNSCRATCH, MNEPC, MNSTATUS, MNCAUSE) and returns via
> mnret. It dispatches to sbi_trap_rnmi_handler(), which optionally calls
> a platform-specific ops->rnmi_handler callback for actual NMI
> processing. If no platform handler is registered or it fails, the
> event is reported as an unhandled NMI.
>
> The RNMI handler reuses the generic trap context structure but stores MN*
> CSR values (MNEPC, MNSTATUS, MNCAUSE) into the corresponding generic
> fields (mepc, mstatus, cause) for compatibility with existing trap
> infrastructure.
>
> Signed-off-by: Evgeny Voevodin <[email protected]>

LGTM.

Reviewed-by: Anup Patel <[email protected]>

Regards,
Anup


> ---
>  firmware/fw_base.S         | 121 +++++++++++++++++++++++++++++++++++++
>  include/sbi/sbi_platform.h |   4 ++
>  include/sbi/sbi_trap.h     |   2 +
>  lib/sbi/sbi_trap.c         |  39 ++++++++++++
>  4 files changed, 166 insertions(+)
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index cdb15429..043de7d7 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -529,6 +529,45 @@ memcmp:
>         csrrw   tp, CSR_MSCRATCH, tp
>  .endm
>
> +.macro TRAP_SAVE_AND_SETUP_SP_T0_NMI
> +       /* Swap TP and MNSCRATCH (for RNMI) */
> +       csrrw   tp, CSR_MNSCRATCH, tp
> +
> +       /* Save T0 in scratch space */
> +       REG_S   t0, SBI_SCRATCH_TMP1_OFFSET(tp)
> +
> +       /*
> +        * Set T0 to appropriate exception stack
> +        *
> +        * Came_From_M_Mode = ((MNSTATUS.MNPP < PRV_M) ? 1 : 0) - 1;
> +        * Exception_Stack = TP ^ (Came_From_M_Mode & (SP ^ TP))
> +        */
> +       csrr    t0, CSR_MNSTATUS
> +       srl     t0, t0, 11              /* MNPP is at bits 11-12 */
> +       and     t0, t0, PRV_M
> +       slti    t0, t0, PRV_M
> +       add     t0, t0, -1
> +       xor     sp, sp, tp
> +       and     t0, t0, sp
> +       xor     sp, sp, tp
> +       xor     t0, tp, t0
> +
> +       /* Save original SP on exception stack */
> +       REG_S   sp, (SBI_TRAP_REGS_OFFSET(sp) - SBI_TRAP_CONTEXT_SIZE)(t0)
> +
> +       /* Set SP to exception stack and make room for trap context */
> +       add     sp, t0, -(SBI_TRAP_CONTEXT_SIZE)
> +
> +       /* Restore T0 from scratch space */
> +       REG_L   t0, SBI_SCRATCH_TMP1_OFFSET(tp)
> +
> +       /* Save T0 on stack */
> +       REG_S   t0, SBI_TRAP_REGS_OFFSET(t0)(sp)
> +
> +       /* Swap TP and MNSCRATCH */
> +       csrrw   tp, CSR_MNSCRATCH, tp
> +.endm
> +
>  .macro TRAP_SAVE_MEPC_MSTATUS have_mstatush
>         /* Save MEPC and MSTATUS CSRs */
>         csrr    t0, CSR_MEPC
> @@ -543,6 +582,20 @@ memcmp:
>         .endif
>  .endm
>
> +.macro TRAP_SAVE_MNEPC_MNSTATUS have_mstatush
> +       /*
> +        * Save MNEPC and MNSTATUS CSRs (for RNMI)
> +        * Note: Trap context structure has generic field names (mepc, mstatus),
> +        * we store MN* CSR values into these same structure fields.
> +        */
> +       csrr    t0, CSR_MNEPC
> +       REG_S   t0, SBI_TRAP_REGS_OFFSET(mepc)(sp)
> +       csrr    t0, CSR_MNSTATUS
> +       REG_S   t0, SBI_TRAP_REGS_OFFSET(mstatus)(sp)
> +       /* MNSTATUSH doesn't exist in SMRNMI spec */
> +       REG_S   zero, SBI_TRAP_REGS_OFFSET(mstatusH)(sp)
> +.endm
> +
>  .macro TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
>         /* Save all general regisers except SP and T0 */
>         REG_S   zero, SBI_TRAP_REGS_OFFSET(zero)(sp)
> @@ -606,12 +659,36 @@ memcmp:
>         CLEAR_MDT t0
>  .endm
>
> +.macro TRAP_SAVE_NMI_INFO
> +       /*
> +        * Save NMI trap info (MNCAUSE, no MNTVAL in spec)
> +        * Note: Trap info structure has generic field names (cause, tval, etc.),
> +        * we store MN* CSR values into these same structure fields.
> +        */
> +       csrr    t0, CSR_MNCAUSE
> +       REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(cause))(sp)
> +       /* MNTVAL doesn't exist in SMRNMI spec */
> +       REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval))(sp)
> +       REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval2))(sp)
> +       REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tinst))(sp)
> +       REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(gva))(sp)
> +
> +       /* We are ready to take another trap, clear MDT */
> +       CLEAR_MDT t0
> +.endm
> +
>  .macro TRAP_CALL_C_ROUTINE
>         /* Call C routine */
>         add     a0, sp, zero
>         call    sbi_trap_handler
>  .endm
>
> +.macro TRAP_CALL_C_RNMI_ROUTINE
> +       /* Call C routine */
> +       add     a0, sp, zero
> +       call    sbi_trap_rnmi_handler
> +.endm
> +
>  .macro TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
>         /* Restore all general regisers except A0 and T0 */
>         REG_L   ra, SBI_TRAP_REGS_OFFSET(ra)(a0)
> @@ -660,6 +737,19 @@ memcmp:
>         csrw    CSR_MEPC, t0
>  .endm
>
> +.macro TRAP_RESTORE_MNEPC_MNSTATUS
> +       /*
> +        * Restore MNSTATUS and MNEPC CSRs (for RNMI)
> +        * Note: Load from generic structure fields (mstatus, mepc) and
> +        * restore to NMI-specific CSRs (MNSTATUS, MNEPC).
> +        * No MNSTATUSH in SMRNMI spec.
> +        */
> +       REG_L   t0, SBI_TRAP_REGS_OFFSET(mstatus)(a0)
> +       csrw    CSR_MNSTATUS, t0
> +       REG_L   t0, SBI_TRAP_REGS_OFFSET(mepc)(a0)
> +       csrw    CSR_MNEPC, t0
> +.endm
> +
>  .macro TRAP_RESTORE_A0_T0
>         /* Restore T0 */
>         REG_L   t0, SBI_TRAP_REGS_OFFSET(t0)(a0)
> @@ -724,6 +814,37 @@ _trap_handler_hyp:
>
>         mret
>
> +       .section .entry, "ax", %progbits
> +       .align 3
> +       .globl _trap_rnmi_handler
> +_trap_rnmi_handler:
> +       /*
> +        * NMI interrupt handler using MN* CSRs
> +        *
> +        * Context detection via MNPP (previous privilege mode):
> +        * - If MNPP < M-mode: use exception stack (TP)
> +        * - If MNPP == M-mode: use current stack (SP)
> +        * This handles nested interrupt cases.
> +        */
> +       TRAP_SAVE_AND_SETUP_SP_T0_NMI
> +
> +       TRAP_SAVE_MNEPC_MNSTATUS 0
> +
> +       TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
> +
> +       TRAP_SAVE_NMI_INFO
> +
> +       TRAP_CALL_C_RNMI_ROUTINE
> +
> +       TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
> +
> +       TRAP_RESTORE_MNEPC_MNSTATUS
> +
> +       TRAP_RESTORE_A0_T0
> +
> +       /* mnret - return from NMI (SMRNMI extension) */
> +       .word 0x70200073
> +
>         .section .entry, "ax", %progbits
>         .align 3
>         .globl _reset_regs
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index e65d9877..715df499 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -149,6 +149,10 @@ struct sbi_platform_operations {
>                         unsigned long log2len);
>         /** platform specific pmp disable on current HART */
>         void (*pmp_disable)(unsigned int n);
> +
> +       /** platform specific Smrnmi NMI handler.
> +        *  Returns SBI_SUCCESS on success, error code if NMI cannot be handled. */
> +       int (*rnmi_handler)(struct sbi_trap_context *tcntx);
>  };
>
>  /** Platform default per-HART stack size for exception/interrupt handling */
> diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
> index 731a0c98..091a2446 100644
> --- a/include/sbi/sbi_trap.h
> +++ b/include/sbi/sbi_trap.h
> @@ -289,6 +289,8 @@ static inline void sbi_trap_set_context(struct sbi_scratch *scratch,
>
>  struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx);
>
> +struct sbi_trap_context *sbi_trap_rnmi_handler(struct sbi_trap_context *tcntx);
> +
>  #endif
>
>  #endif
> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> index f41db4d1..1e55b885 100644
> --- a/lib/sbi/sbi_trap.c
> +++ b/lib/sbi/sbi_trap.c
> @@ -20,6 +20,7 @@
>  #include <sbi/sbi_irqchip.h>
>  #include <sbi/sbi_trap_ldst.h>
>  #include <sbi/sbi_pmu.h>
> +#include <sbi/sbi_platform.h>
>  #include <sbi/sbi_scratch.h>
>  #include <sbi/sbi_sse.h>
>  #include <sbi/sbi_timer.h>
> @@ -375,3 +376,41 @@ trap_done:
>         sbi_trap_set_context(scratch, tcntx->prev_context);
>         return tcntx;
>  }
> +
> +/**
> + * Default Resumable NMI (RNMI) handler
> + *
> + * This function is called from the _trap_rnmi_handler assembly code.
> + * It provides a simple wrapper that calls the platform-specific
> + * NMI handler if registered. If no handler is registered, it prints
> + * diagnostic information and hangs, similar to unhandled traps.
> + *
> + * Note: The trap context stores NMI CSR values (MNCAUSE, MNEPC, MNSTATUS)
> + * in the generic trap context fields (cause, mepc, mstatus).
> + *
> + * @param tcntx Pointer to trap context (saved on stack)
> + * @return Same trap context pointer (needed for restore macros)
> + */
> +struct sbi_trap_context *sbi_trap_rnmi_handler(struct sbi_trap_context *tcntx)
> +{
> +       int rc;
> +       const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> +       const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> +
> +       /* Call platform-specific NMI handler if registered */
> +       if (ops && ops->rnmi_handler) {
> +               rc = ops->rnmi_handler(tcntx);
> +               if (rc) {
> +                       /* Platform handler failed to handle NMI */
> +                       sbi_trap_error("platform NMI handler failed", rc, tcntx);
> +               }
> +               return tcntx;
> +       }
> +
> +       /* No platform handler - treat as unhandled NMI */
> +       sbi_trap_error("unhandled NMI (no platform rnmi_handler)",
> +                      SBI_ENOTSUPP, tcntx);
> +
> +       /* Never returns */
> +       return tcntx;
> +}
> --
> 2.43.0
>

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi