From: Zephyr Li <[email protected]> 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]> Acked-by: Alistair Francis <[email protected]> Message-ID: <[email protected]> Signed-off-by: Alistair Francis <[email protected]> --- 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 e28d8a3d9f..0d1e4e0964 100644 --- a/target/riscv/tcg/vector_helper.c +++ b/target/riscv/tcg/vector_helper.c @@ -721,7 +721,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); @@ -787,22 +787,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++) { @@ -815,7 +817,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, @@ -823,7 +825,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)) { @@ -841,6 +843,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 575571de90..f2c75abd57 100644 --- a/tests/tcg/riscv64/Makefile.softmmu-target +++ b/tests/tcg/riscv64/Makefile.softmmu-target @@ -80,5 +80,10 @@ EXTRA_RUNS += run-test-rv128-sstatus run-test-rv128-sstatus: test-rv128-sstatus $(call run-test, $<, $(QEMU) -cpu x-rv128 $(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.55.0
