Re: [PATCH 0/2] lib: sbi: Fix Smrnmi/RNMI trap handling issues

From: Evgeny Voevodin <evvoevod@tenstorrent.com>
To: opensbi@lists.infradead.org
Cc: Evgeny Voevodin <evvoevod@tenstorrent.com>,
	Nia Su <nia.su@sifive.com>, Nylon Chen <nylon.chen@sifive.com>,
	Anup Patel <anup@brainfault.org>, Nick Hu <nick.hu@sifive.com>
Subject: Re: [PATCH 0/2] lib: sbi: Fix Smrnmi/RNMI trap handling issues
Date: Mon, 31 Aug 2026 17:17:49 +0000	[thread overview]
Message-ID: <20260831171751.3206696-1-evvoevod@tenstorrent.com> (raw)
In-Reply-To: <20260827-rnmi-trap-fixes-v1-0-df371cf92301@sifive.com>

I tried this patch set on artificial setup on Whisper SW system simulator and it
seems to solve issues discussed in [1].

But I'd suggest to split this series to separate changes structurally and make
backporting easier if needed:

  1. Restore the Smrnmi context on non-retentive resume: extract
     sbi_smrnmi_hart_init() and call it from sbi_hart_reinit(). This is the
     actual fix and it is self contained, so anyone on a v1.9 based tree can
     pick just this one.

  2. The sbi_trap.c fix for the clobbered trap context (your patch 2/2). It
     touches a different file and has its own Fixes tag, so it does not
     depend on the rest.

  3. Change smrnmi_handlers_init from void to int, but keep the panic when
     the platform does not provide the callback. Today a callback which
     fails has no way to report it and NMIE is set anyway, so this is a fix
     on its own.

  4. Allow the NULL callback. This one changes what a platform is allowed to
     skip, so I would prefer to discuss it separately: as was mentioned
     earlier in the previous email thread [2], it would be nice to have a
     level of control of a situation when platform forgot to initialize the
     callback rather than just silently set NMIE and proceed. I can see these
     distinct options to handle this:

  a) Boot time output: based on a flag just print whether Smrnmi vectors are
     initialized by platform explicitly or not

         Smrnmi Vectors       : NULL     <-- or "Platform"

  b) Warning print when Smrnmi is present and no callback is registered

  c) Explicit opt out: platform which doesn't need to program vectors
     explicitly sets a corresponding flag in sbi_platform to indicate this.
     Without this flag set and when handlers are missing we panic like now,
     and when the flag is set we go ahead and proceed to setting the NMIE bit.

   I personally would go with a) and c) as this allows full control of what's
   going on and catches errors early. What do you think?

[1] https://lore.kernel.org/opensbi/CAHh=Yk_Sbd1MEH8tVshAOFQptEVKm0QZv+D7bJwMC80qvarVew@mail.gmail.com/
[2] https://lore.kernel.org/opensbi/DIIOKVJ7SVHG.115EOM7LY36I1@tenstorrent.com/

  Thanks,
  Evgeny

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