[PATCH v3 0/6] Add RNMI handler infrastructure for Smrnmi extension

[PATCH v3 0/6] Add RNMI handler infrastructure for Smrnmi extension

[Evgeny Voevodin]

This is v3 of the Smrnmi series. Based on upstream/master at commit
2257e9957103 ("lib: sbi_bitmap_test: add tests for bitmap_empty()").

v1 thread:
https://lore.kernel.org/opensbi/20260310183155.2186463-1-evvoevod@oss.tenstorrent.com/

v2 thread:
https://lore.kernel.org/opensbi/20260501211627.3293126-1-evvoevod@tenstorrent.com/

Changes vs v2:

- Split former patch 4 ("Create a spot ...") into two patches per
  Anup's review:
    * Patch 4 only moves sbi_platform_extensions_init() to the
      beginning of hart_detect_features().
    * Patch 5 moves Zkr stack-guard initialization out of fw_base.S
      into init_coldboot() in C, drops the asm
      __stack_chk_guard_init() helper.

Testing: Verified on Whisper SW system simulator and on HW emulator.

Evgeny Voevodin (6):
  include: sbi_scratch: Add tmp1 scratch space for RNMI context saving
  lib: sbi: Add Smrnmi extension macros for registers and bits
  firmware: Add RNMI handler infrastructure
  lib: sbi: hart: Move device tree features detection before trap-based
    checks
  lib: sbi: Move Zkr entropy initialization from fw_base.S to
    init_coldboot
  lib: sbi: hart: Detect and enable Smrnmi before trap-based feature
    detection

 firmware/fw_base.S           | 156 +++++++++++++++++++++++++++--------
 include/sbi/riscv_encoding.h |  10 +++
 include/sbi/sbi_hart.h       |   2 +
 include/sbi/sbi_platform.h   |   8 ++
 include/sbi/sbi_scratch.h    |  11 ++-
 include/sbi/sbi_trap.h       |   2 +
 lib/sbi/sbi_hart.c           |  37 +++++++--
 lib/sbi/sbi_init.c           |  31 +++++++
 lib/sbi/sbi_trap.c           |  39 +++++++++
 9 files changed, 254 insertions(+), 42 deletions(-)

-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 1/6] include: sbi_scratch: Add tmp1 scratch space for RNMI context saving

[Evgeny Voevodin]

RNMI handlers use MNSCRATCH instead of MSCRATCH and need separate scratch
space from regular trap handling. Add tmp1 for RNMI context while tmp0
remains for regular traps.

Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 firmware/fw_base.S        |  3 ++-
 include/sbi/sbi_scratch.h | 11 ++++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index 63bb4473..cdb15429 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -258,9 +258,10 @@ _scratch_init:
 	/* Store hartid-to-scratch function address in scratch space */
 	lla	a4, _hartid_to_scratch
 	REG_S	a4, SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET(tp)
-	/* Clear trap_context and tmp0 in scratch space */
+	/* Clear trap_context, tmp0 and tmp1 in scratch space */
 	REG_S	zero, SBI_SCRATCH_TRAP_CONTEXT_OFFSET(tp)
 	REG_S	zero, SBI_SCRATCH_TMP0_OFFSET(tp)
+	REG_S	zero, SBI_SCRATCH_TMP1_OFFSET(tp)
 	/* Store firmware options in scratch space */
 	MOV_3R	s0, a0, s1, a1, s2, a2
 #ifdef FW_OPTIONS
diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
index 58d54628..a6edeb2d 100644
--- a/include/sbi/sbi_scratch.h
+++ b/include/sbi/sbi_scratch.h
@@ -40,12 +40,14 @@
 #define SBI_SCRATCH_TRAP_CONTEXT_OFFSET		(11 * __SIZEOF_POINTER__)
 /** Offset of tmp0 member in sbi_scratch */
 #define SBI_SCRATCH_TMP0_OFFSET			(12 * __SIZEOF_POINTER__)
+/** Offset of tmp1 member in sbi_scratch */
+#define SBI_SCRATCH_TMP1_OFFSET			(13 * __SIZEOF_POINTER__)
 /** Offset of options member in sbi_scratch */
-#define SBI_SCRATCH_OPTIONS_OFFSET		(13 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_OPTIONS_OFFSET		(14 * __SIZEOF_POINTER__)
 /** Offset of hartindex member in sbi_scratch */
-#define SBI_SCRATCH_HARTINDEX_OFFSET		(14 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_HARTINDEX_OFFSET		(15 * __SIZEOF_POINTER__)
 /** Offset of extra space in sbi_scratch */
-#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(15 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(16 * __SIZEOF_POINTER__)
 /** Maximum size of sbi_scratch (4KB) */
 #define SBI_SCRATCH_SIZE			(0x1000)
 
@@ -83,6 +85,8 @@ struct sbi_scratch {
 	unsigned long trap_context;
 	/** Temporary storage */
 	unsigned long tmp0;
+	/** Temporary storage */
+	unsigned long tmp1;
 	/** Options for OpenSBI library */
 	unsigned long options;
 	/** Index of the hart */
@@ -106,6 +110,7 @@ assert_member_offset(struct sbi_scratch, platform_addr, SBI_SCRATCH_PLATFORM_ADD
 assert_member_offset(struct sbi_scratch, hartid_to_scratch, SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET);
 assert_member_offset(struct sbi_scratch, trap_context, SBI_SCRATCH_TRAP_CONTEXT_OFFSET);
 assert_member_offset(struct sbi_scratch, tmp0, SBI_SCRATCH_TMP0_OFFSET);
+assert_member_offset(struct sbi_scratch, tmp1, SBI_SCRATCH_TMP1_OFFSET);
 assert_member_offset(struct sbi_scratch, options, SBI_SCRATCH_OPTIONS_OFFSET);
 assert_member_offset(struct sbi_scratch, hartindex, SBI_SCRATCH_HARTINDEX_OFFSET);
 
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 2/6] lib: sbi: Add Smrnmi extension macros for registers and bits

[Evgeny Voevodin]

Add CSR definitions (MNSCRATCH, MNSTATUS, MNEPC, MNCAUSE) and bit definitions
(MNSTATUS_NMIE, MNSTATUS_MNPV, MNSTATUS_MNPP). Also add SBI_HART_EXT_SMRNMI to
the hart extension enumeration.

Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 include/sbi/riscv_encoding.h | 10 ++++++++++
 include/sbi/sbi_hart.h       |  2 ++
 lib/sbi/sbi_hart.c           |  1 +
 3 files changed, 13 insertions(+)

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 3c1d5256..18f7b4a7 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -215,6 +215,10 @@
 
 #endif
 
+#define MNSTATUS_NMIE			(_UL(0x8))
+#define MNSTATUS_MNPV			(_UL(0x80))
+#define MNSTATUS_MNPP			(_UL(0x1800))
+
 #define MHPMEVENT_SSCOF_MASK		_ULL(0xFF00000000000000)
 
 #define ENVCFG_STCE			(_ULL(1) << 63)
@@ -830,6 +834,12 @@
 #define CSR_CUSTOM10_M_RO_BASE		0xFC0
 #define CSR_CUSTOM10_M_RO_COUNT		0x040
 
+/* Smrnmi extension registers */
+#define CSR_MNSCRATCH				0x740
+#define CSR_MNEPC					0x741
+#define CSR_MNCAUSE					0x742
+#define CSR_MNSTATUS				0x744
+
 /* ===== Trap/Exception Causes ===== */
 
 #define CAUSE_MISALIGNED_FETCH		0x0
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index a788b34c..937cdf29 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -87,6 +87,8 @@ enum sbi_hart_extensions {
 	SBI_HART_EXT_XSIFIVE_CFLUSH_D_L1,
 	/** Hart has Xsfcease extension */
 	SBI_HART_EXT_XSIFIVE_CEASE,
+	/** Hart has Smrnmi extension */
+	SBI_HART_EXT_SMRNMI,
 
 	/** Maximum index of Hart extension */
 	SBI_HART_EXT_MAX,
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 99e13990..4aefb759 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -396,6 +396,7 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
 	__SBI_HART_EXT_DATA(ssstateen, SBI_HART_EXT_SSSTATEEN),
 	__SBI_HART_EXT_DATA(xsfcflushdlone, SBI_HART_EXT_XSIFIVE_CFLUSH_D_L1),
 	__SBI_HART_EXT_DATA(xsfcease, SBI_HART_EXT_XSIFIVE_CEASE),
+	__SBI_HART_EXT_DATA(smrnmi, SBI_HART_EXT_SMRNMI),
 };
 
 _Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext),
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 2/6] lib: sbi: Add Smrnmi extension macros for registers and bits

[Nylon Chen]

Evgeny Voevodin <evvoevod@tenstorrent.com> 於 2026年5月8日週五 上午2:08寫道：
>
> Add CSR definitions (MNSCRATCH, MNSTATUS, MNEPC, MNCAUSE) and bit definitions
> (MNSTATUS_NMIE, MNSTATUS_MNPV, MNSTATUS_MNPP). Also add SBI_HART_EXT_SMRNMI to
> the hart extension enumeration.
>
> Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  include/sbi/riscv_encoding.h | 10 ++++++++++
>  include/sbi/sbi_hart.h       |  2 ++
>  lib/sbi/sbi_hart.c           |  1 +
>  3 files changed, 13 insertions(+)
>
> diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> index 3c1d5256..18f7b4a7 100644
> --- a/include/sbi/riscv_encoding.h
> +++ b/include/sbi/riscv_encoding.h
> @@ -215,6 +215,10 @@
>
>  #endif
>
> +#define MNSTATUS_NMIE                  (_UL(0x8))
> +#define MNSTATUS_MNPV                  (_UL(0x80))
> +#define MNSTATUS_MNPP                  (_UL(0x1800))
Please add define fo mnstatus.MNPELP bit as well.
> +
>  #define MHPMEVENT_SSCOF_MASK           _ULL(0xFF00000000000000)
>
>  #define ENVCFG_STCE                    (_ULL(1) << 63)
> @@ -830,6 +834,12 @@
>  #define CSR_CUSTOM10_M_RO_BASE         0xFC0
>  #define CSR_CUSTOM10_M_RO_COUNT                0x040
>
> +/* Smrnmi extension registers */
> +#define CSR_MNSCRATCH                          0x740
> +#define CSR_MNEPC                                      0x741
> +#define CSR_MNCAUSE                                    0x742
> +#define CSR_MNSTATUS                           0x744
> +
>  /* ===== Trap/Exception Causes ===== */
>
>  #define CAUSE_MISALIGNED_FETCH         0x0
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index a788b34c..937cdf29 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -87,6 +87,8 @@ enum sbi_hart_extensions {
>         SBI_HART_EXT_XSIFIVE_CFLUSH_D_L1,
>         /** Hart has Xsfcease extension */
>         SBI_HART_EXT_XSIFIVE_CEASE,
> +       /** Hart has Smrnmi extension */
> +       SBI_HART_EXT_SMRNMI,
>
>         /** Maximum index of Hart extension */
>         SBI_HART_EXT_MAX,
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 99e13990..4aefb759 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -396,6 +396,7 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
>         __SBI_HART_EXT_DATA(ssstateen, SBI_HART_EXT_SSSTATEEN),
>         __SBI_HART_EXT_DATA(xsfcflushdlone, SBI_HART_EXT_XSIFIVE_CFLUSH_D_L1),
>         __SBI_HART_EXT_DATA(xsfcease, SBI_HART_EXT_XSIFIVE_CEASE),
> +       __SBI_HART_EXT_DATA(smrnmi, SBI_HART_EXT_SMRNMI),
>  };
>
>  _Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext),
> --
> 2.43.0
>
Otherwise, LGTM

Reviewed-by: Nylon Chen <nylon.chen@sifive.com>

Regards,
Nylon

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 3/6] firmware: Add RNMI handler infrastructure

[Evgeny Voevodin]

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 <evvoevod@tenstorrent.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 4/6] lib: sbi: hart: Move device tree features detection before trap-based checks

[Evgeny Voevodin]

Smrnmi detection and enablement in the following commits will happen
before any trap-based mechanism. As it relies on device tree, move
sbi_platform_extensions_init() to the beginning of hart_detect_features().

Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
---
 lib/sbi/sbi_hart.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 4aefb759..781161e5 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -522,6 +522,16 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	hfeatures->mhpm_mask = 0;
 	hfeatures->priv_version = SBI_HART_PRIV_VER_UNKNOWN;
 
+	/*
+	 * Parse device tree extensions early, before any trap-based checks.
+	 * Needed to detect Smrnmi and install NMI handlers before CSR probes
+	 * that may trigger traps.
+	 */
+	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
+					  hfeatures);
+	if (rc)
+		return rc;
+
 #define __check_hpm_csr(__csr, __mask) 					  \
 	oldval = csr_read_allowed(__csr, &trap);			  \
 	if (!trap.cause) {						  \
@@ -676,12 +686,6 @@ __pmp_skip:
 
 #undef __check_csr_existence
 
-	/* Let platform populate extensions */
-	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
-					  hfeatures);
-	if (rc)
-		return rc;
-
 	/* Zicntr should only be detected using traps */
 	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
 			    sbi_hart_has_csr(scratch, SBI_HART_CSR_CYCLE) &&
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 4/6] lib: sbi: hart: Move device tree features detection before trap-based checks

[Anup Patel]

On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
<evvoevod@tenstorrent.com> wrote:
>
> Smrnmi detection and enablement in the following commits will happen
> before any trap-based mechanism. As it relies on device tree, move
> sbi_platform_extensions_init() to the beginning of hart_detect_features().
>
> Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  lib/sbi/sbi_hart.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 4aefb759..781161e5 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -522,6 +522,16 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>         hfeatures->mhpm_mask = 0;
>         hfeatures->priv_version = SBI_HART_PRIV_VER_UNKNOWN;
>
> +       /*
> +        * Parse device tree extensions early, before any trap-based checks.
> +        * Needed to detect Smrnmi and install NMI handlers before CSR probes
> +        * that may trigger traps.
> +        */
> +       rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
> +                                         hfeatures);
> +       if (rc)
> +               return rc;
> +
>  #define __check_hpm_csr(__csr, __mask)                                           \
>         oldval = csr_read_allowed(__csr, &trap);                          \
>         if (!trap.cause) {                                                \
> @@ -676,12 +686,6 @@ __pmp_skip:
>
>  #undef __check_csr_existence
>
> -       /* Let platform populate extensions */
> -       rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
> -                                         hfeatures);
> -       if (rc)
> -               return rc;
> -
>         /* Zicntr should only be detected using traps */
>         __sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
>                             sbi_hart_has_csr(scratch, SBI_HART_CSR_CYCLE) &&
> --
> 2.43.0
>

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 5/6] lib: sbi: Move Zkr entropy initialization from fw_base.S to init_coldboot

[Evgeny Voevodin]

