On Thu, 2026-08-13 at 10:59 +0800, Zephyr Li wrote:
> A unit-stride fault-only-first load must leave vl unchanged when
> element
> zero raises a synchronous exception.
>
> The current implementation updates vl after probing the accesses but
> before performing the actual loads. For unassigned MMIO, the probe
> can
> succeed even though the subsequent device transaction raises an
> exception. If this happens for element zero, the trap handler
> observes
> a prematurely shortened vl.
>
> 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 using an unassigned MMIO address
> and check that vl remains unchanged in the trap handler.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3544
> Reviewed-by: Chao Liu <[email protected]>
> Reviewed-by: Max Chou <[email protected]>
> Signed-off-by: Zephyr Li <[email protected]>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> Changes in v4:
> - Clarify that the test uses unassigned MMIO, not an unmapped
> address.
> - Explain why the MMIO transaction fault is needed to reproduce the
> bug.
>
> Changes in v3:
> - Rebase onto current master.
> - Regenerate the patch to fix the malformed v2 submission.
>
> Changes in v2:
> - Fix commit message formatting.
> - Enable vector state in the bare-metal test.
> - Use an aligned unassigned MMIO 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 | 58
> +++++++++++++++++++++++
> 3 files changed, 77 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..2cffe8659c
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-vle32ff.S
> @@ -0,0 +1,58 @@
> +/*
> + * 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
> + /* Unassigned MMIO: probe succeeds, load faults. */
> + li t0, 0x18000000
> + 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
