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

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