Current placement of entropy initialization via Zkr extension requires a
trap-based mechanism to handle absent Zkr extension case. In presence of
Smrnmi extension no trap-based mechanisms should be used before Smrnmi is
detected and enabled otherwise trap will jump to undefined location.
Move stack guard initialization into init_coldboot function body after
device tree has been parsed so we know if Zkr extension is implemented by
the platform which helps to avoid trap-based discovery.
init_coldboot() is a safe place to initialize entropy because it doesn't
return so no check of __stack_chk_guard against value on entry
will be done.

Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
---
 firmware/fw_base.S | 32 --------------------------------
 lib/sbi/sbi_init.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index 043de7d7..5d6540d7 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -107,30 +107,6 @@ _bss_zero:
 	add	s4, s4, __SIZEOF_POINTER__
 	blt	s4, s5, _bss_zero
 
-	/* Trying to initialize the stack guard via the Zkr extension */
-	lla	t0, __stack_chk_guard_done
-	csrw	CSR_MTVEC, t0
-	li	t0, 0
-	li	t3, SEED_OPTS_ES16
-	li	t4, SEED_ENTROPY_MASK
-	li	t5, __SIZEOF_POINTER__
-__stack_chk_guard_loop:
-	csrrw	t1, CSR_SEED, x0
-	li	t2, SEED_OPTS_MASK
-	and	t2, t2, t1
-	bgtu	t2, t3, __stack_chk_guard_done
-	bltu	t2, t3, __stack_chk_guard_loop
-	and	t1, t1, t4
-	slli	t0, t0, 16
-	or	t0, t0, t1
-	addi	t5, t5, -2
-	bgtz	t5, __stack_chk_guard_loop
-	lla	t1, __stack_chk_guard
-	REG_S	t0, 0(t1)
-	j	__stack_chk_guard_done
-	.align 3
-__stack_chk_guard_done:
-
 	/* Setup temporary trap handler */
 	lla	s4, _start_hang
 	csrw	CSR_MTVEC, s4
@@ -895,14 +871,6 @@ __stack_chk_fail:
 	la	a0, .Lstack_corrupt_msg
 	call	sbi_panic
 
-	/* Initial value of the stack guard variable */
-	.section .data
-	.align 3
-	.globl __stack_chk_guard
-	.type __stack_chk_guard, %object
-__stack_chk_guard:
-	RISCV_PTR	0x95B5FF5A
-
 #ifdef FW_FDT_PATH
 	.section .rodata
 	.align 4
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index aae035e1..b248e73f 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -218,6 +218,8 @@ static void wake_coldboot_harts(struct sbi_scratch *scratch)
 	__smp_store_release(&coldboot_done, 1);
 }
 
+unsigned long __stack_chk_guard = 0x95B5FF5A;
+
 static unsigned long entry_count_offset;
 static unsigned long init_count_offset;
 
@@ -269,6 +271,35 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
 	if (rc)
 		sbi_hart_hang();
 
+	/*
+	 * Initialize stack guard via Zkr entropy source if Zkr is
+	 * implemented according to device tree. Writing new seed value
+	 * to __stack_chk_guard is safe here because function doesn't
+	 * return and no check against value on entry will be done.
+	 */
+	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZKR)) {
+		unsigned long guard_val = 0;
+		int chunks = sizeof(unsigned long) / sizeof(uint16_t);
+		bool res = true;
+
+		while (chunks) {
+			unsigned long seed = csr_swap(CSR_SEED, 0);
+			unsigned long opst = seed & SEED_OPTS_MASK;
+
+			if (opst == SEED_OPTS_DEAD) {
+				res = false;
+				break;
+			}
+			if (opst == SEED_OPTS_ES16) {
+				guard_val = (guard_val << 16) | (seed & SEED_ENTROPY_MASK);
+				chunks--;
+			}
+			continue;
+		}
+		if (res)
+			__stack_chk_guard = guard_val;
+	}
+
 	rc = sbi_timer_init(scratch, true);
 	if (rc)
 		sbi_hart_hang();
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 5/6] lib: sbi: Move Zkr entropy initialization from fw_base.S to init_coldboot

[Anup Patel]

On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
<evvoevod@tenstorrent.com> wrote:
>
> Current placement of entropy initialization via Zkr extension requires a
> trap-based mechanism to handle absent Zkr extension case. In presence of
> Smrnmi extension no trap-based mechanisms should be used before Smrnmi is
> detected and enabled otherwise trap will jump to undefined location.
> Move stack guard initialization into init_coldboot function body after
> device tree has been parsed so we know if Zkr extension is implemented by
> the platform which helps to avoid trap-based discovery.
> init_coldboot() is a safe place to initialize entropy because it doesn't
> return so no check of __stack_chk_guard against value on entry
> will be done.
>
> Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  firmware/fw_base.S | 32 --------------------------------
>  lib/sbi/sbi_init.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 043de7d7..5d6540d7 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -107,30 +107,6 @@ _bss_zero:
>         add     s4, s4, __SIZEOF_POINTER__
>         blt     s4, s5, _bss_zero
>
> -       /* Trying to initialize the stack guard via the Zkr extension */
> -       lla     t0, __stack_chk_guard_done
> -       csrw    CSR_MTVEC, t0
> -       li      t0, 0
> -       li      t3, SEED_OPTS_ES16
> -       li      t4, SEED_ENTROPY_MASK
> -       li      t5, __SIZEOF_POINTER__
> -__stack_chk_guard_loop:
> -       csrrw   t1, CSR_SEED, x0
> -       li      t2, SEED_OPTS_MASK
> -       and     t2, t2, t1
> -       bgtu    t2, t3, __stack_chk_guard_done
> -       bltu    t2, t3, __stack_chk_guard_loop
> -       and     t1, t1, t4
> -       slli    t0, t0, 16
> -       or      t0, t0, t1
> -       addi    t5, t5, -2
> -       bgtz    t5, __stack_chk_guard_loop
> -       lla     t1, __stack_chk_guard
> -       REG_S   t0, 0(t1)
> -       j       __stack_chk_guard_done
> -       .align 3
> -__stack_chk_guard_done:
> -
>         /* Setup temporary trap handler */
>         lla     s4, _start_hang
>         csrw    CSR_MTVEC, s4
> @@ -895,14 +871,6 @@ __stack_chk_fail:
>         la      a0, .Lstack_corrupt_msg
>         call    sbi_panic
>
> -       /* Initial value of the stack guard variable */
> -       .section .data
> -       .align 3
> -       .globl __stack_chk_guard
> -       .type __stack_chk_guard, %object
> -__stack_chk_guard:
> -       RISCV_PTR       0x95B5FF5A
> -
>  #ifdef FW_FDT_PATH
>         .section .rodata
>         .align 4
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index aae035e1..b248e73f 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -218,6 +218,8 @@ static void wake_coldboot_harts(struct sbi_scratch *scratch)
>         __smp_store_release(&coldboot_done, 1);
>  }
>
> +unsigned long __stack_chk_guard = 0x95B5FF5A;
> +
>  static unsigned long entry_count_offset;
>  static unsigned long init_count_offset;
>
> @@ -269,6 +271,35 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
>         if (rc)
>                 sbi_hart_hang();
>
> +       /*
> +        * Initialize stack guard via Zkr entropy source if Zkr is
> +        * implemented according to device tree. Writing new seed value
> +        * to __stack_chk_guard is safe here because function doesn't
> +        * return and no check against value on entry will be done.
> +        */
> +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZKR)) {
> +               unsigned long guard_val = 0;
> +               int chunks = sizeof(unsigned long) / sizeof(uint16_t);
> +               bool res = true;
> +
> +               while (chunks) {
> +                       unsigned long seed = csr_swap(CSR_SEED, 0);
> +                       unsigned long opst = seed & SEED_OPTS_MASK;
> +
> +                       if (opst == SEED_OPTS_DEAD) {
> +                               res = false;
> +                               break;
> +                       }
> +                       if (opst == SEED_OPTS_ES16) {
> +                               guard_val = (guard_val << 16) | (seed & SEED_ENTROPY_MASK);
> +                               chunks--;
> +                       }
> +                       continue;
> +               }
> +               if (res)
> +                       __stack_chk_guard = guard_val;
> +       }
> +
>         rc = sbi_timer_init(scratch, true);
>         if (rc)
>                 sbi_hart_hang();
> --
> 2.43.0
>

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Evgeny Voevodin]

The location of the RNMI/E trap vectors in the Smrnmi extension is
implementation-defined, so platforms with vendor-specific NMI vector
mechanisms must install the firmware's NMI entry points themselves.

Add an smrnmi_handlers_init() callback to sbi_platform_operations that
receives the firmware entry points and lets platform code install them
at the hardware-specific vector locations. Two pointers are passed:

  - _trap_rnmi_handler: the dedicated RNMI entry point that saves
    context using the Smrnmi MN* CSRs and returns via mnret.
  - _trap_handler: the regular M-mode trap entry since RNME is taken
    as a regular M-mode trap with NMIE=0.

When Smrnmi is present, install the platform's NMI vectors via the new
callback, initialize MNSCRATCH with the per-hart scratch pointer, and
set MNSTATUS.NMIE.

Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
extension is detected but no callback is registered, sbi_panic() is
called since enabling NMIs without handlers in place would route
subsequent traps into nowhere.

Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
---
 include/sbi/sbi_platform.h |  4 ++++
 lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index 715df499..fe382b56 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -150,6 +150,10 @@ struct sbi_platform_operations {
 	/** platform specific pmp disable on current HART */
 	void (*pmp_disable)(unsigned int n);
 
+	/** platform specific Smrnmi handlers init on current HART */
+	void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
+			void (*rnme_handler)(void));
+
 	/** 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);
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 781161e5..92c602aa 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	if (rc)
 		return rc;
 
+	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
+		const struct sbi_platform *plat = sbi_platform_thishart_ptr();
+		const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
+		extern void _trap_rnmi_handler(void);
+		extern void _trap_handler(void);
+
+		if (!ops || !ops->smrnmi_handlers_init)
+			sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
+
+		/* Reuse _trap_handler for the RNME slot since RNME is taken
+		 * as a regular M-mode trap with NMIE=0. */
+		ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
+
+		/* Initialize MNSCRATCH for the RNMI handler */
+		csr_write(CSR_MNSCRATCH, scratch);
+
+		/* Enable NMIs */
+		csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
+	}
+
 #define __check_hpm_csr(__csr, __mask) 					  \
 	oldval = csr_read_allowed(__csr, &trap);			  \
 	if (!trap.cause) {						  \
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Anup Patel]

On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
<evvoevod@tenstorrent.com> wrote:
>
> The location of the RNMI/E trap vectors in the Smrnmi extension is
> implementation-defined, so platforms with vendor-specific NMI vector
> mechanisms must install the firmware's NMI entry points themselves.
>
> Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> receives the firmware entry points and lets platform code install them
> at the hardware-specific vector locations. Two pointers are passed:
>
>   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
>     context using the Smrnmi MN* CSRs and returns via mnret.
>   - _trap_handler: the regular M-mode trap entry since RNME is taken
>     as a regular M-mode trap with NMIE=0.
>
> When Smrnmi is present, install the platform's NMI vectors via the new
> callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> set MNSTATUS.NMIE.
>
> Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> extension is detected but no callback is registered, sbi_panic() is
> called since enabling NMIs without handlers in place would route
> subsequent traps into nowhere.
>
> Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  include/sbi/sbi_platform.h |  4 ++++
>  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 715df499..fe382b56 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -150,6 +150,10 @@ struct sbi_platform_operations {
>         /** platform specific pmp disable on current HART */
>         void (*pmp_disable)(unsigned int n);
>
> +       /** platform specific Smrnmi handlers init on current HART */
> +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> +                       void (*rnme_handler)(void));
> +
>         /** 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);
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 781161e5..92c602aa 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>         if (rc)
>                 return rc;
>
> +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> +               extern void _trap_rnmi_handler(void);
> +               extern void _trap_handler(void);
> +
> +               if (!ops || !ops->smrnmi_handlers_init)
> +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> +
> +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> +                * as a regular M-mode trap with NMIE=0. */
> +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> +
> +               /* Initialize MNSCRATCH for the RNMI handler */
> +               csr_write(CSR_MNSCRATCH, scratch);
> +
> +               /* Enable NMIs */
> +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> +       }
> +
>  #define __check_hpm_csr(__csr, __mask)                                           \
>         oldval = csr_read_allowed(__csr, &trap);                          \
>         if (!trap.cause) {                                                \
> --
> 2.43.0
>

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Nylon Chen]

Anup Patel <anup@brainfault.org> 於 2026年5月9日週六 下午2:51寫道：

>
> On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
> <evvoevod@tenstorrent.com> wrote:
> >
> > The location of the RNMI/E trap vectors in the Smrnmi extension is
> > implementation-defined, so platforms with vendor-specific NMI vector
> > mechanisms must install the firmware's NMI entry points themselves.
> >
> > Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> > receives the firmware entry points and lets platform code install them
> > at the hardware-specific vector locations. Two pointers are passed:
> >
> >   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
> >     context using the Smrnmi MN* CSRs and returns via mnret.
> >   - _trap_handler: the regular M-mode trap entry since RNME is taken
> >     as a regular M-mode trap with NMIE=0.
> >
> > When Smrnmi is present, install the platform's NMI vectors via the new
> > callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> > set MNSTATUS.NMIE.
> >
> > Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> > extension is detected but no callback is registered, sbi_panic() is
> > called since enabling NMIs without handlers in place would route
> > subsequent traps into nowhere.
> >
> > Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
>
> LGTM.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
>
> > ---
> >  include/sbi/sbi_platform.h |  4 ++++
> >  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > index 715df499..fe382b56 100644
> > --- a/include/sbi/sbi_platform.h
> > +++ b/include/sbi/sbi_platform.h
> > @@ -150,6 +150,10 @@ struct sbi_platform_operations {
> >         /** platform specific pmp disable on current HART */
> >         void (*pmp_disable)(unsigned int n);
> >
> > +       /** platform specific Smrnmi handlers init on current HART */
> > +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> > +                       void (*rnme_handler)(void));
> > +
> >         /** 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);
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 781161e5..92c602aa 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> >         if (rc)
> >                 return rc;
> >
> > +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> > +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> > +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> > +               extern void _trap_rnmi_handler(void);
> > +               extern void _trap_handler(void);
> > +
> > +               if (!ops || !ops->smrnmi_handlers_init)
> > +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> > +
> > +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> > +                * as a regular M-mode trap with NMIE=0. */
> > +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> > +
> > +               /* Initialize MNSCRATCH for the RNMI handler */
> > +               csr_write(CSR_MNSCRATCH, scratch);
> > +
> > +               /* Enable NMIs */
> > +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
Two concerns about this block

