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]> --- 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 -- 2.43.0
