[PATCH 2/3] lib: sbi: Enable Smrnmi extension handler
Nicholas Piggin
npiggin at gmail.com
Mon Mar 9 21:53:11 PDT 2026
On Tue, Mar 10, 2026 at 09:42:26AM +0530, Anup Patel wrote:
> On Tue, Mar 10, 2026 at 7:58 AM Nicholas Piggin <npiggin at gmail.com> wrote:
> >
> > On Mon, Feb 02, 2026 at 03:26:33PM +0000, Radim Krcmar wrote:
> > > 2026-01-29T00:13:42-08:00, Nylon Chen <nylon.chen at sifive.com>:
> > > > Enable the Smrnmi extension by:
> > > > 1. Adding sbi_rnmi_vector_init() to allocate per-HART RNMI context and enable RNMI via MNSTATUS.NMIE
> > > > 2. Saving and restoring CSR_MNSCRATCH and CSR_MNSTATUS across non-retentive suspend/resume
> > > > 3. Calling sbi_rnmi_vector_init() from the cold and warm init paths
> > > >
> > > > Co-developed-by: Zong Li <zong.li at sifive.com>
> > > > Signed-off-by: Zong Li <zong.li at sifive.com>
> > > > Suggested-by: Nick Hu <nick.hu at sifive.com>
> > > > Suggested-by: Samuel Holland <samuel.holland at sifive.com>
> > > > Signed-off-by: Nylon Chen <nylon.chen at sifive.com>
> > > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang at sifive.com>
> > > > ---
> > > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > > > @@ -149,6 +149,15 @@ struct sbi_platform_operations {
> > > > unsigned long log2len);
> > > > /** platform specific pmp disable on current HART */
> > > > void (*pmp_disable)(unsigned int n);
> > > > + /**
> > > > + * Platform-specific helper to program the RNMI trap vector.
> > > > + *
> > > > + * The core will call this from sbi_rnmi_vector_init() with the
> > > > + * firmware RNMI handler entry address (rnmi_handler) and the
> > > > + * cold_boot flag. Platforms that support Smrnmi should implement
> > > > + * this; others can leave it NULL.
> > > > + */
> > >
> > > If the hart supports Smrnmi, then it begins with nmstatus.NMIE=0, which
> > > means that any exception is going to enter a critical state.
> > > The platform must somehow support Smrnmi to be useful, and I think it
> > > would be better not to do it behind opensbi's back.
> > >
> > > If the platform wants to handle Smrnmi itself, it can hide Smrnmi from
> > > opensbi, so I think the requirement here would be better as MUST.
> > >
> > > > + int (*set_rnmi_trap_vector)(uintptr_t handler_addr, bool cold_boot);
> > > > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > > > @@ -273,6 +273,10 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
> > > > if (rc)
> > > > sbi_hart_hang();
> > > >
> > > > + rc = sbi_rnmi_vector_init(scratch, true);
> > >
> > > Are we sure that earlier opensbi code won't try to trigger a trap before
> > > this point? OpenSBI would fail to boot if it did...
> >
> > This appears to come right after sbi_hart_init() which does a bunch of
> > CSR detection using traps, so I'm not sure if it can work.
>
> Why not ?
I should put a better way, I think it's workable for some but not
all implementations. When NMIE=0 the spec says normal exceptions go to
an implementation defined trap handler. This may be mtvec, but not
necessarily. In the case of Ascalon it is not. I guess the SiFive
implementation must use mtvec hence it does work there.
> > Evgeny (+cc) has been doing a lot of work to enable Smrnmi on Ascalon
> > and unfortunately we did not see there is this duplicate work happening
> > upstream. His changes also has a few other things such as RNMI exception
> > vector setting, permits a platform specific handler, and has an Ascalon
> > platform implementation.
>
> This is exactly why upstreaming should be prioritized so that people
> can leverage each others work.
Yes I agree. We should be doing a better with upstream participation.
I will let Evgeny post his series. Again, please don't take it as
ignoring this existing work upstream. And I think we would be
happy to rebase our stuff on top of it, if that ends up being the
best approach. The low level trap handling and platform vector set
call stuff should all be very similar.
Thanks,
Nick
More information about the opensbi
mailing list