First, sbi_panic() when smrnmi_handlers_init is NULL is too strict.
Not all platforms with Smrnmi need to program a vendor-specific NMI
vector register.
Some implementations have a fixed NMI vector address or route NMIs
through mtvec by default; for these, setting MNSCRATCH and NMIE is
sufficient and no platform callback is needed.

This can be reproduced with QEMU virt:

qemu-system-riscv64 -M virt -cpu rv64,smrnmi=on -m 256m \
      -bios fw_jump.bin -nographic -no-reboot

The firmware hangs with zero output. The panic fires before
sbi_platform_early_init() so the console is not yet initialized and
the message is silently dropped.

Adding a no-op stub to the generic platform confirms the panic is  the
sole cause -- boot proceeds normally with the stub in place.

Second, the callback returns void, so there is no way to detect
whether the platform succeeded in programming the NMI vector.
csr_set(MNSTATUS, NMIE) then runs unconditionally.
If the callback fails silently, NMIs are enabled but the hardware
vector register is left at its reset value. The first NMI would jump
to an undefined address.

Suggested fix:

/* allow NULL: platforms with fixed or mtvec-based NMI vectors
* do not need to program any vendor register */
if (ops && ops->smrnmi_handlers_init) {
        int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
                                            _trap_handler);
        if (ret)
            return ret;
}

csr_write(CSR_MNSCRATCH, scratch);
csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);

This requires changing the callback signature from void to int.

Platforms that have nothing to program return 0, platforms that fail
return an error, and platforms with no callback at all are handled
gracefully without a panic.

> > +       }
> > +
> >  #define __check_hpm_csr(__csr, __mask)                                           \
> >         oldval = csr_read_allowed(__csr, &trap);                          \
> >         if (!trap.cause) {                                                \
> > --
> > 2.43.0
> >

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Anup Patel]

On Tue, May 12, 2026 at 12:14 PM Nylon Chen <nylon.chen@sifive.com> wrote:
>
> Anup Patel <anup@brainfault.org> 於 2026年5月9日週六 下午2:51寫道：
>
> >
> > On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
> > <evvoevod@tenstorrent.com> wrote:
> > >
> > > The location of the RNMI/E trap vectors in the Smrnmi extension is
> > > implementation-defined, so platforms with vendor-specific NMI vector
> > > mechanisms must install the firmware's NMI entry points themselves.
> > >
> > > Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> > > receives the firmware entry points and lets platform code install them
> > > at the hardware-specific vector locations. Two pointers are passed:
> > >
> > >   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
> > >     context using the Smrnmi MN* CSRs and returns via mnret.
> > >   - _trap_handler: the regular M-mode trap entry since RNME is taken
> > >     as a regular M-mode trap with NMIE=0.
> > >
> > > When Smrnmi is present, install the platform's NMI vectors via the new
> > > callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> > > set MNSTATUS.NMIE.
> > >
> > > Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> > > extension is detected but no callback is registered, sbi_panic() is
> > > called since enabling NMIs without handlers in place would route
> > > subsequent traps into nowhere.
> > >
> > > Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
> >
> > LGTM.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
> >
> > Regards,
> > Anup
> >
> > > ---
> > >  include/sbi/sbi_platform.h |  4 ++++
> > >  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > > index 715df499..fe382b56 100644
> > > --- a/include/sbi/sbi_platform.h
> > > +++ b/include/sbi/sbi_platform.h
> > > @@ -150,6 +150,10 @@ struct sbi_platform_operations {
> > >         /** platform specific pmp disable on current HART */
> > >         void (*pmp_disable)(unsigned int n);
> > >
> > > +       /** platform specific Smrnmi handlers init on current HART */
> > > +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> > > +                       void (*rnme_handler)(void));
> > > +
> > >         /** 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);
> > > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > > index 781161e5..92c602aa 100644
> > > --- a/lib/sbi/sbi_hart.c
> > > +++ b/lib/sbi/sbi_hart.c
> > > @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> > >         if (rc)
> > >                 return rc;
> > >
> > > +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> > > +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> > > +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> > > +               extern void _trap_rnmi_handler(void);
> > > +               extern void _trap_handler(void);
> > > +
> > > +               if (!ops || !ops->smrnmi_handlers_init)
> > > +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> > > +
> > > +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> > > +                * as a regular M-mode trap with NMIE=0. */
> > > +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> > > +
> > > +               /* Initialize MNSCRATCH for the RNMI handler */
> > > +               csr_write(CSR_MNSCRATCH, scratch);
> > > +
> > > +               /* Enable NMIs */
> > > +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> Two concerns about this block
>
> First, sbi_panic() when smrnmi_handlers_init is NULL is too strict.
> Not all platforms with Smrnmi need to program a vendor-specific NMI
> vector register.
> Some implementations have a fixed NMI vector address or route NMIs
> through mtvec by default; for these, setting MNSCRATCH and NMIE is
> sufficient and no platform callback is needed.
>
> This can be reproduced with QEMU virt:
>
> qemu-system-riscv64 -M virt -cpu rv64,smrnmi=on -m 256m \
>       -bios fw_jump.bin -nographic -no-reboot
>
> The firmware hangs with zero output. The panic fires before
> sbi_platform_early_init() so the console is not yet initialized and
> the message is silently dropped.
>
> Adding a no-op stub to the generic platform confirms the panic is  the
> sole cause -- boot proceeds normally with the stub in place.
>
> Second, the callback returns void, so there is no way to detect
> whether the platform succeeded in programming the NMI vector.
> csr_set(MNSTATUS, NMIE) then runs unconditionally.
> If the callback fails silently, NMIs are enabled but the hardware
> vector register is left at its reset value. The first NMI would jump
> to an undefined address.
>
> Suggested fix:
>
> /* allow NULL: platforms with fixed or mtvec-based NMI vectors
> * do not need to program any vendor register */
> if (ops && ops->smrnmi_handlers_init) {
>         int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
>                                             _trap_handler);
>         if (ret)
>             return ret;
> }
>
> csr_write(CSR_MNSCRATCH, scratch);
> csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
>
> This requires changing the callback signature from void to int.
>
> Platforms that have nothing to program return 0, platforms that fail
> return an error, and platforms with no callback at all are handled
> gracefully without a panic.
>

Please send a patch with appropriate Fixes tag.

Regards,
Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Nylon Chen]

