Re: [PATCH v2] target/riscv: preserve vl on element-zero fault

Thanks Chao.

I have sent v3 to rebase onto current master and regenerate the malformed
v2 submission. There are no functional changes from v2, so Chao's
Reviewed-by still applies.

Thanks,
Zephyr

On Wed, Aug 12, 2026 at 10:52 AM Chao Liu <[email protected]>
wrote:

> On Wed, Aug 12, 2026 at 05:41:00AM +0800, Alistair wrote:
> > On Mon, 2026-07-27 at 16:42 +0800, Zephyr Li wrote:
> > > A unit-stride fault-only-first load must leave vl unchanged when
> > > element
> > > zero raises a synchronous exception. Keep the shortened value in a
> > > local
> > > bound until all loads complete, and only then update the
> > > architectural vl.
> > >
> > > Add a bare-metal RVV regression test which faults vle32ff.v on
> > > element zero
> > > and checks that vl remains unchanged in the trap handler.
> > >
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3544
> > > Reviewed-by: Chao Liu <[email protected]>
> >
> > Was this Reviewed?
> I’ve checked v2. The test improvements look good to me and the
> fix is unchanged.
>
> Thanks,
> Chao
> >
> > Alistair
> >
> > > Signed-off-by: Zephyr Li <[email protected]>
> > > ---
> > > Changes in v2:
> > > - Fix commit message formatting.
> > > - Enable vector state in the bare-metal test.
> > > - Use an aligned unmapped address and compare against the vsetvli
> > > result.
> > > - Express the mstatus.VS setting as a field shift.
> > >
> > >  target/riscv/tcg/vector_helper.c          | 23 +++++----
> > >  tests/tcg/riscv64/Makefile.softmmu-target |  5 ++
> > >  tests/tcg/riscv64/test-vle32ff.S          | 57
> > > +++++++++++++++++++++++
> > >  3 files changed, 76 insertions(+), 9 deletions(-)
> > >  create mode 100644 tests/tcg/riscv64/test-vle32ff.S
> > >
> > > diff --git a/target/riscv/tcg/vector_helper.c
> > > b/target/riscv/tcg/vector_helper.c
> > > index e321ca2616..5a310822b7 100644
> > > --- a/target/riscv/tcg/vector_helper.c
> > > +++ b/target/riscv/tcg/vector_helper.c
> > > @@ -686,7 +686,7 @@ vext_ldff(void *vd, void *v0, target_ulong base,
> > > CPURISCVState *env,
> > >            uint32_t desc, vext_ldst_elem_fn_tlb *ldst_tlb,
> > >            vext_ldst_elem_fn_host *ldst_host, uint32_t log2_esz,
> > > uintptr_t ra)
> > >  {
> > > -    uint32_t i, k, vl = 0;
> > > +    uint32_t i, k, vl = 0, load_vl;
> > >      uint32_t nf = vext_nf(desc);
> > >      uint32_t vm = vext_vm(desc);
> > >      uint32_t max_elems = vext_max_elems(desc, log2_esz);
> > > @@ -752,22 +752,24 @@ vext_ldff(void *vd, void *v0, target_ulong
> > > base, CPURISCVState *env,
> > >          }
> > >      }
> > >  ProbeSuccess:
> > > -    /* load bytes from guest memory */
> > > -    if (vl != 0) {
> > > -        env->vl = vl;
> > > -    }
> > > +    /*
> > > +     * Keep a shortened vl local until all loads complete. In
> > > particular,
> > > +     * an exception from element zero must leave the architectural
> > > vl alone.
> > > +     */
> > > +    load_vl = vl ? vl : env->vl;
> > >
> > > -    if (env->vstart < env->vl) {
> > > +    if (env->vstart < load_vl) {
> > >          if (vm) {
> > >              /* Load/store elements in the first page */
> > >              if (likely(elems)) {
> > > +                elems = MIN(elems, load_vl - env->vstart);
> > >                  vext_page_ldst_us(env, vd, addr, elems, nf,
> > > max_elems,
> > >                                    log2_esz, true, mmu_index,
> > > ldst_tlb,
> > >                                    ldst_host, ra);
> > >              }
> > >
> > >              /* Load/store elements in the second page */
> > > -            if (unlikely(env->vstart < env->vl)) {
> > > +            if (unlikely(env->vstart < load_vl)) {
> > >                  /* Cross page element */
> > >                  if (unlikely(page_split % msize)) {
> > >                      for (k = 0; k < nf; k++) {
> > > @@ -780,7 +782,7 @@ ProbeSuccess:
> > >
> > >                  addr = base + ((env->vstart * nf) << log2_esz);
> > >                  /* Get number of elements of second page */
> > > -                elems = env->vl - env->vstart;
> > > +                elems = load_vl - env->vstart;
> > >
> > >                  /* Load/store elements in the second page */
> > >                  vext_page_ldst_us(env, vd, addr, elems, nf,
> > > max_elems,
> > > @@ -788,7 +790,7 @@ ProbeSuccess:
> > >                                    ldst_host, ra);
> > >              }
> > >          } else {
> > > -            for (i = env->vstart; i < env->vl; i++) {
> > > +            for (i = env->vstart; i < load_vl; i++) {
> > >                  k = 0;
> > >                  while (k < nf) {
> > >                      if (!vext_elem_mask(v0, i)) {
> > > @@ -806,6 +808,9 @@ ProbeSuccess:
> > >              }
> > >          }
> > >      }
> > > +    if (vl != 0) {
> > > +        env->vl = vl;
> > > +    }
> > >      env->vstart = 0;
> > >
> > >      vext_set_tail_elems_1s(env->vl, vd, desc, nf, esz, max_elems);
> > > diff --git a/tests/tcg/riscv64/Makefile.softmmu-target
> > > b/tests/tcg/riscv64/Makefile.softmmu-target
> > > index 82be8a2c91..7ae225bd73 100644
> > > --- a/tests/tcg/riscv64/Makefile.softmmu-target
> > > +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> > > @@ -41,5 +41,10 @@ comma:= ,
> > >  run-test-crc32: test-crc32
> > >     $(call run-test, $<, $(QEMU) -cpu rv64$(comma)xlrbr=true
> > > $(QEMU_OPTS)$<)
> > >
> > > +EXTRA_RUNS += run-test-vle32ff
> > > +run-test-vle32ff: test-vle32ff
> > > +   $(call run-test, $<, $(QEMU) -cpu rv64$(comma)v=true
> > > $(QEMU_OPTS)$<)
> > > +test-vle32ff: CFLAGS += -march=rv64gcv
> > > +
> > >  # We don't currently support the multiarch system tests
> > >  undefine MULTIARCH_TESTS
> > > diff --git a/tests/tcg/riscv64/test-vle32ff.S
> > > b/tests/tcg/riscv64/test-vle32ff.S
> > > new file mode 100644
> > > index 0000000000..f510960c05
> > > --- /dev/null
> > > +++ b/tests/tcg/riscv64/test-vle32ff.S
> > > @@ -0,0 +1,57 @@
> > > +/*
> > > + * Verify that a fault-only-first load keeps vl unchanged when
> > > element zero
> > > + * raises a synchronous exception.
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + */
> > > +
> > > +   .option norvc
> > > +
> > > +   .text
> > > +   .globl _start
> > > +_start:
> > > +   lla     t0, trap
> > > +   csrw    mtvec, t0
> > > +
> > > +   li      t0, (1 << 9)    /* Set mstatus.VS to Initial. */
> > > +   csrs    mstatus, t0
> > > +   li      t1, 4
> > > +   vsetvli t2, t1, e32, m1, ta, ma
> > > +   li      t0, 0x18000000  /* Unmapped gap in the virt memory
> > > map. */
> > > +   vle32ff.v       v1, (t0)
> > > +
> > > +   /* Element zero did not trap. */
> > > +   li      a0, 1
> > > +   j       _exit
> > > +
> > > +trap:
> > > +   csrr    t0, mcause
> > > +   li      t1, 5           /* Load access fault. */
> > > +   bne     t0, t1, trap_fail
> > > +
> > > +   csrr    t0, vl
> > > +   bne     t0, t2, trap_fail
> > > +
> > > +   li      a0, 0
> > > +   j       _exit
> > > +
> > > +trap_fail:
> > > +   li      a0, 1
> > > +
> > > +_exit:
> > > +   lla     a1, semiargs
> > > +   li      t0, 0x20026     /* ADP_Stopped_ApplicationExit */
> > > +   sd      t0, 0(a1)
> > > +   sd      a0, 8(a1)
> > > +   li      a0, 0x20        /* TARGET_SYS_EXIT_EXTENDED */
> > > +
> > > +   .balign 16
> > > +   slli    zero, zero, 0x1f
> > > +   ebreak
> > > +   srai    zero, zero, 0x7
> > > +   j       .
> > > +
> > > +   .data
> > > +   .balign 8
> > > +semiargs:
> > > +   .space  16
>