Re: [PATCH v2 4/5] lib: sbi: Create a spot to place Smrnmi detection before traps and after DT is ready

On Sat, May 2, 2026 at 2:46 AM Evgeny Voevodin <[email protected]> wrote:
>
> Since Smrnmi is detected from the device tree, move
> sbi_platform_extensions_init() before trap-based feature detection so
> the extension is known when the next commit acts on it.
>
> The Zkr seed-CSR access can trap if Zkr is not implemented. On Smrnmi
> platforms NMIE=0 by default after reset, which routes that trap to
> NMEVEC rather than MTVEC. Until Smrnmi handlers are installed and
> NMIE=1, no NMEVEC handler is in place, so an early-boot trap there
> would crash. Refactor the inline seed loop in fw_base.S into a
> callable __stack_chk_guard_init function and invoke it from
> init_coldboot() after sbi_hart_init() returns; the next commit makes
> sbi_hart_init() install Smrnmi handlers and set NMIE=1, so by the time
> __stack_chk_guard_init runs the trap path is safe.
>
> Signed-off-by: Evgeny Voevodin <[email protected]>
> ---
>  firmware/fw_base.S | 64 +++++++++++++++++++++++++++++-----------------
>  lib/sbi/sbi_hart.c | 16 +++++++-----
>  lib/sbi/sbi_init.c | 10 ++++++++
>  3 files changed, 60 insertions(+), 30 deletions(-)
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 043de7d7..ebecfecc 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,6 +871,46 @@ __stack_chk_fail:
>         la      a0, .Lstack_corrupt_msg
>         call    sbi_panic
>
> +       /*
> +        * Initialize __stack_chk_guard from the Zkr seed CSR. Called from C
> +        * after Smrnmi has been enabled (NMIE=1), so that a missing-Zkr
> +        * illegal-instruction trap is delivered to MTVEC normally and is
> +        * caught by our temporary MTVEC redirect below.
> +        */
> +       .align 3
> +       .globl __stack_chk_guard_init
> +       .type __stack_chk_guard_init, %function
> +__stack_chk_guard_init:
> +       /* Save current MTVEC so we can restore it on return */
> +       csrr    t6, CSR_MTVEC
> +
> +       /* Redirect MTVEC to local skip target for Zkr-not-supported case */
> +       lla     t0, 1f
> +       csrw    CSR_MTVEC, t0
> +
> +       li      t0, 0
> +       li      t3, SEED_OPTS_ES16
> +       li      t4, SEED_ENTROPY_MASK
> +       li      t5, __SIZEOF_POINTER__
> +2:
> +       csrrw   t1, CSR_SEED, x0
> +       li      t2, SEED_OPTS_MASK
> +       and     t2, t2, t1
> +       bgtu    t2, t3, 1f
> +       bltu    t2, t3, 2b
> +       and     t1, t1, t4
> +       slli    t0, t0, 16
> +       or      t0, t0, t1
> +       addi    t5, t5, -2
> +       bgtz    t5, 2b
> +       lla     t1, __stack_chk_guard
> +       REG_S   t0, 0(t1)
> +       j       1f
> +       .align 3
> +1:
> +       csrw    CSR_MTVEC, t6
> +       ret
> +

Initializing __stack_chk_guard from C code may break stack
canary already pushed on stack hence it is better to initialize
__stack_chk_guard in low-level code before entering C code.

Instead of the approach taken by this patch, my suggestion
is to set mnstatus.NMIE early on so that mtvec based trap
behave normally and setup NMI trap handler later in the
fw_platform_init() or nascent_init().

Something like below, can help ...

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index 043de7d7..21d59e71 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -39,6 +39,16 @@
     li     \tmp, MSTATUS_MDT
     csrc    CSR_MSTATUS, \tmp
 #endif
+.endm
+
+.macro PARTIALLY_ENABLE_SMRNMI tmp
+    lla    \tmp, 9999f
+    csrw    CSR_MTVEC, \tmp
+    li    \tmp, MNSTATUS_NMIE
+    csrs    CSR_MNSTATUS, MNSTATUS_NMIE
+    j    9999f
+    .align 3
+9999:
 .endm

     .section .entry, "ax", %progbits
@@ -107,6 +117,12 @@ _bss_zero:
     add    s4, s4, __SIZEOF_POINTER__
     blt    s4, s5, _bss_zero

+    /*
+     * Partially enable Smrnmi early for boot HART so that
+     * normal traps get routed to mtvec trap handler.
+     */
+    PARTIALLY_ENABLE_SMRNMI t0
+
     /* Trying to initialize the stack guard via the Zkr extension */
     lla    t0, __stack_chk_guard_done
     csrw    CSR_MTVEC, t0
@@ -339,6 +355,12 @@ _wait_for_boot_hart:
     bne    t0, t1, _wait_for_boot_hart

 _start_warm:
+    /*
+     * Partially enable Smrnmi early for current HART so that
+     * normal traps get routed to mtvec trap handler.
+     */
+    PARTIALLY_ENABLE_SMRNMI t0
+
     /* Reset all registers except ra, a0, a1, a2, a3 and a4 for
non-boot HART */
     li    ra, 0
     call    _reset_regs

Regards,
Anup

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