Anup Patel <anup@brainfault.org> 於 2026年5月12日週二 下午3:59寫道：
>
> On Tue, May 12, 2026 at 12:14 PM Nylon Chen <nylon.chen@sifive.com> wrote:
> >
> > Anup Patel <anup@brainfault.org> 於 2026年5月9日週六 下午2:51寫道：
> >
> > >
> > > On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
> > > <evvoevod@tenstorrent.com> wrote:
> > > >
> > > > The location of the RNMI/E trap vectors in the Smrnmi extension is
> > > > implementation-defined, so platforms with vendor-specific NMI vector
> > > > mechanisms must install the firmware's NMI entry points themselves.
> > > >
> > > > Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> > > > receives the firmware entry points and lets platform code install them
> > > > at the hardware-specific vector locations. Two pointers are passed:
> > > >
> > > >   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
> > > >     context using the Smrnmi MN* CSRs and returns via mnret.
> > > >   - _trap_handler: the regular M-mode trap entry since RNME is taken
> > > >     as a regular M-mode trap with NMIE=0.
> > > >
> > > > When Smrnmi is present, install the platform's NMI vectors via the new
> > > > callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> > > > set MNSTATUS.NMIE.
> > > >
> > > > Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> > > > extension is detected but no callback is registered, sbi_panic() is
> > > > called since enabling NMIs without handlers in place would route
> > > > subsequent traps into nowhere.
> > > >
> > > > Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
> > >
> > > LGTM.
> > >
> > > Reviewed-by: Anup Patel <anup@brainfault.org>
> > >
> > > Regards,
> > > Anup
> > >
> > > > ---
> > > >  include/sbi/sbi_platform.h |  4 ++++
> > > >  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
> > > >  2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > > > index 715df499..fe382b56 100644
> > > > --- a/include/sbi/sbi_platform.h
> > > > +++ b/include/sbi/sbi_platform.h
> > > > @@ -150,6 +150,10 @@ struct sbi_platform_operations {
> > > >         /** platform specific pmp disable on current HART */
> > > >         void (*pmp_disable)(unsigned int n);
> > > >
> > > > +       /** platform specific Smrnmi handlers init on current HART */
> > > > +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> > > > +                       void (*rnme_handler)(void));
> > > > +
> > > >         /** 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);
> > > > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > > > index 781161e5..92c602aa 100644
> > > > --- a/lib/sbi/sbi_hart.c
> > > > +++ b/lib/sbi/sbi_hart.c
> > > > @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> > > >         if (rc)
> > > >                 return rc;
> > > >
> > > > +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> > > > +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> > > > +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> > > > +               extern void _trap_rnmi_handler(void);
> > > > +               extern void _trap_handler(void);
> > > > +
> > > > +               if (!ops || !ops->smrnmi_handlers_init)
> > > > +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> > > > +
> > > > +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> > > > +                * as a regular M-mode trap with NMIE=0. */
> > > > +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> > > > +
> > > > +               /* Initialize MNSCRATCH for the RNMI handler */
> > > > +               csr_write(CSR_MNSCRATCH, scratch);
> > > > +
> > > > +               /* Enable NMIs */
> > > > +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> > Two concerns about this block
> >
> > First, sbi_panic() when smrnmi_handlers_init is NULL is too strict.
> > Not all platforms with Smrnmi need to program a vendor-specific NMI
> > vector register.
> > Some implementations have a fixed NMI vector address or route NMIs
> > through mtvec by default; for these, setting MNSCRATCH and NMIE is
> > sufficient and no platform callback is needed.
> >
> > This can be reproduced with QEMU virt:
> >
> > qemu-system-riscv64 -M virt -cpu rv64,smrnmi=on -m 256m \
> >       -bios fw_jump.bin -nographic -no-reboot
> >
> > The firmware hangs with zero output. The panic fires before
> > sbi_platform_early_init() so the console is not yet initialized and
> > the message is silently dropped.
> >
> > Adding a no-op stub to the generic platform confirms the panic is  the
> > sole cause -- boot proceeds normally with the stub in place.
> >
> > Second, the callback returns void, so there is no way to detect
> > whether the platform succeeded in programming the NMI vector.
> > csr_set(MNSTATUS, NMIE) then runs unconditionally.
> > If the callback fails silently, NMIs are enabled but the hardware
> > vector register is left at its reset value. The first NMI would jump
> > to an undefined address.
> >
> > Suggested fix:
> >
> > /* allow NULL: platforms with fixed or mtvec-based NMI vectors
> > * do not need to program any vendor register */
> > if (ops && ops->smrnmi_handlers_init) {
> >         int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
> >                                             _trap_handler);
> >         if (ret)
> >             return ret;
> > }
> >
> > csr_write(CSR_MNSCRATCH, scratch);
> > csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> >
> > This requires changing the callback signature from void to int.
> >
> > Platforms that have nothing to program return 0, platforms that fail
> > return an error, and platforms with no callback at all are handled
> > gracefully without a panic.
> >
>
> Please send a patch with appropriate Fixes tag.
Will do.

While preparing the patch, I also noticed the series is missing
non-retentive suspend/resume handling for CSR_MNSCRATCH and
  MNSTATUS.NMIE.

After non-retentive suspend, the hart is fully powered off and all CSR
state is reset.

On resume, sbi_hsm_hart_resume_finish() calls
__sbi_hsm_suspend_non_ret_restore() and jumps directly to the kernel
without going through sbi_hart_init(), so the Smrnmi CSRs set up in
hart_detect_features() are never re-initialized.

I will send both fixes together as a two-patch series after testing.

>
> Regards,
> Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Anup Patel]

On Tue, May 12, 2026 at 2:00 PM Nylon Chen <nylon.chen@sifive.com> wrote:
>
> Anup Patel <anup@brainfault.org> 於 2026年5月12日週二 下午3:59寫道：
> >
> > On Tue, May 12, 2026 at 12:14 PM Nylon Chen <nylon.chen@sifive.com> wrote:
> > >
> > > Anup Patel <anup@brainfault.org> 於 2026年5月9日週六 下午2:51寫道：
> > >
> > > >
> > > > On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
> > > > <evvoevod@tenstorrent.com> wrote:
> > > > >
> > > > > The location of the RNMI/E trap vectors in the Smrnmi extension is
> > > > > implementation-defined, so platforms with vendor-specific NMI vector
> > > > > mechanisms must install the firmware's NMI entry points themselves.
> > > > >
> > > > > Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> > > > > receives the firmware entry points and lets platform code install them
> > > > > at the hardware-specific vector locations. Two pointers are passed:
> > > > >
> > > > >   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
> > > > >     context using the Smrnmi MN* CSRs and returns via mnret.
> > > > >   - _trap_handler: the regular M-mode trap entry since RNME is taken
> > > > >     as a regular M-mode trap with NMIE=0.
> > > > >
> > > > > When Smrnmi is present, install the platform's NMI vectors via the new
> > > > > callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> > > > > set MNSTATUS.NMIE.
> > > > >
> > > > > Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> > > > > extension is detected but no callback is registered, sbi_panic() is
> > > > > called since enabling NMIs without handlers in place would route
> > > > > subsequent traps into nowhere.
> > > > >
> > > > > Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
> > > >
> > > > LGTM.
> > > >
> > > > Reviewed-by: Anup Patel <anup@brainfault.org>
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > > ---
> > > > >  include/sbi/sbi_platform.h |  4 ++++
> > > > >  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
> > > > >  2 files changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > > > > index 715df499..fe382b56 100644
> > > > > --- a/include/sbi/sbi_platform.h
> > > > > +++ b/include/sbi/sbi_platform.h
> > > > > @@ -150,6 +150,10 @@ struct sbi_platform_operations {
> > > > >         /** platform specific pmp disable on current HART */
> > > > >         void (*pmp_disable)(unsigned int n);
> > > > >
> > > > > +       /** platform specific Smrnmi handlers init on current HART */
> > > > > +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> > > > > +                       void (*rnme_handler)(void));
> > > > > +
> > > > >         /** 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);
> > > > > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > > > > index 781161e5..92c602aa 100644
> > > > > --- a/lib/sbi/sbi_hart.c
> > > > > +++ b/lib/sbi/sbi_hart.c
> > > > > @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> > > > >         if (rc)
> > > > >                 return rc;
> > > > >
> > > > > +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> > > > > +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> > > > > +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> > > > > +               extern void _trap_rnmi_handler(void);
> > > > > +               extern void _trap_handler(void);
> > > > > +
> > > > > +               if (!ops || !ops->smrnmi_handlers_init)
> > > > > +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> > > > > +
> > > > > +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> > > > > +                * as a regular M-mode trap with NMIE=0. */
> > > > > +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> > > > > +
> > > > > +               /* Initialize MNSCRATCH for the RNMI handler */
> > > > > +               csr_write(CSR_MNSCRATCH, scratch);
> > > > > +
> > > > > +               /* Enable NMIs */
> > > > > +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> > > Two concerns about this block
> > >
> > > First, sbi_panic() when smrnmi_handlers_init is NULL is too strict.
> > > Not all platforms with Smrnmi need to program a vendor-specific NMI
> > > vector register.
> > > Some implementations have a fixed NMI vector address or route NMIs
> > > through mtvec by default; for these, setting MNSCRATCH and NMIE is
> > > sufficient and no platform callback is needed.
> > >
> > > This can be reproduced with QEMU virt:
> > >
> > > qemu-system-riscv64 -M virt -cpu rv64,smrnmi=on -m 256m \
> > >       -bios fw_jump.bin -nographic -no-reboot
> > >
> > > The firmware hangs with zero output. The panic fires before
> > > sbi_platform_early_init() so the console is not yet initialized and
> > > the message is silently dropped.
> > >
> > > Adding a no-op stub to the generic platform confirms the panic is  the
> > > sole cause -- boot proceeds normally with the stub in place.
> > >
> > > Second, the callback returns void, so there is no way to detect
> > > whether the platform succeeded in programming the NMI vector.
> > > csr_set(MNSTATUS, NMIE) then runs unconditionally.
> > > If the callback fails silently, NMIs are enabled but the hardware
> > > vector register is left at its reset value. The first NMI would jump
> > > to an undefined address.
> > >
> > > Suggested fix:
> > >
> > > /* allow NULL: platforms with fixed or mtvec-based NMI vectors
> > > * do not need to program any vendor register */
> > > if (ops && ops->smrnmi_handlers_init) {
> > >         int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
> > >                                             _trap_handler);
> > >         if (ret)
> > >             return ret;
> > > }
> > >
> > > csr_write(CSR_MNSCRATCH, scratch);
> > > csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> > >
> > > This requires changing the callback signature from void to int.
> > >
> > > Platforms that have nothing to program return 0, platforms that fail
> > > return an error, and platforms with no callback at all are handled
> > > gracefully without a panic.
> > >
> >
> > Please send a patch with appropriate Fixes tag.
> Will do.
>
> While preparing the patch, I also noticed the series is missing
> non-retentive suspend/resume handling for CSR_MNSCRATCH and
>   MNSTATUS.NMIE.
>
> After non-retentive suspend, the hart is fully powered off and all CSR
> state is reset.
>
> On resume, sbi_hsm_hart_resume_finish() calls
> __sbi_hsm_suspend_non_ret_restore() and jumps directly to the kernel
> without going through sbi_hart_init(), so the Smrnmi CSRs set up in
> hart_detect_features() are never re-initialized.

