[PATCH v2 0/4] Fixes for vector misaligned load/store handlers
Anirudh Srinivasan
asrinivasan at oss.tenstorrent.com
Tue Jun 9 16:59:36 PDT 2026
On Tue, Jun 9, 2026 at 6:49 PM Bo Gan <ganboing at gmail.com> 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 <ganboing at gmail.com> 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 <asrinivasan at oss.tenstorrent.com>
On Tue, Jun 9, 2026 at 6:49 PM Bo Gan <ganboing at gmail.com> 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 <ganboing at gmail.com> 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
>
More information about the opensbi
mailing list