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

On Fri, Jul 24, 2026 at 11:01:45AM +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.
Please wrap the commit message at 76 columns or fewer.

see:
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message


Reviewed-by: Chao Liu <[email protected]>

Thanks,
Chao
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3544
> Signed-off-by: Zephyr Li <[email protected]>
> ---
>  target/riscv/tcg/vector_helper.c          | 23 ++++++----
>  tests/tcg/riscv64/Makefile.softmmu-target |  5 ++
>  tests/tcg/riscv64/test-vle32ff.S          | 56 +++++++++++++++++++++++
>  3 files changed, 75 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..4c687bf1e4
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-vle32ff.S
> @@ -0,0 +1,56 @@
> +/*
> + * 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	t1, 4
> +	vsetvli	t2, t1, e32, m1, ta, ma
> +	li	t0, 1
> +	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
> +	li	t1, 4
> +	bne	t0, t1, 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