I think Smrnmi in hart_detect_features() should be moved to a
separate function which should be called from two places:
1) At the current location from hart_detect_features() before
we use trap-based extension detection
2) In sbi_hart_init() for both cold boot and warm boot path

Regards,
Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 6/6] lib: sbi: hart: Detect and enable Smrnmi before trap-based feature detection

[Evgeny Voevodin]

On Tue May 12, 2026 at 3:40 PM UTC, Anup Patel wrote:
> On Tue, May 12, 2026 at 2:00 PM Nylon Chen <nylon.chen@sifive.com> wrote:
> >
> > Anup Patel <anup@brainfault.org> 於 2026年5月12日週二 下午3:59寫道：
> > >
> > > On Tue, May 12, 2026 at 12:14 PM Nylon Chen <nylon.chen@sifive.com> wrote:
> > > >
> > > > Anup Patel <anup@brainfault.org> 於 2026年5月9日週六 下午2:51寫道：
> > > >
> > > > >
> > > > > On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
> > > > > <evvoevod@tenstorrent.com> wrote:
> > > > > >
> > > > > > The location of the RNMI/E trap vectors in the Smrnmi extension is
> > > > > > implementation-defined, so platforms with vendor-specific NMI vector
> > > > > > mechanisms must install the firmware's NMI entry points themselves.
> > > > > >
> > > > > > Add an smrnmi_handlers_init() callback to sbi_platform_operations that
> > > > > > receives the firmware entry points and lets platform code install them
> > > > > > at the hardware-specific vector locations. Two pointers are passed:
> > > > > >
> > > > > >   - _trap_rnmi_handler: the dedicated RNMI entry point that saves
> > > > > >     context using the Smrnmi MN* CSRs and returns via mnret.
> > > > > >   - _trap_handler: the regular M-mode trap entry since RNME is taken
> > > > > >     as a regular M-mode trap with NMIE=0.
> > > > > >
> > > > > > When Smrnmi is present, install the platform's NMI vectors via the new
> > > > > > callback, initialize MNSCRATCH with the per-hart scratch pointer, and
> > > > > > set MNSTATUS.NMIE.
> > > > > >
> > > > > > Smrnmi-enabled platforms must register smrnmi_handlers_init; if the
> > > > > > extension is detected but no callback is registered, sbi_panic() is
> > > > > > called since enabling NMIs without handlers in place would route
> > > > > > subsequent traps into nowhere.
> > > > > >
> > > > > > Signed-off-by: Evgeny Voevodin <evvoevod@tenstorrent.com>
> > > > >
> > > > > LGTM.
> > > > >
> > > > > Reviewed-by: Anup Patel <anup@brainfault.org>
> > > > >
> > > > > Regards,
> > > > > Anup
> > > > >
> > > > > > ---
> > > > > >  include/sbi/sbi_platform.h |  4 ++++
> > > > > >  lib/sbi/sbi_hart.c         | 20 ++++++++++++++++++++
> > > > > >  2 files changed, 24 insertions(+)
> > > > > >
> > > > > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > > > > > index 715df499..fe382b56 100644
> > > > > > --- a/include/sbi/sbi_platform.h
> > > > > > +++ b/include/sbi/sbi_platform.h
> > > > > > @@ -150,6 +150,10 @@ struct sbi_platform_operations {
> > > > > >         /** platform specific pmp disable on current HART */
> > > > > >         void (*pmp_disable)(unsigned int n);
> > > > > >
> > > > > > +       /** platform specific Smrnmi handlers init on current HART */
> > > > > > +       void (*smrnmi_handlers_init)(void (*rnmi_handler)(void),
> > > > > > +                       void (*rnme_handler)(void));
> > > > > > +
> > > > > >         /** 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);
> > > > > > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > > > > > index 781161e5..92c602aa 100644
> > > > > > --- a/lib/sbi/sbi_hart.c
> > > > > > +++ b/lib/sbi/sbi_hart.c
> > > > > > @@ -532,6 +532,26 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> > > > > >         if (rc)
> > > > > >                 return rc;
> > > > > >
> > > > > > +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
> > > > > > +               const struct sbi_platform *plat = sbi_platform_thishart_ptr();
> > > > > > +               const struct sbi_platform_operations *ops = sbi_platform_ops(plat);
> > > > > > +               extern void _trap_rnmi_handler(void);
> > > > > > +               extern void _trap_handler(void);
> > > > > > +
> > > > > > +               if (!ops || !ops->smrnmi_handlers_init)
> > > > > > +                       sbi_panic("Smrnmi detected, but platform lacks smrnmi_handlers_init callback\n");
> > > > > > +
> > > > > > +               /* Reuse _trap_handler for the RNME slot since RNME is taken
> > > > > > +                * as a regular M-mode trap with NMIE=0. */
> > > > > > +               ops->smrnmi_handlers_init(_trap_rnmi_handler, _trap_handler);
> > > > > > +
> > > > > > +               /* Initialize MNSCRATCH for the RNMI handler */
> > > > > > +               csr_write(CSR_MNSCRATCH, scratch);
> > > > > > +
> > > > > > +               /* Enable NMIs */
> > > > > > +               csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> > > > Two concerns about this block
> > > >
> > > > First, sbi_panic() when smrnmi_handlers_init is NULL is too strict.
> > > > Not all platforms with Smrnmi need to program a vendor-specific NMI
> > > > vector register.
> > > > Some implementations have a fixed NMI vector address or route NMIs
> > > > through mtvec by default; for these, setting MNSCRATCH and NMIE is
> > > > sufficient and no platform callback is needed.
> > > >
> > > > This can be reproduced with QEMU virt:
> > > >
> > > > qemu-system-riscv64 -M virt -cpu rv64,smrnmi=on -m 256m \
> > > >       -bios fw_jump.bin -nographic -no-reboot
> > > >
> > > > The firmware hangs with zero output. The panic fires before
> > > > sbi_platform_early_init() so the console is not yet initialized and
> > > > the message is silently dropped.

Agree, silent hang regresses platforms which don't need to setup Rnmi/e
handlers explicitly and just use default vectors.

> > > >
> > > > Adding a no-op stub to the generic platform confirms the panic is  the
> > > > sole cause -- boot proceeds normally with the stub in place.
> > > >
> > > > Second, the callback returns void, so there is no way to detect
> > > > whether the platform succeeded in programming the NMI vector.
> > > > csr_set(MNSTATUS, NMIE) then runs unconditionally.
> > > > If the callback fails silently, NMIs are enabled but the hardware
> > > > vector register is left at its reset value. The first NMI would jump
> > > > to an undefined address.
> > > >
> > > > Suggested fix:
> > > >
> > > > /* allow NULL: platforms with fixed or mtvec-based NMI vectors
> > > > * do not need to program any vendor register */
> > > > if (ops && ops->smrnmi_handlers_init) {
> > > >         int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
> > > >                                             _trap_handler);
> > > >         if (ret)
> > > >             return ret;
> > > > }
> > > >
> > > > csr_write(CSR_MNSCRATCH, scratch);
> > > > csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
> > > >
> > > > This requires changing the callback signature from void to int.
> > > >
> > > > Platforms that have nothing to program return 0, platforms that fail
> > > > return an error, and platforms with no callback at all are handled
> > > > gracefully without a panic.

