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

On Thu, May 7, 2026 at 11:38 PM Evgeny Voevodin
<[email protected]> 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 <[email protected]>

LGTM.

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

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
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi