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

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