What I would suggest here:
Instead of a stub handler provided by a platform which doesn't require
handlers setup, let's introduce a flag that will simply indicate if
platform provided handlers setup callback or not. Later, when console
becomes available, just print the informational line in the boot banner:

Smrnmi subsystem     : Default     <-- or "Platform"

This will help to not regress existing platforms and also provide some
diagnostics for those platforms which require handlers initialization
but missed that. Sketch implementation could be:
      enum {
          SMRNMI_INIT_NONE,
          SMRNMI_INIT_DEFAULT,
          SMRNMI_INIT_PLATFORM,
      };
      static int smrnmi_init_mode;

      if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMRNMI)) {
          if (ops && ops->smrnmi_handlers_init) {
              int ret = ops->smrnmi_handlers_init(_trap_rnmi_handler,
                                                  _trap_handler);
              if (ret)
                  return ret;
              smrnmi_init_mode = SMRNMI_INIT_PLATFORM;
          } else {
              smrnmi_init_mode = SMRNMI_INIT_DEFAULT;
          }
          csr_write(CSR_MNSCRATCH, scratch);
          csr_set(CSR_MNSTATUS, MNSTATUS_NMIE);
      }

  And in the boot info printer (sbi_boot_print_general_info() or
  wherever the banner lives):

      if (smrnmi_init_mode != SMRNMI_INIT_NONE)
          sbi_printf("Smrnmi subsystem            : %s\n",
                     smrnmi_init_mode == SMRNMI_INIT_PLATFORM
                         ? "Platform" : "Default");


> > > Please send a patch with appropriate Fixes tag.
> > Will do.
> >
> > While preparing the patch, I also noticed the series is missing
> > non-retentive suspend/resume handling for CSR_MNSCRATCH and
> >   MNSTATUS.NMIE.
> >
> > After non-retentive suspend, the hart is fully powered off and all CSR
> > state is reset.
> >
> > On resume, sbi_hsm_hart_resume_finish() calls
> > __sbi_hsm_suspend_non_ret_restore() and jumps directly to the kernel
> > without going through sbi_hart_init(), so the Smrnmi CSRs set up in
> > hart_detect_features() are never re-initialized.
>
> I think Smrnmi in hart_detect_features() should be moved to a
> separate function which should be called from two places:
> 1) At the current location from hart_detect_features() before
> we use trap-based extension detection
> 2) In sbi_hart_init() for both cold boot and warm boot path
>
> Regards,
> Anup

I think __sbi_hsm_suspend_non_ret_save/restore is the right place to
preserve and restore MNSCRATCH and MNSTATUS CSRs as recovery from
non-retentive state bypasses sbi_hart_init(). Also, these functions are
placeholders for other CSRs save/restore which don't survive during
non-retentive entry.

Thanks,
Evgeny


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v3 0/6] Add RNMI handler infrastructure for Smrnmi extension

[Anup Patel]

On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
<evvoevod@tenstorrent.com> wrote:
>
> This is v3 of the Smrnmi series. Based on upstream/master at commit
> 2257e9957103 ("lib: sbi_bitmap_test: add tests for bitmap_empty()").
>
> v1 thread:
> https://lore.kernel.org/opensbi/20260310183155.2186463-1-evvoevod@oss.tenstorrent.com/
>
> v2 thread:
> https://lore.kernel.org/opensbi/20260501211627.3293126-1-evvoevod@tenstorrent.com/
>
> Changes vs v2:
>
> - Split former patch 4 ("Create a spot ...") into two patches per
>   Anup's review:
>     * Patch 4 only moves sbi_platform_extensions_init() to the
>       beginning of hart_detect_features().
>     * Patch 5 moves Zkr stack-guard initialization out of fw_base.S
>       into init_coldboot() in C, drops the asm
>       __stack_chk_guard_init() helper.
>
> Testing: Verified on Whisper SW system simulator and on HW emulator.
>
> Evgeny Voevodin (6):
>   include: sbi_scratch: Add tmp1 scratch space for RNMI context saving
>   lib: sbi: Add Smrnmi extension macros for registers and bits
>   firmware: Add RNMI handler infrastructure
>   lib: sbi: hart: Move device tree features detection before trap-based
>     checks
>   lib: sbi: Move Zkr entropy initialization from fw_base.S to
>     init_coldboot
>   lib: sbi: hart: Detect and enable Smrnmi before trap-based feature
>     detection
>
>  firmware/fw_base.S           | 156 +++++++++++++++++++++++++++--------
>  include/sbi/riscv_encoding.h |  10 +++
>  include/sbi/sbi_hart.h       |   2 +
>  include/sbi/sbi_platform.h   |   8 ++
>  include/sbi/sbi_scratch.h    |  11 ++-
>  include/sbi/sbi_trap.h       |   2 +
>  lib/sbi/sbi_hart.c           |  37 +++++++--
>  lib/sbi/sbi_init.c           |  31 +++++++
>  lib/sbi/sbi_trap.c           |  39 +++++++++
>  9 files changed, 254 insertions(+), 42 deletions(-)
>
> --
> 2.43.0
>

Applied this series to the riscv/opensbi repo.

Thanks,
Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi