Re: [PATCH v2 0/4] Fixes for vector misaligned load/store handlers

On Tue, Jun 9, 2026 at 6:49 PM Bo Gan <[email protected]> wrote:
>
> Hi Anirudh,
>
> On 6/9/26 15:02, Anirudh Srinivasan wrote:
> > Hi Bo,
> >
> > On Tue, Jun 9, 2026 at 1:02 AM Bo Gan <[email protected]> wrote:
> >>
> >> Re-visit the vector misaligned load/store handlers and fix:
> >>
> >> a. Avoid stack overflow by using a small sliding mask[] buffer,
> >>     thus optimizes stack usage *IMPORTANT* (correctness). There's no-
> >>     longer a need to have a pre-defined vlen maximum, and worry about
> >>     whether the stack can hold the mask[] variable.
> >>
> >> b. Maintain the value of vstart when redirecting uptrap. (optmization)
> >>     Avoids doing duplicate work when the instruction gets restarted.
> >>
> >> c. Explicitly set VS dirty in (V)SSTATUS. (correctness), VS in
> >>     VSSTATUS must be set dirty if coming from V=1.
> >>
> >> d. Zero out tinst in uptrap if not guest-page fault (correctness).
> >>
> >> This is a follow up patch to [1] and should be applied on top.
> >> [1] https://lore.kernel.org/opensbi/CAEev2e_Vg1mMP4mOKanFX_EGeUzpczRcWW++vBBuN1CfyM0BJw@mail.gmail.com/T/#t
> >> ---
> >> v2: Fix the wrong PATCH 4/4 generated in v1.
> >
> > Testing on Tenstorrent Blackhole with Sifive X280 cores.
> >
> > After adding some logging like this, I'm still able to break the boot
> > (like I'd reported on your previous patch). Full logs here
> > https://gist.github.com/asrinivasanTT/120646cbb7194e7b3505428ebefbdb30
> >
> > diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> > index 0f29dcf9..0f73c339 100644
> > --- a/lib/sbi/sbi_trap_v_ldst.c
> > +++ b/lib/sbi/sbi_trap_v_ldst.c
> > @@ -17,6 +17,7 @@
> >   #include <sbi/sbi_trap.h>
> >   #include <sbi/sbi_unpriv.h>
> >   #include <sbi/sbi_vector.h>
> > +#include <sbi/sbi_console.h>
> >
> >   #ifdef OPENSBI_CC_SUPPORT_VECTOR
> >
> > @@ -163,6 +164,8 @@ static inline void
> > sbi_misaligned_v_tinst_fixup(struct sbi_trap_info *uptrap)
> >
> >   int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> >   {
> > +       sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > +                  __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> >          struct sbi_trap_regs *regs = &tcntx->regs;
> >          struct sbi_trap_info uptrap;
> >          ulong vl = csr_read(CSR_VL);
> > @@ -276,6 +279,8 @@ done:
> >
> >   int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> >   {
> > +       sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > +               __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> >          struct sbi_trap_regs *regs = &tcntx->regs;
> >          struct sbi_trap_info uptrap;
> >          ulong vl = csr_read(CSR_VL);
> >
> > Is this expected? Are the logging prints causing the overflow?
> >
> > If I don't have this logging prints added, I'm able to boot fine into
> > linux. I was able to do this with your last patch, so this patch
> > doesn't change much in that aspect.
> >
>
> As discussed in the IRC DM, it's most likely a Linux race condition bug.
> Adding the prints delays the probe function significantly, and the page
> containing the function, marked as __init, can be concurrently unmapped
> and free'd, thus triggering instruction page fault, cause=0xc. Can you
> confirm that removing __init (as a hack) fixes the problem? Looks like
> you've tried it already and the issue seems to be gone. Can you confirm?

Yup, removing __init fixes it. It's a bug in linux.

Tested-by: Anirudh Srinivasan <[email protected]>


On Tue, Jun 9, 2026 at 6:49 PM Bo Gan <[email protected]> wrote:
>
> Hi Anirudh,
>
> On 6/9/26 15:02, Anirudh Srinivasan wrote:
> > Hi Bo,
> >
> > On Tue, Jun 9, 2026 at 1:02 AM Bo Gan <[email protected]> wrote:
> >>
> >> Re-visit the vector misaligned load/store handlers and fix:
> >>
> >> a. Avoid stack overflow by using a small sliding mask[] buffer,
> >>     thus optimizes stack usage *IMPORTANT* (correctness). There's no-
> >>     longer a need to have a pre-defined vlen maximum, and worry about
> >>     whether the stack can hold the mask[] variable.
> >>
> >> b. Maintain the value of vstart when redirecting uptrap. (optmization)
> >>     Avoids doing duplicate work when the instruction gets restarted.
> >>
> >> c. Explicitly set VS dirty in (V)SSTATUS. (correctness), VS in
> >>     VSSTATUS must be set dirty if coming from V=1.
> >>
> >> d. Zero out tinst in uptrap if not guest-page fault (correctness).
> >>
> >> This is a follow up patch to [1] and should be applied on top.
> >> [1] https://lore.kernel.org/opensbi/CAEev2e_Vg1mMP4mOKanFX_EGeUzpczRcWW++vBBuN1CfyM0BJw@mail.gmail.com/T/#t
> >> ---
> >> v2: Fix the wrong PATCH 4/4 generated in v1.
> >
> > Testing on Tenstorrent Blackhole with Sifive X280 cores.
> >
> > After adding some logging like this, I'm still able to break the boot
> > (like I'd reported on your previous patch). Full logs here
> > https://gist.github.com/asrinivasanTT/120646cbb7194e7b3505428ebefbdb30
> >
> > diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> > index 0f29dcf9..0f73c339 100644
> > --- a/lib/sbi/sbi_trap_v_ldst.c
> > +++ b/lib/sbi/sbi_trap_v_ldst.c
> > @@ -17,6 +17,7 @@
> >   #include <sbi/sbi_trap.h>
> >   #include <sbi/sbi_unpriv.h>
> >   #include <sbi/sbi_vector.h>
> > +#include <sbi/sbi_console.h>
> >
> >   #ifdef OPENSBI_CC_SUPPORT_VECTOR
> >
> > @@ -163,6 +164,8 @@ static inline void
> > sbi_misaligned_v_tinst_fixup(struct sbi_trap_info *uptrap)
> >
> >   int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> >   {
> > +       sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > +                  __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> >          struct sbi_trap_regs *regs = &tcntx->regs;
> >          struct sbi_trap_info uptrap;
> >          ulong vl = csr_read(CSR_VL);
> > @@ -276,6 +279,8 @@ done:
> >
> >   int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> >   {
> > +       sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > +               __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> >          struct sbi_trap_regs *regs = &tcntx->regs;
> >          struct sbi_trap_info uptrap;
> >          ulong vl = csr_read(CSR_VL);
> >
> > Is this expected? Are the logging prints causing the overflow?
> >
> > If I don't have this logging prints added, I'm able to boot fine into
> > linux. I was able to do this with your last patch, so this patch
> > doesn't change much in that aspect.
> >
>
> As discussed in the IRC DM, it's most likely a Linux race condition bug.
> Adding the prints delays the probe function significantly, and the page
> containing the function, marked as __init, can be concurrently unmapped
> and free'd, thus triggering instruction page fault, cause=0xc. Can you
> confirm that removing __init (as a hack) fixes the problem? Looks like
> you've tried it already and the issue seems to be gone. Can you confirm?
>
> It definitely warrants a proper fix to upstream linux. It can't rely on
> timing to avoid a crash like this.
>
> >>
> >> ---
> >> Bo Gan (4):
> >>    lib: sbi: cosmetic changes to reduce indentation
> >>    lib: sbi: Rework and split sbi_misaligned(_v)_tinst_fixup
> >>    lib: sbi: Add variable-length unprivilege access functions
> >>    lib: sbi: Rework misaligned vector load/store
> >>
> >>   include/sbi/sbi_trap_ldst.h |   3 -
> >>   include/sbi/sbi_unpriv.h    |  16 +++
> >>   include/sbi/sbi_vector.h    |   6 ++
> >>   lib/sbi/sbi_trap_ldst.c     |  66 +++++++-----
> >>   lib/sbi/sbi_trap_v_ldst.c   | 201 ++++++++++++++++++++++--------------
> >>   lib/sbi/sbi_unpriv.c        |  88 ++++++++++++++--
> >>   6 files changed, 270 insertions(+), 110 deletions(-)
> >>
> >> --
> >> 2.34.1
> >>
>
> Bo
>

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