* [PATCH v3 0/3] target/riscv: corner case fixes
@ 2026-03-21 14:45 Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-21 14:45 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
Changes:
v3:
* Added vloxei8.v to overflow test.
* Added store variants of interrupted vector ops tests.
v2:
* Added a tcg tests build-time check for vector intrinsics support
in target compiler before building new tests that require it.
ci images may not support these yet unfortunately, but upgrading
those will be a separate effort.
Thanks,
Nick
Nicholas Piggin (3):
target/riscv: Fix IALIGN check in misa write
target/riscv: Fix vector whole ldst vstart check
tests/tcg: Add riscv test for interrupted vector ops
target/riscv/csr.c | 16 +-
target/riscv/vector_helper.c | 2 +
tests/tcg/riscv64/Makefile.softmmu-target | 5 +
tests/tcg/riscv64/Makefile.target | 16 ++
tests/tcg/riscv64/misa-ialign.S | 88 ++++++
tests/tcg/riscv64/test-interrupted-v.c | 329 ++++++++++++++++++++++
tests/tcg/riscv64/test-vstart-overflow.c | 78 +++++
7 files changed, 531 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/riscv64/misa-ialign.S
create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c
--
2.51.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-21 14:45 [PATCH v3 0/3] target/riscv: corner case fixes Nicholas Piggin
@ 2026-03-21 14:45 ` Nicholas Piggin
2026-03-25 1:35 ` Alistair Francis
2026-03-25 3:08 ` Chao Liu
2026-03-21 14:45 ` [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-21 14:45 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
The instruction alignment check for the C extension was inverted.
The new value should be checked for C bit clear (thus increasing
IALIGN). If IALIGN is incompatible, then the write to misa should
be suppressed, not just ignoring the update to the C bit.
From the ISA:
Writing misa may increase IALIGN, e.g., by disabling the "C"
extension. If an instruction that would write misa increases IALIGN,
and the subsequent instruction’s address is not IALIGN-bit aligned,
the write to misa is suppressed, leaving misa unchanged.
This was found with a verification test generator based on RiESCUE.
Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/riscv/csr.c | 16 ++++-
tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
3 files changed, 106 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/riscv64/misa-ialign.S
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5064483917..91421a2dd8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
/* Mask extensions that are not supported by this hart */
val &= env->misa_ext_mask;
- /* Suppress 'C' if next instruction is not aligned. */
- if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
- val &= ~RVC;
+ /*
+ * misa writes that increase IALIGN beyond alignment of the next
+ * instruction cause the write to misa to be suppressed. Clearing
+ * "C" extension increases IALIGN.
+ */
+ if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
+ /*
+ * If the next instruction is unaligned mod 4 then "C" must be
+ * set or this instruction could not be executing, so we know
+ * this is is clearing "C" (and not just keeping it clear).
+ */
+ g_assert(env->misa_ext & RVC);
+ return RISCV_EXCP_NONE;
}
/* Disable RVG if any of its dependencies are disabled */
diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
index eb1ce6504a..f176f87ed0 100644
--- a/tests/tcg/riscv64/Makefile.softmmu-target
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
$(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
$(QEMU_OPTS)$<)
+EXTRA_RUNS += run-misa-ialign
+run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
+run-misa-ialign: misa-ialign
+ $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
+
# We don't currently support the multiarch system tests
undefine MULTIARCH_TESTS
diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
new file mode 100644
index 0000000000..7f1eb30023
--- /dev/null
+++ b/tests/tcg/riscv64/misa-ialign.S
@@ -0,0 +1,88 @@
+/*
+ * Test for MISA changing C and related IALIGN alignment cases
+ *
+ * This test verifies that the "C" extension can be cleared and set in MISA,
+ * that a branch to 2-byte aligned instructions can be executed when "C" is
+ * enabled, and that a write to MISA which would increase IALIGN and cause
+ * the next instruction to be unaligned is ignored.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#define RVC (1 << ('C'-'A'))
+#define RVV (1 << ('V'-'A'))
+
+.option norvc
+ .text
+ .global _start
+_start:
+ lla t0, trap
+ csrw mtvec, t0
+
+ csrr t0, misa
+ li t1, RVC
+ not t1, t1
+ and t0, t0, t1
+ csrw misa, t0
+ csrr t1, misa
+ li a0, 2 # fail code
+ bne t0, t1, _exit # Could not clear RVC in MISA
+
+ li t1, RVC
+ or t0, t0, t1
+ csrw misa, t0
+ csrr t1, misa
+ li a0, 3 # fail code
+ bne t0, t1, _exit # Could not set RVC in MISA
+
+ j unalign
+. = . + 2
+unalign:
+
+ li t1, RVC
+ not t1, t1
+ and t0, t0, t1
+ csrw misa, t0
+ csrr t1, misa
+ li a0, 4 # fail code
+ beq t0, t1, _exit # Was able to clear RVC in MISA
+
+ li t0, (RVC|RVV)
+ not t0, t0
+ and t0, t0, t1
+ csrw misa, t0
+ csrr t0, misa
+ li a0, 5 # fail code
+ bne t0, t1, _exit # MISA write was not ignored (RVV was cleared)
+
+ j realign
+. = . + 2
+realign:
+
+ # Success!
+ li a0, 0
+ j _exit
+
+trap:
+ # Any trap is a fail code 1
+ li a0, 1
+
+# Exit code in a0
+_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
+
+ # Semihosting call sequence
+ .balign 16
+ slli zero, zero, 0x1f
+ ebreak
+ srai zero, zero, 0x7
+ j .
+
+ .data
+ .balign 16
+semiargs:
+ .space 16
--
2.51.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check
2026-03-21 14:45 [PATCH v3 0/3] target/riscv: corner case fixes Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
@ 2026-03-21 14:45 ` Nicholas Piggin
2026-03-25 1:57 ` Alistair Francis
2026-03-25 2:10 ` Chao Liu
2026-03-21 14:45 ` [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops Nicholas Piggin
2026-03-25 2:20 ` [PATCH v3 0/3] target/riscv: corner case fixes Alistair Francis
3 siblings, 2 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-21 14:45 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
The whole vector ldst instructions do not include a vstart check, so an
overflowed vstart can result in an underflowed memory address offset and
crash:
accel/tcg/cputlb.c:1465:probe_access_flags:
assertion failed: (-(addr | TARGET_PAGE_MASK) >= size)
Add the VSTART_CHECK_EARLY_EXIT() check for these helpers.
This was found with a verification test generator based on RiESCUE.
Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/riscv/vector_helper.c | 2 +
tests/tcg/riscv64/Makefile.target | 5 ++
tests/tcg/riscv64/test-vstart-overflow.c | 78 ++++++++++++++++++++++++
3 files changed, 85 insertions(+)
create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index caa8dd9c12..4126447d11 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -825,6 +825,8 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
uint32_t esz = 1 << log2_esz;
int mmu_index = riscv_env_mmu_index(env, false);
+ VSTART_CHECK_EARLY_EXIT(env, evl);
+
/* Calculate the page range of first page */
addr = base + (env->vstart << log2_esz);
page_split = -(addr | TARGET_PAGE_MASK);
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index 4da5b9a3b3..19a49b6467 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -18,3 +18,8 @@ TESTS += test-fcvtmod
test-fcvtmod: CFLAGS += -march=rv64imafdc
test-fcvtmod: LDFLAGS += -static
run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
+
+# Test for vstart >= vl
+TESTS += test-vstart-overflow
+test-vstart-overflow: CFLAGS += -march=rv64gcv
+run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
diff --git a/tests/tcg/riscv64/test-vstart-overflow.c b/tests/tcg/riscv64/test-vstart-overflow.c
new file mode 100644
index 0000000000..6c904ab309
--- /dev/null
+++ b/tests/tcg/riscv64/test-vstart-overflow.c
@@ -0,0 +1,78 @@
+/*
+ * Test for VSTART set to overflow VL
+ *
+ * TCG vector instructions should call VSTART_CHECK_EARLY_EXIT() to check
+ * this case, otherwise memory addresses can underflow and misbehave or
+ * crash QEMU.
+ *
+ * TODO: Add stores and other instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdint.h>
+
+#define VSTART_OVERFLOW_TEST(insn) \
+({ \
+ uint8_t vmem[64] = { 0 }; \
+ uint64_t vstart; \
+ asm volatile(" \r\n \
+ # Set VL=52 and VSTART=56 \r\n \
+ li t0, 52 \r\n \
+ vsetvli x0, t0, e8, m4, ta, ma \r\n \
+ li t0, 56 \r\n \
+ csrrw x0, vstart, t0 \r\n \
+ li t1, 64 \r\n \
+ " insn " \r\n \
+ csrr %0, vstart \r\n \
+ " : "=r"(vstart), "+A"(vmem) :: "t0", "t1", "v24", "memory"); \
+ vstart; \
+})
+
+int run_vstart_overflow_tests()
+{
+ /*
+ * An implementation is permitted to raise an illegal instruction
+ * exception when executing a vector instruction if vstart is set to a
+ * value that could not be produced by the execution of that instruction
+ * with the same vtype. If TCG is changed to do this, then this test
+ * could be updated to handle the SIGILL.
+ */
+ if (VSTART_OVERFLOW_TEST("vl1re16.v v24, %1")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vs1r.v v24, %1")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vle16.v v24, %1")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vse16.v v24, %1")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vluxei8.v v24, %1, v20")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vloxei8.v v24, %1, v20")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vlse16.v v24, %1, t1")) {
+ return 1;
+ }
+
+ if (VSTART_OVERFLOW_TEST("vlseg2e8.v v24, %1")) {
+ return 1;
+ }
+
+ return 0;
+}
+
+int main()
+{
+ return run_vstart_overflow_tests();
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops
2026-03-21 14:45 [PATCH v3 0/3] target/riscv: corner case fixes Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check Nicholas Piggin
@ 2026-03-21 14:45 ` Nicholas Piggin
2026-03-25 2:08 ` Alistair Francis
2026-03-25 3:19 ` Chao Liu
2026-03-25 2:20 ` [PATCH v3 0/3] target/riscv: corner case fixes Alistair Francis
3 siblings, 2 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-21 14:45 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
riscv vector instructions can be interrupted with a trap, and partial
completion is recorded in the vstart register. Some causes are
implementation dependent, for example an asynchronous interrupt (which I
don't think TCG allows). Others are architectural, typically memory
access faults on vector load/store instructions.
Add some TCG tests for interrupting vector load instructions and
resuming partially completed ones.
This would have caught a recent (now reverted) regression in vector
stride load implementation, commit 28c12c1f2f50d ("Generate strided
vector loads/stores with tcg nodes.")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/tcg/riscv64/Makefile.target | 11 +
tests/tcg/riscv64/test-interrupted-v.c | 329 +++++++++++++++++++++++++
2 files changed, 340 insertions(+)
create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index 19a49b6467..b2b2325843 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -1,6 +1,10 @@
# -*- Mode: makefile -*-
# RISC-V specific tweaks
+# Not all environments have compilers with vector intrinsics yet.
+HAVE_RISCV_VECTOR_INTRINSICS := $(shell echo '#ifndef __riscv_v_intrinsic\n#error\n#endif' | \
+ $(CC) -march=rv64gcv -E -x c - >/dev/null 2>&1 && echo y)
+
VPATH += $(SRC_PATH)/tests/tcg/riscv64
TESTS += test-div
TESTS += noexec
@@ -23,3 +27,10 @@ run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
TESTS += test-vstart-overflow
test-vstart-overflow: CFLAGS += -march=rv64gcv
run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
+
+ifeq ($(HAVE_RISCV_VECTOR_INTRINSICS),y)
+# Test for interrupted vector instructions
+TESTS += test-interrupted-v
+test-interrupted-v: CFLAGS += -march=rv64gcv
+run-test-interrupted-v: QEMU_OPTS += -cpu rv64,v=on
+endif
diff --git a/tests/tcg/riscv64/test-interrupted-v.c b/tests/tcg/riscv64/test-interrupted-v.c
new file mode 100644
index 0000000000..3d0d21b49b
--- /dev/null
+++ b/tests/tcg/riscv64/test-interrupted-v.c
@@ -0,0 +1,329 @@
+/*
+ * Test for interrupted vector operations.
+ *
+ * Some vector instructions can be interrupted partially complete, vstart will
+ * be set to where the operation has progressed to, and the instruction can be
+ * re-executed with vstart != 0. It is implementation dependent as to what
+ * instructions can be interrupted and what vstart values are permitted when
+ * executing them. Vector memory operations can typically be interrupted
+ * (as they can take page faults), so these are easy to test.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <stdio.h>
+#include <assert.h>
+#include <signal.h>
+#include <unistd.h>
+#include <riscv_vector.h>
+
+static unsigned long page_size;
+
+static volatile int nr_segv;
+static volatile unsigned long fault_start, fault_end;
+static volatile bool fault_write;
+
+/*
+ * Careful: qemu-user does not save/restore vector state in
+ * signals yet, so any library or compiler autovec code will
+ * corrupt our test.
+ *
+ * Do only minimal work in the signal handler.
+ */
+static void SEGV_handler(int signo, siginfo_t *info, void *context)
+{
+ unsigned long page = (unsigned long)info->si_addr &
+ ~(unsigned long)(page_size - 1);
+
+ assert((unsigned long)info->si_addr >= fault_start);
+ assert((unsigned long)info->si_addr < fault_end);
+ if (fault_write) {
+ mprotect((void *)page, page_size, PROT_READ | PROT_WRITE);
+ } else {
+ mprotect((void *)page, page_size, PROT_READ);
+ }
+ nr_segv++;
+}
+
+/* Use noinline to make generated code easier to inspect */
+static __attribute__((noinline))
+uint8_t unit_load(uint8_t *mem, size_t nr, bool ff)
+{
+ size_t vl;
+ vuint8m1_t vec, redvec, sum;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ if (ff) {
+ vec = __riscv_vle8ff_v_u8m1(mem, &vl, vl);
+ } else {
+ vec = __riscv_vle8_v_u8m1(mem, vl);
+ }
+ redvec = __riscv_vmv_v_x_u8m1(0, vl);
+ sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
+ return __riscv_vmv_x_s_u8m1_u8(sum);
+}
+
+static __attribute__((noinline))
+uint8_t seg2_load(uint8_t *mem, size_t nr, bool ff)
+{
+ size_t vl;
+ vuint8m1x2_t segvec;
+ vuint8m1_t vec, redvec, sum;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ if (ff) {
+ segvec = __riscv_vlseg2e8ff_v_u8m1x2(mem, &vl, vl);
+ } else {
+ segvec = __riscv_vlseg2e8_v_u8m1x2(mem, vl);
+ }
+ vec = __riscv_vadd_vv_u8m1(__riscv_vget_v_u8m1x2_u8m1(segvec, 0),
+ __riscv_vget_v_u8m1x2_u8m1(segvec, 1), vl);
+ redvec = __riscv_vmv_v_x_u8m1(0, vl);
+ sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
+ return __riscv_vmv_x_s_u8m1_u8(sum);
+}
+
+static __attribute__((noinline))
+uint8_t strided_load(uint8_t *mem, size_t nr, size_t stride)
+{
+ size_t vl;
+ vuint8m1_t vec, redvec, sum;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ vec = __riscv_vlse8_v_u8m1(mem, stride, vl);
+ redvec = __riscv_vmv_v_x_u8m1(0, vl);
+ sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
+ return __riscv_vmv_x_s_u8m1_u8(sum);
+}
+
+static __attribute__((noinline))
+uint8_t indexed_load(uint8_t *mem, size_t nr, uint32_t *indices)
+{
+ size_t vl;
+ vuint32m4_t idx;
+ vuint8m1_t vec, redvec, sum;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ idx = __riscv_vle32_v_u32m4(indices, vl);
+ vec = __riscv_vloxei32_v_u8m1(mem, idx, vl);
+ redvec = __riscv_vmv_v_x_u8m1(0, vl);
+ sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
+ return __riscv_vmv_x_s_u8m1_u8(sum);
+}
+
+/* New store functions */
+static __attribute__((noinline))
+void unit_store(uint8_t *mem, size_t nr, vuint8m1_t vec)
+{
+ size_t vl;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ __riscv_vse8_v_u8m1(mem, vec, vl);
+}
+
+static __attribute__((noinline))
+void seg2_store(uint8_t *mem, size_t nr, vuint8m1x2_t segvec)
+{
+ size_t vl;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ __riscv_vsseg2e8_v_u8m1x2(mem, segvec, vl);
+}
+
+static __attribute__((noinline))
+void strided_store(uint8_t *mem, size_t nr, size_t stride, vuint8m1_t vec)
+{
+ size_t vl;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ __riscv_vsse8_v_u8m1(mem, stride, vec, vl);
+}
+
+static __attribute__((noinline))
+void indexed_store(uint8_t *mem, size_t nr, uint32_t *indices, vuint8m1_t vec)
+{
+ size_t vl;
+ vuint32m4_t idx;
+
+ vl = __riscv_vsetvl_e8m1(nr);
+ idx = __riscv_vle32_v_u32m4(indices, vl);
+ __riscv_vsoxei32_v_u8m1(mem, idx, vec, vl);
+}
+
+/* Use e8 elements, 128-bit vectors */
+#define NR_ELEMS 16
+
+static int run_interrupted_v_tests(void)
+{
+ struct sigaction act = { 0 };
+ uint8_t *mem;
+ uint32_t indices[NR_ELEMS];
+ int i;
+
+ page_size = sysconf(_SC_PAGESIZE);
+
+ act.sa_flags = SA_SIGINFO;
+ act.sa_sigaction = &SEGV_handler;
+ if (sigaction(SIGSEGV, &act, NULL) == -1) {
+ perror("sigaction");
+ exit(EXIT_FAILURE);
+ }
+
+ mem = mmap(NULL, NR_ELEMS * page_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ assert(mem != MAP_FAILED);
+ madvise(mem, NR_ELEMS * page_size, MADV_NOHUGEPAGE);
+
+ /*** Load tests ***/
+ fault_write = false;
+
+ /* Unit-stride tests load memory crossing a page boundary */
+ memset(mem, 0, NR_ELEMS * page_size);
+ for (i = 0; i < NR_ELEMS; i++) {
+ mem[page_size - NR_ELEMS + i] = 3;
+ }
+ for (i = 0; i < NR_ELEMS; i++) {
+ mem[page_size + i] = 5;
+ }
+
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
+ fault_end = fault_start + NR_ELEMS;
+ mprotect(mem, page_size * 2, PROT_NONE);
+ assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, false)
+ == 8 * NR_ELEMS / 2);
+ assert(nr_segv == 2);
+
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
+ fault_end = fault_start + NR_ELEMS * 2;
+ mprotect(mem, page_size * 2, PROT_NONE);
+ assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS, false)
+ == 8 * NR_ELEMS);
+ assert(nr_segv == 2);
+
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
+ fault_end = fault_start + (NR_ELEMS / 2);
+ mprotect(mem, page_size * 2, PROT_NONE);
+ assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, true)
+ == 3 * NR_ELEMS / 2);
+ assert(nr_segv == 1); /* fault-first does not fault the second page */
+
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
+ fault_end = fault_start + NR_ELEMS;
+ mprotect(mem, page_size * 2, PROT_NONE);
+ assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS * 2, true)
+ == 3 * NR_ELEMS);
+ assert(nr_segv == 1); /* fault-first does not fault the second page */
+
+ /* Following tests load one element from first byte of each page */
+ mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
+ memset(mem, 0, NR_ELEMS * page_size);
+ for (i = 0; i < NR_ELEMS; i++) {
+ mem[i * page_size] = 3;
+ indices[i] = i * page_size;
+ }
+
+ nr_segv = 0;
+ fault_start = (unsigned long)mem;
+ fault_end = fault_start + NR_ELEMS * page_size;
+ mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
+ assert(strided_load(mem, NR_ELEMS, page_size) == 3 * NR_ELEMS);
+ assert(nr_segv == NR_ELEMS);
+
+ nr_segv = 0;
+ fault_start = (unsigned long)mem;
+ fault_end = fault_start + NR_ELEMS * page_size;
+ mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
+ assert(indexed_load(mem, NR_ELEMS, indices) == 3 * NR_ELEMS);
+ assert(nr_segv == NR_ELEMS);
+
+ /*** Store tests ***/
+ fault_write = true;
+
+ uint8_t store_data[NR_ELEMS];
+ uint8_t store_data_seg0[NR_ELEMS];
+ uint8_t store_data_seg1[NR_ELEMS];
+ vuint8m1_t vec;
+ vuint8m1x2_t segvec;
+ size_t vl = __riscv_vsetvl_e8m1(NR_ELEMS);
+
+ /* Create some data to store */
+ for (i = 0; i < NR_ELEMS; i++) {
+ store_data[i] = i * 3;
+ store_data_seg0[i] = i * 5;
+ store_data_seg1[i] = i * 7;
+ }
+ vec = __riscv_vle8_v_u8m1(store_data, vl);
+ segvec = __riscv_vcreate_v_u8m1x2(
+ __riscv_vle8_v_u8m1(store_data_seg0, vl),
+ __riscv_vle8_v_u8m1(store_data_seg1, vl));
+
+ /* Unit-stride store test crossing a page boundary */
+ mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
+ memset(mem, 0, page_size * 2);
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
+ fault_end = fault_start + NR_ELEMS;
+ mprotect(mem, page_size * 2, PROT_NONE);
+ unit_store(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, vec);
+ assert(nr_segv == 2);
+ for (i = 0; i < NR_ELEMS; i++) {
+ assert(mem[page_size - (NR_ELEMS / 2) + i] == store_data[i]);
+ }
+
+ /* Segmented store test crossing a page boundary */
+ mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
+ memset(mem, 0, page_size * 2);
+ nr_segv = 0;
+ fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
+ fault_end = fault_start + NR_ELEMS * 2;
+ mprotect(mem, page_size * 2, PROT_NONE);
+ seg2_store(&mem[page_size - NR_ELEMS], NR_ELEMS, segvec);
+ assert(nr_segv == 2);
+ for (i = 0; i < NR_ELEMS; i++) {
+ assert(mem[page_size - NR_ELEMS + i * 2] == store_data_seg0[i]);
+ assert(mem[page_size - NR_ELEMS + i * 2 + 1] == store_data_seg1[i]);
+ }
+
+ /* Strided store test to one element on each page */
+ mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
+ memset(mem, 0, NR_ELEMS * page_size);
+ nr_segv = 0;
+ fault_start = (unsigned long)mem;
+ fault_end = fault_start + NR_ELEMS * page_size;
+ mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
+ strided_store(mem, NR_ELEMS, page_size, vec);
+ assert(nr_segv == NR_ELEMS);
+ for (i = 0; i < NR_ELEMS; i++) {
+ assert(mem[i * page_size] == store_data[i]);
+ }
+
+ /* Indexed store test to one element on each page */
+ mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
+ memset(mem, 0, NR_ELEMS * page_size);
+ nr_segv = 0;
+ fault_start = (unsigned long)mem;
+ fault_end = fault_start + NR_ELEMS * page_size;
+ mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
+ indexed_store(mem, NR_ELEMS, indices, vec);
+ assert(nr_segv == NR_ELEMS);
+ for (i = 0; i < NR_ELEMS; i++) {
+ assert(mem[indices[i]] == store_data[i]);
+ }
+
+ munmap(mem, NR_ELEMS * page_size);
+
+ return 0;
+}
+
+int main(void)
+{
+ return run_interrupted_v_tests();
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
@ 2026-03-25 1:35 ` Alistair Francis
2026-03-25 3:08 ` Chao Liu
1 sibling, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2026-03-25 1:35 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Sun, Mar 22, 2026 at 12:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The instruction alignment check for the C extension was inverted.
> The new value should be checked for C bit clear (thus increasing
> IALIGN). If IALIGN is incompatible, then the write to misa should
> be suppressed, not just ignoring the update to the C bit.
>
> From the ISA:
>
> Writing misa may increase IALIGN, e.g., by disabling the "C"
> extension. If an instruction that would write misa increases IALIGN,
> and the subsequent instruction’s address is not IALIGN-bit aligned,
> the write to misa is suppressed, leaving misa unchanged.
>
> This was found with a verification test generator based on RiESCUE.
>
> Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 16 ++++-
> tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
> tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
> 3 files changed, 106 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/riscv64/misa-ialign.S
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5064483917..91421a2dd8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> /* Mask extensions that are not supported by this hart */
> val &= env->misa_ext_mask;
>
> - /* Suppress 'C' if next instruction is not aligned. */
> - if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> - val &= ~RVC;
> + /*
> + * misa writes that increase IALIGN beyond alignment of the next
> + * instruction cause the write to misa to be suppressed. Clearing
> + * "C" extension increases IALIGN.
> + */
> + if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> + /*
> + * If the next instruction is unaligned mod 4 then "C" must be
> + * set or this instruction could not be executing, so we know
> + * this is is clearing "C" (and not just keeping it clear).
> + */
> + g_assert(env->misa_ext & RVC);
> + return RISCV_EXCP_NONE;
> }
>
> /* Disable RVG if any of its dependencies are disabled */
> diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> index eb1ce6504a..f176f87ed0 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
> $(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
> $(QEMU_OPTS)$<)
>
> +EXTRA_RUNS += run-misa-ialign
> +run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
> +run-misa-ialign: misa-ialign
> + $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> +
> # We don't currently support the multiarch system tests
> undefine MULTIARCH_TESTS
> diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
> new file mode 100644
> index 0000000000..7f1eb30023
> --- /dev/null
> +++ b/tests/tcg/riscv64/misa-ialign.S
> @@ -0,0 +1,88 @@
> +/*
> + * Test for MISA changing C and related IALIGN alignment cases
> + *
> + * This test verifies that the "C" extension can be cleared and set in MISA,
> + * that a branch to 2-byte aligned instructions can be executed when "C" is
> + * enabled, and that a write to MISA which would increase IALIGN and cause
> + * the next instruction to be unaligned is ignored.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#define RVC (1 << ('C'-'A'))
> +#define RVV (1 << ('V'-'A'))
> +
> +.option norvc
> + .text
> + .global _start
> +_start:
> + lla t0, trap
> + csrw mtvec, t0
> +
> + csrr t0, misa
> + li t1, RVC
> + not t1, t1
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 2 # fail code
> + bne t0, t1, _exit # Could not clear RVC in MISA
> +
> + li t1, RVC
> + or t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 3 # fail code
> + bne t0, t1, _exit # Could not set RVC in MISA
> +
> + j unalign
> +. = . + 2
> +unalign:
> +
> + li t1, RVC
> + not t1, t1
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 4 # fail code
> + beq t0, t1, _exit # Was able to clear RVC in MISA
> +
> + li t0, (RVC|RVV)
> + not t0, t0
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t0, misa
> + li a0, 5 # fail code
> + bne t0, t1, _exit # MISA write was not ignored (RVV was cleared)
> +
> + j realign
> +. = . + 2
> +realign:
> +
> + # Success!
> + li a0, 0
> + j _exit
> +
> +trap:
> + # Any trap is a fail code 1
> + li a0, 1
> +
> +# Exit code in a0
> +_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
> +
> + # Semihosting call sequence
> + .balign 16
> + slli zero, zero, 0x1f
> + ebreak
> + srai zero, zero, 0x7
> + j .
> +
> + .data
> + .balign 16
> +semiargs:
> + .space 16
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check
2026-03-21 14:45 ` [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check Nicholas Piggin
@ 2026-03-25 1:57 ` Alistair Francis
2026-03-25 2:10 ` Chao Liu
1 sibling, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2026-03-25 1:57 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Sun, Mar 22, 2026 at 12:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The whole vector ldst instructions do not include a vstart check, so an
> overflowed vstart can result in an underflowed memory address offset and
> crash:
>
> accel/tcg/cputlb.c:1465:probe_access_flags:
> assertion failed: (-(addr | TARGET_PAGE_MASK) >= size)
>
> Add the VSTART_CHECK_EARLY_EXIT() check for these helpers.
>
> This was found with a verification test generator based on RiESCUE.
>
> Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/vector_helper.c | 2 +
> tests/tcg/riscv64/Makefile.target | 5 ++
> tests/tcg/riscv64/test-vstart-overflow.c | 78 ++++++++++++++++++++++++
> 3 files changed, 85 insertions(+)
> create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index caa8dd9c12..4126447d11 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -825,6 +825,8 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
> uint32_t esz = 1 << log2_esz;
> int mmu_index = riscv_env_mmu_index(env, false);
>
> + VSTART_CHECK_EARLY_EXIT(env, evl);
> +
> /* Calculate the page range of first page */
> addr = base + (env->vstart << log2_esz);
> page_split = -(addr | TARGET_PAGE_MASK);
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index 4da5b9a3b3..19a49b6467 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -18,3 +18,8 @@ TESTS += test-fcvtmod
> test-fcvtmod: CFLAGS += -march=rv64imafdc
> test-fcvtmod: LDFLAGS += -static
> run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
> +
> +# Test for vstart >= vl
> +TESTS += test-vstart-overflow
> +test-vstart-overflow: CFLAGS += -march=rv64gcv
> +run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
> diff --git a/tests/tcg/riscv64/test-vstart-overflow.c b/tests/tcg/riscv64/test-vstart-overflow.c
> new file mode 100644
> index 0000000000..6c904ab309
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-vstart-overflow.c
> @@ -0,0 +1,78 @@
> +/*
> + * Test for VSTART set to overflow VL
> + *
> + * TCG vector instructions should call VSTART_CHECK_EARLY_EXIT() to check
> + * this case, otherwise memory addresses can underflow and misbehave or
> + * crash QEMU.
> + *
> + * TODO: Add stores and other instructions.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <stdint.h>
> +
> +#define VSTART_OVERFLOW_TEST(insn) \
> +({ \
> + uint8_t vmem[64] = { 0 }; \
> + uint64_t vstart; \
> + asm volatile(" \r\n \
> + # Set VL=52 and VSTART=56 \r\n \
> + li t0, 52 \r\n \
> + vsetvli x0, t0, e8, m4, ta, ma \r\n \
> + li t0, 56 \r\n \
> + csrrw x0, vstart, t0 \r\n \
> + li t1, 64 \r\n \
> + " insn " \r\n \
> + csrr %0, vstart \r\n \
> + " : "=r"(vstart), "+A"(vmem) :: "t0", "t1", "v24", "memory"); \
> + vstart; \
> +})
> +
> +int run_vstart_overflow_tests()
> +{
> + /*
> + * An implementation is permitted to raise an illegal instruction
> + * exception when executing a vector instruction if vstart is set to a
> + * value that could not be produced by the execution of that instruction
> + * with the same vtype. If TCG is changed to do this, then this test
> + * could be updated to handle the SIGILL.
> + */
> + if (VSTART_OVERFLOW_TEST("vl1re16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vs1r.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vle16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vse16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vluxei8.v v24, %1, v20")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vloxei8.v v24, %1, v20")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vlse16.v v24, %1, t1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vlseg2e8.v v24, %1")) {
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +int main()
> +{
> + return run_vstart_overflow_tests();
> +}
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops
2026-03-21 14:45 ` [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops Nicholas Piggin
@ 2026-03-25 2:08 ` Alistair Francis
2026-03-25 3:19 ` Chao Liu
1 sibling, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2026-03-25 2:08 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
On Sun, Mar 22, 2026 at 12:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> riscv vector instructions can be interrupted with a trap, and partial
> completion is recorded in the vstart register. Some causes are
> implementation dependent, for example an asynchronous interrupt (which I
> don't think TCG allows). Others are architectural, typically memory
> access faults on vector load/store instructions.
>
> Add some TCG tests for interrupting vector load instructions and
> resuming partially completed ones.
>
> This would have caught a recent (now reverted) regression in vector
> stride load implementation, commit 28c12c1f2f50d ("Generate strided
> vector loads/stores with tcg nodes.")
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> tests/tcg/riscv64/Makefile.target | 11 +
> tests/tcg/riscv64/test-interrupted-v.c | 329 +++++++++++++++++++++++++
> 2 files changed, 340 insertions(+)
> create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
>
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index 19a49b6467..b2b2325843 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -1,6 +1,10 @@
> # -*- Mode: makefile -*-
> # RISC-V specific tweaks
>
> +# Not all environments have compilers with vector intrinsics yet.
> +HAVE_RISCV_VECTOR_INTRINSICS := $(shell echo '#ifndef __riscv_v_intrinsic\n#error\n#endif' | \
> + $(CC) -march=rv64gcv -E -x c - >/dev/null 2>&1 && echo y)
> +
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> @@ -23,3 +27,10 @@ run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
> TESTS += test-vstart-overflow
> test-vstart-overflow: CFLAGS += -march=rv64gcv
> run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
> +
> +ifeq ($(HAVE_RISCV_VECTOR_INTRINSICS),y)
> +# Test for interrupted vector instructions
> +TESTS += test-interrupted-v
> +test-interrupted-v: CFLAGS += -march=rv64gcv
> +run-test-interrupted-v: QEMU_OPTS += -cpu rv64,v=on
> +endif
> diff --git a/tests/tcg/riscv64/test-interrupted-v.c b/tests/tcg/riscv64/test-interrupted-v.c
> new file mode 100644
> index 0000000000..3d0d21b49b
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-interrupted-v.c
> @@ -0,0 +1,329 @@
> +/*
> + * Test for interrupted vector operations.
> + *
> + * Some vector instructions can be interrupted partially complete, vstart will
> + * be set to where the operation has progressed to, and the instruction can be
> + * re-executed with vstart != 0. It is implementation dependent as to what
> + * instructions can be interrupted and what vstart values are permitted when
> + * executing them. Vector memory operations can typically be interrupted
> + * (as they can take page faults), so these are easy to test.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <assert.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <riscv_vector.h>
> +
> +static unsigned long page_size;
> +
> +static volatile int nr_segv;
> +static volatile unsigned long fault_start, fault_end;
> +static volatile bool fault_write;
> +
> +/*
> + * Careful: qemu-user does not save/restore vector state in
> + * signals yet, so any library or compiler autovec code will
> + * corrupt our test.
> + *
> + * Do only minimal work in the signal handler.
> + */
> +static void SEGV_handler(int signo, siginfo_t *info, void *context)
> +{
> + unsigned long page = (unsigned long)info->si_addr &
> + ~(unsigned long)(page_size - 1);
> +
> + assert((unsigned long)info->si_addr >= fault_start);
> + assert((unsigned long)info->si_addr < fault_end);
> + if (fault_write) {
> + mprotect((void *)page, page_size, PROT_READ | PROT_WRITE);
> + } else {
> + mprotect((void *)page, page_size, PROT_READ);
> + }
> + nr_segv++;
> +}
> +
> +/* Use noinline to make generated code easier to inspect */
> +static __attribute__((noinline))
> +uint8_t unit_load(uint8_t *mem, size_t nr, bool ff)
> +{
> + size_t vl;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + if (ff) {
> + vec = __riscv_vle8ff_v_u8m1(mem, &vl, vl);
> + } else {
> + vec = __riscv_vle8_v_u8m1(mem, vl);
> + }
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t seg2_load(uint8_t *mem, size_t nr, bool ff)
> +{
> + size_t vl;
> + vuint8m1x2_t segvec;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + if (ff) {
> + segvec = __riscv_vlseg2e8ff_v_u8m1x2(mem, &vl, vl);
> + } else {
> + segvec = __riscv_vlseg2e8_v_u8m1x2(mem, vl);
> + }
> + vec = __riscv_vadd_vv_u8m1(__riscv_vget_v_u8m1x2_u8m1(segvec, 0),
> + __riscv_vget_v_u8m1x2_u8m1(segvec, 1), vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t strided_load(uint8_t *mem, size_t nr, size_t stride)
> +{
> + size_t vl;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + vec = __riscv_vlse8_v_u8m1(mem, stride, vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t indexed_load(uint8_t *mem, size_t nr, uint32_t *indices)
> +{
> + size_t vl;
> + vuint32m4_t idx;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + idx = __riscv_vle32_v_u32m4(indices, vl);
> + vec = __riscv_vloxei32_v_u8m1(mem, idx, vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +/* New store functions */
> +static __attribute__((noinline))
> +void unit_store(uint8_t *mem, size_t nr, vuint8m1_t vec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vse8_v_u8m1(mem, vec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void seg2_store(uint8_t *mem, size_t nr, vuint8m1x2_t segvec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vsseg2e8_v_u8m1x2(mem, segvec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void strided_store(uint8_t *mem, size_t nr, size_t stride, vuint8m1_t vec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vsse8_v_u8m1(mem, stride, vec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void indexed_store(uint8_t *mem, size_t nr, uint32_t *indices, vuint8m1_t vec)
> +{
> + size_t vl;
> + vuint32m4_t idx;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + idx = __riscv_vle32_v_u32m4(indices, vl);
> + __riscv_vsoxei32_v_u8m1(mem, idx, vec, vl);
> +}
> +
> +/* Use e8 elements, 128-bit vectors */
> +#define NR_ELEMS 16
> +
> +static int run_interrupted_v_tests(void)
> +{
> + struct sigaction act = { 0 };
> + uint8_t *mem;
> + uint32_t indices[NR_ELEMS];
> + int i;
> +
> + page_size = sysconf(_SC_PAGESIZE);
> +
> + act.sa_flags = SA_SIGINFO;
> + act.sa_sigaction = &SEGV_handler;
> + if (sigaction(SIGSEGV, &act, NULL) == -1) {
> + perror("sigaction");
> + exit(EXIT_FAILURE);
> + }
> +
> + mem = mmap(NULL, NR_ELEMS * page_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + assert(mem != MAP_FAILED);
> + madvise(mem, NR_ELEMS * page_size, MADV_NOHUGEPAGE);
> +
> + /*** Load tests ***/
> + fault_write = false;
> +
> + /* Unit-stride tests load memory crossing a page boundary */
> + memset(mem, 0, NR_ELEMS * page_size);
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[page_size - NR_ELEMS + i] = 3;
> + }
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[page_size + i] = 5;
> + }
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, false)
> + == 8 * NR_ELEMS / 2);
> + assert(nr_segv == 2);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS * 2;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS, false)
> + == 8 * NR_ELEMS);
> + assert(nr_segv == 2);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + (NR_ELEMS / 2);
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, true)
> + == 3 * NR_ELEMS / 2);
> + assert(nr_segv == 1); /* fault-first does not fault the second page */
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS * 2, true)
> + == 3 * NR_ELEMS);
> + assert(nr_segv == 1); /* fault-first does not fault the second page */
> +
> + /* Following tests load one element from first byte of each page */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[i * page_size] = 3;
> + indices[i] = i * page_size;
> + }
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + assert(strided_load(mem, NR_ELEMS, page_size) == 3 * NR_ELEMS);
> + assert(nr_segv == NR_ELEMS);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + assert(indexed_load(mem, NR_ELEMS, indices) == 3 * NR_ELEMS);
> + assert(nr_segv == NR_ELEMS);
> +
> + /*** Store tests ***/
> + fault_write = true;
> +
> + uint8_t store_data[NR_ELEMS];
> + uint8_t store_data_seg0[NR_ELEMS];
> + uint8_t store_data_seg1[NR_ELEMS];
> + vuint8m1_t vec;
> + vuint8m1x2_t segvec;
> + size_t vl = __riscv_vsetvl_e8m1(NR_ELEMS);
> +
> + /* Create some data to store */
> + for (i = 0; i < NR_ELEMS; i++) {
> + store_data[i] = i * 3;
> + store_data_seg0[i] = i * 5;
> + store_data_seg1[i] = i * 7;
> + }
> + vec = __riscv_vle8_v_u8m1(store_data, vl);
> + segvec = __riscv_vcreate_v_u8m1x2(
> + __riscv_vle8_v_u8m1(store_data_seg0, vl),
> + __riscv_vle8_v_u8m1(store_data_seg1, vl));
> +
> + /* Unit-stride store test crossing a page boundary */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, page_size * 2);
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + unit_store(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, vec);
> + assert(nr_segv == 2);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[page_size - (NR_ELEMS / 2) + i] == store_data[i]);
> + }
> +
> + /* Segmented store test crossing a page boundary */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, page_size * 2);
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS * 2;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + seg2_store(&mem[page_size - NR_ELEMS], NR_ELEMS, segvec);
> + assert(nr_segv == 2);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[page_size - NR_ELEMS + i * 2] == store_data_seg0[i]);
> + assert(mem[page_size - NR_ELEMS + i * 2 + 1] == store_data_seg1[i]);
> + }
> +
> + /* Strided store test to one element on each page */
> + mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + strided_store(mem, NR_ELEMS, page_size, vec);
> + assert(nr_segv == NR_ELEMS);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[i * page_size] == store_data[i]);
> + }
> +
> + /* Indexed store test to one element on each page */
> + mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + indexed_store(mem, NR_ELEMS, indices, vec);
> + assert(nr_segv == NR_ELEMS);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[indices[i]] == store_data[i]);
> + }
> +
> + munmap(mem, NR_ELEMS * page_size);
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + return run_interrupted_v_tests();
> +}
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check
2026-03-21 14:45 ` [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check Nicholas Piggin
2026-03-25 1:57 ` Alistair Francis
@ 2026-03-25 2:10 ` Chao Liu
1 sibling, 0 replies; 15+ messages in thread
From: Chao Liu @ 2026-03-25 2:10 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Sun, Mar 22, 2026 at 12:45:53AM +1000, Nicholas Piggin wrote:
> The whole vector ldst instructions do not include a vstart check, so an
> overflowed vstart can result in an underflowed memory address offset and
> crash:
>
> accel/tcg/cputlb.c:1465:probe_access_flags:
> assertion failed: (-(addr | TARGET_PAGE_MASK) >= size)
>
> Add the VSTART_CHECK_EARLY_EXIT() check for these helpers.
>
> This was found with a verification test generator based on RiESCUE.
Good catch! I had previously fixed some vector helpers that were
missing vstart checks, but I didn’t realize there were still some
omissions.
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Thanks,
Chao
>
> Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/riscv/vector_helper.c | 2 +
> tests/tcg/riscv64/Makefile.target | 5 ++
> tests/tcg/riscv64/test-vstart-overflow.c | 78 ++++++++++++++++++++++++
> 3 files changed, 85 insertions(+)
> create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index caa8dd9c12..4126447d11 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -825,6 +825,8 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
> uint32_t esz = 1 << log2_esz;
> int mmu_index = riscv_env_mmu_index(env, false);
>
> + VSTART_CHECK_EARLY_EXIT(env, evl);
> +
> /* Calculate the page range of first page */
> addr = base + (env->vstart << log2_esz);
> page_split = -(addr | TARGET_PAGE_MASK);
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index 4da5b9a3b3..19a49b6467 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -18,3 +18,8 @@ TESTS += test-fcvtmod
> test-fcvtmod: CFLAGS += -march=rv64imafdc
> test-fcvtmod: LDFLAGS += -static
> run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
> +
> +# Test for vstart >= vl
> +TESTS += test-vstart-overflow
> +test-vstart-overflow: CFLAGS += -march=rv64gcv
> +run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
> diff --git a/tests/tcg/riscv64/test-vstart-overflow.c b/tests/tcg/riscv64/test-vstart-overflow.c
> new file mode 100644
> index 0000000000..6c904ab309
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-vstart-overflow.c
> @@ -0,0 +1,78 @@
> +/*
> + * Test for VSTART set to overflow VL
> + *
> + * TCG vector instructions should call VSTART_CHECK_EARLY_EXIT() to check
> + * this case, otherwise memory addresses can underflow and misbehave or
> + * crash QEMU.
> + *
> + * TODO: Add stores and other instructions.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <stdint.h>
> +
> +#define VSTART_OVERFLOW_TEST(insn) \
> +({ \
> + uint8_t vmem[64] = { 0 }; \
> + uint64_t vstart; \
> + asm volatile(" \r\n \
> + # Set VL=52 and VSTART=56 \r\n \
> + li t0, 52 \r\n \
> + vsetvli x0, t0, e8, m4, ta, ma \r\n \
> + li t0, 56 \r\n \
> + csrrw x0, vstart, t0 \r\n \
> + li t1, 64 \r\n \
> + " insn " \r\n \
> + csrr %0, vstart \r\n \
> + " : "=r"(vstart), "+A"(vmem) :: "t0", "t1", "v24", "memory"); \
> + vstart; \
> +})
> +
> +int run_vstart_overflow_tests()
> +{
> + /*
> + * An implementation is permitted to raise an illegal instruction
> + * exception when executing a vector instruction if vstart is set to a
> + * value that could not be produced by the execution of that instruction
> + * with the same vtype. If TCG is changed to do this, then this test
> + * could be updated to handle the SIGILL.
> + */
> + if (VSTART_OVERFLOW_TEST("vl1re16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vs1r.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vle16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vse16.v v24, %1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vluxei8.v v24, %1, v20")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vloxei8.v v24, %1, v20")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vlse16.v v24, %1, t1")) {
> + return 1;
> + }
> +
> + if (VSTART_OVERFLOW_TEST("vlseg2e8.v v24, %1")) {
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +int main()
> +{
> + return run_vstart_overflow_tests();
> +}
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/3] target/riscv: corner case fixes
2026-03-21 14:45 [PATCH v3 0/3] target/riscv: corner case fixes Nicholas Piggin
` (2 preceding siblings ...)
2026-03-21 14:45 ` [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops Nicholas Piggin
@ 2026-03-25 2:20 ` Alistair Francis
3 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2026-03-25 2:20 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
On Sun, Mar 22, 2026 at 12:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Changes:
> v3:
> * Added vloxei8.v to overflow test.
> * Added store variants of interrupted vector ops tests.
>
> v2:
> * Added a tcg tests build-time check for vector intrinsics support
> in target compiler before building new tests that require it.
> ci images may not support these yet unfortunately, but upgrading
> those will be a separate effort.
>
> Thanks,
> Nick
>
>
> Nicholas Piggin (3):
> target/riscv: Fix IALIGN check in misa write
> target/riscv: Fix vector whole ldst vstart check
> tests/tcg: Add riscv test for interrupted vector ops
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> target/riscv/csr.c | 16 +-
> target/riscv/vector_helper.c | 2 +
> tests/tcg/riscv64/Makefile.softmmu-target | 5 +
> tests/tcg/riscv64/Makefile.target | 16 ++
> tests/tcg/riscv64/misa-ialign.S | 88 ++++++
> tests/tcg/riscv64/test-interrupted-v.c | 329 ++++++++++++++++++++++
> tests/tcg/riscv64/test-vstart-overflow.c | 78 +++++
> 7 files changed, 531 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/riscv64/misa-ialign.S
> create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
> create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
2026-03-25 1:35 ` Alistair Francis
@ 2026-03-25 3:08 ` Chao Liu
2026-03-25 3:26 ` Alistair Francis
1 sibling, 1 reply; 15+ messages in thread
From: Chao Liu @ 2026-03-25 3:08 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Sun, Mar 22, 2026 at 12:45:52AM +1000, Nicholas Piggin wrote:
> The instruction alignment check for the C extension was inverted.
> The new value should be checked for C bit clear (thus increasing
> IALIGN). If IALIGN is incompatible, then the write to misa should
> be suppressed, not just ignoring the update to the C bit.
>
> From the ISA:
>
> Writing misa may increase IALIGN, e.g., by disabling the "C"
> extension. If an instruction that would write misa increases IALIGN,
> and the subsequent instruction’s address is not IALIGN-bit aligned,
> the write to misa is suppressed, leaving misa unchanged.
>
> This was found with a verification test generator based on RiESCUE.
>
> Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/riscv/csr.c | 16 ++++-
> tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
> tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
> 3 files changed, 106 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/riscv64/misa-ialign.S
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5064483917..91421a2dd8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> /* Mask extensions that are not supported by this hart */
> val &= env->misa_ext_mask;
>
> - /* Suppress 'C' if next instruction is not aligned. */
> - if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> - val &= ~RVC;
> + /*
> + * misa writes that increase IALIGN beyond alignment of the next
> + * instruction cause the write to misa to be suppressed. Clearing
> + * "C" extension increases IALIGN.
> + */
> + if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> + /*
> + * If the next instruction is unaligned mod 4 then "C" must be
> + * set or this instruction could not be executing, so we know
> + * this is is clearing "C" (and not just keeping it clear).
"this is is clearing" — double "is"
> + */
> + g_assert(env->misa_ext & RVC);
> + return RISCV_EXCP_NONE;
write_misa() is also reachable via riscv_csrrw_debug()
with ra=0, where get_next_pc() falls back to env->pc.
A debugger can set PC to a 2-byte-aligned address while C is
already disabled, then write misa keeping C=0. This hits
the condition and fires the g_assert.
The ISA spec language:
"if an instruction that would write misa..."
does not cover debug writes, so the IALIGN suppression
arguably should not apply in that case at all.
We can:
if (ra && !(val & RVC)
&& (get_next_pc(env, ra) & 3) != 0) {
g_assert(env->misa_ext & RVC);
return RISCV_EXCP_NONE;
}
Thanks,
Chao
> }
>
> /* Disable RVG if any of its dependencies are disabled */
> diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> index eb1ce6504a..f176f87ed0 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
> $(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
> $(QEMU_OPTS)$<)
>
> +EXTRA_RUNS += run-misa-ialign
> +run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
> +run-misa-ialign: misa-ialign
> + $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> +
> # We don't currently support the multiarch system tests
> undefine MULTIARCH_TESTS
> diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
> new file mode 100644
> index 0000000000..7f1eb30023
> --- /dev/null
> +++ b/tests/tcg/riscv64/misa-ialign.S
> @@ -0,0 +1,88 @@
> +/*
> + * Test for MISA changing C and related IALIGN alignment cases
> + *
> + * This test verifies that the "C" extension can be cleared and set in MISA,
> + * that a branch to 2-byte aligned instructions can be executed when "C" is
> + * enabled, and that a write to MISA which would increase IALIGN and cause
> + * the next instruction to be unaligned is ignored.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#define RVC (1 << ('C'-'A'))
> +#define RVV (1 << ('V'-'A'))
> +
> +.option norvc
> + .text
> + .global _start
> +_start:
> + lla t0, trap
> + csrw mtvec, t0
> +
> + csrr t0, misa
> + li t1, RVC
> + not t1, t1
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 2 # fail code
> + bne t0, t1, _exit # Could not clear RVC in MISA
> +
> + li t1, RVC
> + or t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 3 # fail code
> + bne t0, t1, _exit # Could not set RVC in MISA
> +
> + j unalign
> +. = . + 2
> +unalign:
> +
> + li t1, RVC
> + not t1, t1
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t1, misa
> + li a0, 4 # fail code
> + beq t0, t1, _exit # Was able to clear RVC in MISA
> +
> + li t0, (RVC|RVV)
> + not t0, t0
> + and t0, t0, t1
> + csrw misa, t0
> + csrr t0, misa
> + li a0, 5 # fail code
> + bne t0, t1, _exit # MISA write was not ignored (RVV was cleared)
> +
> + j realign
> +. = . + 2
> +realign:
> +
> + # Success!
> + li a0, 0
> + j _exit
> +
> +trap:
> + # Any trap is a fail code 1
> + li a0, 1
> +
> +# Exit code in a0
> +_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
> +
> + # Semihosting call sequence
> + .balign 16
> + slli zero, zero, 0x1f
> + ebreak
> + srai zero, zero, 0x7
> + j .
> +
> + .data
> + .balign 16
> +semiargs:
> + .space 16
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops
2026-03-21 14:45 ` [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops Nicholas Piggin
2026-03-25 2:08 ` Alistair Francis
@ 2026-03-25 3:19 ` Chao Liu
2026-03-26 6:32 ` Nicholas Piggin
1 sibling, 1 reply; 15+ messages in thread
From: Chao Liu @ 2026-03-25 3:19 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
On Sun, Mar 22, 2026 at 12:45:54AM +1000, Nicholas Piggin wrote:
> riscv vector instructions can be interrupted with a trap, and partial
> completion is recorded in the vstart register. Some causes are
> implementation dependent, for example an asynchronous interrupt (which I
> don't think TCG allows). Others are architectural, typically memory
> access faults on vector load/store instructions.
>
> Add some TCG tests for interrupting vector load instructions and
> resuming partially completed ones.
>
> This would have caught a recent (now reverted) regression in vector
> stride load implementation, commit 28c12c1f2f50d ("Generate strided
> vector loads/stores with tcg nodes.")
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> tests/tcg/riscv64/Makefile.target | 11 +
> tests/tcg/riscv64/test-interrupted-v.c | 329 +++++++++++++++++++++++++
> 2 files changed, 340 insertions(+)
> create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
>
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index 19a49b6467..b2b2325843 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -1,6 +1,10 @@
> # -*- Mode: makefile -*-
> # RISC-V specific tweaks
>
> +# Not all environments have compilers with vector intrinsics yet.
> +HAVE_RISCV_VECTOR_INTRINSICS := $(shell echo '#ifndef __riscv_v_intrinsic\n#error\n#endif' | \
> + $(CC) -march=rv64gcv -E -x c - >/dev/null 2>&1 && echo y)
The feature probe relies on echo interpreting '\n'
as newlines, but this is not portable.
POSIX leaves echo's handling of backslash sequences
implementation-defined.
On bash (the default /bin/sh on many systems including
Fedora, Arch, and some CI images), echo outputs literal
'\n', so the preprocessor sees a malformed single-line input
and always fails. This silently disables test-interrupted-v
even when the compiler supports vector intrinsics.
I suggest using printf for portability:
HAVE_RISCV_VECTOR_INTRINSICS := $(shell printf \
'#ifndef __riscv_v_intrinsic\n#error\n#endif\n' | \
$(CC) -march=rv64gcv -E -x c - \
>/dev/null 2>&1 && echo y)
> +
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> @@ -23,3 +27,10 @@ run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
> TESTS += test-vstart-overflow
> test-vstart-overflow: CFLAGS += -march=rv64gcv
> run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
> +
> +ifeq ($(HAVE_RISCV_VECTOR_INTRINSICS),y)
> +# Test for interrupted vector instructions
> +TESTS += test-interrupted-v
> +test-interrupted-v: CFLAGS += -march=rv64gcv
> +run-test-interrupted-v: QEMU_OPTS += -cpu rv64,v=on
> +endif
> diff --git a/tests/tcg/riscv64/test-interrupted-v.c b/tests/tcg/riscv64/test-interrupted-v.c
> new file mode 100644
> index 0000000000..3d0d21b49b
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-interrupted-v.c
> @@ -0,0 +1,329 @@
> +/*
> + * Test for interrupted vector operations.
> + *
> + * Some vector instructions can be interrupted partially complete, vstart will
> + * be set to where the operation has progressed to, and the instruction can be
> + * re-executed with vstart != 0. It is implementation dependent as to what
> + * instructions can be interrupted and what vstart values are permitted when
> + * executing them. Vector memory operations can typically be interrupted
> + * (as they can take page faults), so these are easy to test.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <assert.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <riscv_vector.h>
> +
> +static unsigned long page_size;
> +
> +static volatile int nr_segv;
> +static volatile unsigned long fault_start, fault_end;
> +static volatile bool fault_write;
checkpatch errors:
ERROR: Use of volatile is usually wrong, please add a comment
#84: FILE: tests/tcg/riscv64/test-interrupted-v.c:26:
+static volatile int nr_segv;
ERROR: Use of volatile is usually wrong, please add a comment
#85: FILE: tests/tcg/riscv64/test-interrupted-v.c:27:
+static volatile unsigned long fault_start, fault_end;
ERROR: Use of volatile is usually wrong, please add a comment
#86: FILE: tests/tcg/riscv64/test-interrupted-v.c:28:
+static volatile bool fault_write;
Please fix it.
> +
> +/*
> + * Careful: qemu-user does not save/restore vector state in
> + * signals yet, so any library or compiler autovec code will
> + * corrupt our test.
> + *
> + * Do only minimal work in the signal handler.
> + */
> +static void SEGV_handler(int signo, siginfo_t *info, void *context)
> +{
> + unsigned long page = (unsigned long)info->si_addr &
> + ~(unsigned long)(page_size - 1);
> +
> + assert((unsigned long)info->si_addr >= fault_start);
> + assert((unsigned long)info->si_addr < fault_end);
> + if (fault_write) {
> + mprotect((void *)page, page_size, PROT_READ | PROT_WRITE);
> + } else {
> + mprotect((void *)page, page_size, PROT_READ);
> + }
> + nr_segv++;
> +}
> +
> +/* Use noinline to make generated code easier to inspect */
> +static __attribute__((noinline))
> +uint8_t unit_load(uint8_t *mem, size_t nr, bool ff)
> +{
> + size_t vl;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + if (ff) {
> + vec = __riscv_vle8ff_v_u8m1(mem, &vl, vl);
> + } else {
> + vec = __riscv_vle8_v_u8m1(mem, vl);
> + }
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t seg2_load(uint8_t *mem, size_t nr, bool ff)
> +{
> + size_t vl;
> + vuint8m1x2_t segvec;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + if (ff) {
> + segvec = __riscv_vlseg2e8ff_v_u8m1x2(mem, &vl, vl);
> + } else {
> + segvec = __riscv_vlseg2e8_v_u8m1x2(mem, vl);
> + }
> + vec = __riscv_vadd_vv_u8m1(__riscv_vget_v_u8m1x2_u8m1(segvec, 0),
> + __riscv_vget_v_u8m1x2_u8m1(segvec, 1), vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t strided_load(uint8_t *mem, size_t nr, size_t stride)
> +{
> + size_t vl;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + vec = __riscv_vlse8_v_u8m1(mem, stride, vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +static __attribute__((noinline))
> +uint8_t indexed_load(uint8_t *mem, size_t nr, uint32_t *indices)
> +{
> + size_t vl;
> + vuint32m4_t idx;
> + vuint8m1_t vec, redvec, sum;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + idx = __riscv_vle32_v_u32m4(indices, vl);
> + vec = __riscv_vloxei32_v_u8m1(mem, idx, vl);
> + redvec = __riscv_vmv_v_x_u8m1(0, vl);
> + sum = __riscv_vredsum_vs_u8m1_u8m1(vec, redvec, vl);
> + return __riscv_vmv_x_s_u8m1_u8(sum);
> +}
> +
> +/* New store functions */
> +static __attribute__((noinline))
> +void unit_store(uint8_t *mem, size_t nr, vuint8m1_t vec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vse8_v_u8m1(mem, vec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void seg2_store(uint8_t *mem, size_t nr, vuint8m1x2_t segvec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vsseg2e8_v_u8m1x2(mem, segvec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void strided_store(uint8_t *mem, size_t nr, size_t stride, vuint8m1_t vec)
> +{
> + size_t vl;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + __riscv_vsse8_v_u8m1(mem, stride, vec, vl);
> +}
> +
> +static __attribute__((noinline))
> +void indexed_store(uint8_t *mem, size_t nr, uint32_t *indices, vuint8m1_t vec)
> +{
> + size_t vl;
> + vuint32m4_t idx;
> +
> + vl = __riscv_vsetvl_e8m1(nr);
> + idx = __riscv_vle32_v_u32m4(indices, vl);
> + __riscv_vsoxei32_v_u8m1(mem, idx, vec, vl);
> +}
> +
> +/* Use e8 elements, 128-bit vectors */
> +#define NR_ELEMS 16
> +
> +static int run_interrupted_v_tests(void)
> +{
> + struct sigaction act = { 0 };
> + uint8_t *mem;
> + uint32_t indices[NR_ELEMS];
> + int i;
> +
> + page_size = sysconf(_SC_PAGESIZE);
Regarding other checkpatch errors: the volatile usage and
sysconf(_SC_PAGESIZE) are correct for a guest test binary
that has no access to QEMU internals. These are
false positives.
Thanks,
Chao
> +
> + act.sa_flags = SA_SIGINFO;
> + act.sa_sigaction = &SEGV_handler;
> + if (sigaction(SIGSEGV, &act, NULL) == -1) {
> + perror("sigaction");
> + exit(EXIT_FAILURE);
> + }
> +
> + mem = mmap(NULL, NR_ELEMS * page_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + assert(mem != MAP_FAILED);
> + madvise(mem, NR_ELEMS * page_size, MADV_NOHUGEPAGE);
> +
> + /*** Load tests ***/
> + fault_write = false;
> +
> + /* Unit-stride tests load memory crossing a page boundary */
> + memset(mem, 0, NR_ELEMS * page_size);
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[page_size - NR_ELEMS + i] = 3;
> + }
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[page_size + i] = 5;
> + }
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, false)
> + == 8 * NR_ELEMS / 2);
> + assert(nr_segv == 2);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS * 2;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS, false)
> + == 8 * NR_ELEMS);
> + assert(nr_segv == 2);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + (NR_ELEMS / 2);
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(unit_load(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, true)
> + == 3 * NR_ELEMS / 2);
> + assert(nr_segv == 1); /* fault-first does not fault the second page */
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + assert(seg2_load(&mem[page_size - NR_ELEMS], NR_ELEMS * 2, true)
> + == 3 * NR_ELEMS);
> + assert(nr_segv == 1); /* fault-first does not fault the second page */
> +
> + /* Following tests load one element from first byte of each page */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + for (i = 0; i < NR_ELEMS; i++) {
> + mem[i * page_size] = 3;
> + indices[i] = i * page_size;
> + }
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + assert(strided_load(mem, NR_ELEMS, page_size) == 3 * NR_ELEMS);
> + assert(nr_segv == NR_ELEMS);
> +
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + assert(indexed_load(mem, NR_ELEMS, indices) == 3 * NR_ELEMS);
> + assert(nr_segv == NR_ELEMS);
> +
> + /*** Store tests ***/
> + fault_write = true;
> +
> + uint8_t store_data[NR_ELEMS];
> + uint8_t store_data_seg0[NR_ELEMS];
> + uint8_t store_data_seg1[NR_ELEMS];
> + vuint8m1_t vec;
> + vuint8m1x2_t segvec;
> + size_t vl = __riscv_vsetvl_e8m1(NR_ELEMS);
> +
> + /* Create some data to store */
> + for (i = 0; i < NR_ELEMS; i++) {
> + store_data[i] = i * 3;
> + store_data_seg0[i] = i * 5;
> + store_data_seg1[i] = i * 7;
> + }
> + vec = __riscv_vle8_v_u8m1(store_data, vl);
> + segvec = __riscv_vcreate_v_u8m1x2(
> + __riscv_vle8_v_u8m1(store_data_seg0, vl),
> + __riscv_vle8_v_u8m1(store_data_seg1, vl));
> +
> + /* Unit-stride store test crossing a page boundary */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, page_size * 2);
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - (NR_ELEMS / 2)];
> + fault_end = fault_start + NR_ELEMS;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + unit_store(&mem[page_size - (NR_ELEMS / 2)], NR_ELEMS, vec);
> + assert(nr_segv == 2);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[page_size - (NR_ELEMS / 2) + i] == store_data[i]);
> + }
> +
> + /* Segmented store test crossing a page boundary */
> + mprotect(mem, page_size * 2, PROT_READ | PROT_WRITE);
> + memset(mem, 0, page_size * 2);
> + nr_segv = 0;
> + fault_start = (unsigned long)&mem[page_size - NR_ELEMS];
> + fault_end = fault_start + NR_ELEMS * 2;
> + mprotect(mem, page_size * 2, PROT_NONE);
> + seg2_store(&mem[page_size - NR_ELEMS], NR_ELEMS, segvec);
> + assert(nr_segv == 2);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[page_size - NR_ELEMS + i * 2] == store_data_seg0[i]);
> + assert(mem[page_size - NR_ELEMS + i * 2 + 1] == store_data_seg1[i]);
> + }
> +
> + /* Strided store test to one element on each page */
> + mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + strided_store(mem, NR_ELEMS, page_size, vec);
> + assert(nr_segv == NR_ELEMS);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[i * page_size] == store_data[i]);
> + }
> +
> + /* Indexed store test to one element on each page */
> + mprotect(mem, NR_ELEMS * page_size, PROT_READ | PROT_WRITE);
> + memset(mem, 0, NR_ELEMS * page_size);
> + nr_segv = 0;
> + fault_start = (unsigned long)mem;
> + fault_end = fault_start + NR_ELEMS * page_size;
> + mprotect(mem, NR_ELEMS * page_size, PROT_NONE);
> + indexed_store(mem, NR_ELEMS, indices, vec);
> + assert(nr_segv == NR_ELEMS);
> + for (i = 0; i < NR_ELEMS; i++) {
> + assert(mem[indices[i]] == store_data[i]);
> + }
> +
> + munmap(mem, NR_ELEMS * page_size);
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + return run_interrupted_v_tests();
> +}
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-25 3:08 ` Chao Liu
@ 2026-03-25 3:26 ` Alistair Francis
2026-03-25 3:40 ` Chao Liu
0 siblings, 1 reply; 15+ messages in thread
From: Alistair Francis @ 2026-03-25 3:26 UTC (permalink / raw)
To: Chao Liu
Cc: Nicholas Piggin, qemu-riscv, Laurent Vivier, Palmer Dabbelt,
Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
qemu-devel, Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Wed, Mar 25, 2026 at 1:09 PM Chao Liu <chao.liu.zevorn@gmail.com> wrote:
>
> On Sun, Mar 22, 2026 at 12:45:52AM +1000, Nicholas Piggin wrote:
> > The instruction alignment check for the C extension was inverted.
> > The new value should be checked for C bit clear (thus increasing
> > IALIGN). If IALIGN is incompatible, then the write to misa should
> > be suppressed, not just ignoring the update to the C bit.
> >
> > From the ISA:
> >
> > Writing misa may increase IALIGN, e.g., by disabling the "C"
> > extension. If an instruction that would write misa increases IALIGN,
> > and the subsequent instruction’s address is not IALIGN-bit aligned,
> > the write to misa is suppressed, leaving misa unchanged.
> >
> > This was found with a verification test generator based on RiESCUE.
> >
> > Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> > Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > target/riscv/csr.c | 16 ++++-
> > tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
> > tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
> > 3 files changed, 106 insertions(+), 3 deletions(-)
> > create mode 100644 tests/tcg/riscv64/misa-ialign.S
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 5064483917..91421a2dd8 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> > /* Mask extensions that are not supported by this hart */
> > val &= env->misa_ext_mask;
> >
> > - /* Suppress 'C' if next instruction is not aligned. */
> > - if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > - val &= ~RVC;
> > + /*
> > + * misa writes that increase IALIGN beyond alignment of the next
> > + * instruction cause the write to misa to be suppressed. Clearing
> > + * "C" extension increases IALIGN.
> > + */
> > + if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > + /*
> > + * If the next instruction is unaligned mod 4 then "C" must be
> > + * set or this instruction could not be executing, so we know
> > + * this is is clearing "C" (and not just keeping it clear).
> "this is is clearing" — double "is"
>
> > + */
> > + g_assert(env->misa_ext & RVC);
> > + return RISCV_EXCP_NONE;
> write_misa() is also reachable via riscv_csrrw_debug()
> with ra=0, where get_next_pc() falls back to env->pc.
Ah good catch
> A debugger can set PC to a 2-byte-aligned address while C is
> already disabled, then write misa keeping C=0. This hits
> the condition and fires the g_assert.
I'm not convinced that that's necessarily bad, as that's an odd and
invalid thing to be writing. But we probably shouldn't assert
>
> The ISA spec language:
>
> "if an instruction that would write misa..."
>
> does not cover debug writes, so the IALIGN suppression
> arguably should not apply in that case at all.
>
> We can:
>
> if (ra && !(val & RVC)
> && (get_next_pc(env, ra) & 3) != 0) {
> g_assert(env->misa_ext & RVC);
> return RISCV_EXCP_NONE;
> }
Maybe it's best to change the `g_assert()` to a log GUEST_ERROR
instead. That way we flag that something fishy is going on, but don't
exit QEMU
Alistair
>
> Thanks,
> Chao
> > }
> >
> > /* Disable RVG if any of its dependencies are disabled */
> > diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> > index eb1ce6504a..f176f87ed0 100644
> > --- a/tests/tcg/riscv64/Makefile.softmmu-target
> > +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> > @@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
> > $(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
> > $(QEMU_OPTS)$<)
> >
> > +EXTRA_RUNS += run-misa-ialign
> > +run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
> > +run-misa-ialign: misa-ialign
> > + $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> > +
> > # We don't currently support the multiarch system tests
> > undefine MULTIARCH_TESTS
> > diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
> > new file mode 100644
> > index 0000000000..7f1eb30023
> > --- /dev/null
> > +++ b/tests/tcg/riscv64/misa-ialign.S
> > @@ -0,0 +1,88 @@
> > +/*
> > + * Test for MISA changing C and related IALIGN alignment cases
> > + *
> > + * This test verifies that the "C" extension can be cleared and set in MISA,
> > + * that a branch to 2-byte aligned instructions can be executed when "C" is
> > + * enabled, and that a write to MISA which would increase IALIGN and cause
> > + * the next instruction to be unaligned is ignored.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#define RVC (1 << ('C'-'A'))
> > +#define RVV (1 << ('V'-'A'))
> > +
> > +.option norvc
> > + .text
> > + .global _start
> > +_start:
> > + lla t0, trap
> > + csrw mtvec, t0
> > +
> > + csrr t0, misa
> > + li t1, RVC
> > + not t1, t1
> > + and t0, t0, t1
> > + csrw misa, t0
> > + csrr t1, misa
> > + li a0, 2 # fail code
> > + bne t0, t1, _exit # Could not clear RVC in MISA
> > +
> > + li t1, RVC
> > + or t0, t0, t1
> > + csrw misa, t0
> > + csrr t1, misa
> > + li a0, 3 # fail code
> > + bne t0, t1, _exit # Could not set RVC in MISA
> > +
> > + j unalign
> > +. = . + 2
> > +unalign:
> > +
> > + li t1, RVC
> > + not t1, t1
> > + and t0, t0, t1
> > + csrw misa, t0
> > + csrr t1, misa
> > + li a0, 4 # fail code
> > + beq t0, t1, _exit # Was able to clear RVC in MISA
> > +
> > + li t0, (RVC|RVV)
> > + not t0, t0
> > + and t0, t0, t1
> > + csrw misa, t0
> > + csrr t0, misa
> > + li a0, 5 # fail code
> > + bne t0, t1, _exit # MISA write was not ignored (RVV was cleared)
> > +
> > + j realign
> > +. = . + 2
> > +realign:
> > +
> > + # Success!
> > + li a0, 0
> > + j _exit
> > +
> > +trap:
> > + # Any trap is a fail code 1
> > + li a0, 1
> > +
> > +# Exit code in a0
> > +_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
> > +
> > + # Semihosting call sequence
> > + .balign 16
> > + slli zero, zero, 0x1f
> > + ebreak
> > + srai zero, zero, 0x7
> > + j .
> > +
> > + .data
> > + .balign 16
> > +semiargs:
> > + .space 16
> > --
> > 2.51.0
> >
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-25 3:26 ` Alistair Francis
@ 2026-03-25 3:40 ` Chao Liu
2026-03-26 6:29 ` Nicholas Piggin
0 siblings, 1 reply; 15+ messages in thread
From: Chao Liu @ 2026-03-25 3:40 UTC (permalink / raw)
To: Alistair Francis
Cc: Nicholas Piggin, qemu-riscv, Laurent Vivier, Palmer Dabbelt,
Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
qemu-devel, Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Wed, Mar 25, 2026 at 01:26:56PM +1000, Alistair Francis wrote:
> On Wed, Mar 25, 2026 at 1:09 PM Chao Liu <chao.liu.zevorn@gmail.com> wrote:
> >
> > On Sun, Mar 22, 2026 at 12:45:52AM +1000, Nicholas Piggin wrote:
> > > The instruction alignment check for the C extension was inverted.
> > > The new value should be checked for C bit clear (thus increasing
> > > IALIGN). If IALIGN is incompatible, then the write to misa should
> > > be suppressed, not just ignoring the update to the C bit.
> > >
> > > From the ISA:
> > >
> > > Writing misa may increase IALIGN, e.g., by disabling the "C"
> > > extension. If an instruction that would write misa increases IALIGN,
> > > and the subsequent instruction’s address is not IALIGN-bit aligned,
> > > the write to misa is suppressed, leaving misa unchanged.
> > >
> > > This was found with a verification test generator based on RiESCUE.
> > >
> > > Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> > > Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > > target/riscv/csr.c | 16 ++++-
> > > tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
> > > tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
> > > 3 files changed, 106 insertions(+), 3 deletions(-)
> > > create mode 100644 tests/tcg/riscv64/misa-ialign.S
> > >
> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > index 5064483917..91421a2dd8 100644
> > > --- a/target/riscv/csr.c
> > > +++ b/target/riscv/csr.c
> > > @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> > > /* Mask extensions that are not supported by this hart */
> > > val &= env->misa_ext_mask;
> > >
> > > - /* Suppress 'C' if next instruction is not aligned. */
> > > - if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > > - val &= ~RVC;
> > > + /*
> > > + * misa writes that increase IALIGN beyond alignment of the next
> > > + * instruction cause the write to misa to be suppressed. Clearing
> > > + * "C" extension increases IALIGN.
> > > + */
> > > + if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > > + /*
> > > + * If the next instruction is unaligned mod 4 then "C" must be
> > > + * set or this instruction could not be executing, so we know
> > > + * this is is clearing "C" (and not just keeping it clear).
> > "this is is clearing" — double "is"
> >
> > > + */
> > > + g_assert(env->misa_ext & RVC);
> > > + return RISCV_EXCP_NONE;
> > write_misa() is also reachable via riscv_csrrw_debug()
> > with ra=0, where get_next_pc() falls back to env->pc.
>
> Ah good catch
>
> > A debugger can set PC to a 2-byte-aligned address while C is
> > already disabled, then write misa keeping C=0. This hits
> > the condition and fires the g_assert.
>
> I'm not convinced that that's necessarily bad, as that's an odd and
> invalid thing to be writing. But we probably shouldn't assert
>
> >
> > The ISA spec language:
> >
> > "if an instruction that would write misa..."
> >
> > does not cover debug writes, so the IALIGN suppression
> > arguably should not apply in that case at all.
> >
> > We can:
> >
> > if (ra && !(val & RVC)
> > && (get_next_pc(env, ra) & 3) != 0) {
> > g_assert(env->misa_ext & RVC);
> > return RISCV_EXCP_NONE;
> > }
>
> Maybe it's best to change the `g_assert()` to a log GUEST_ERROR
> instead. That way we flag that something fishy is going on, but don't
> exit QEMU
>
Agreed!
Replacing the g_assert() with qemu_log_mask(LOG_GUEST_ERROR, ...)
sounds like the right balance — it still surfaces the anomaly for
anyone debugging, without taking down QEMU over what is ultimately
a debugger-induced corner case.
The IALIGN suppression logic itself is still correct for the normal
execution path, so there's no reason to skip it entirely; just don't
crash on the weird one.
Thanks,
Chao
> Alistair
>
> >
> > Thanks,
> > Chao
> > > }
> > >
> > > /* Disable RVG if any of its dependencies are disabled */
> > > diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> > > index eb1ce6504a..f176f87ed0 100644
> > > --- a/tests/tcg/riscv64/Makefile.softmmu-target
> > > +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> > > @@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
> > > $(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
> > > $(QEMU_OPTS)$<)
> > >
> > > +EXTRA_RUNS += run-misa-ialign
> > > +run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
> > > +run-misa-ialign: misa-ialign
> > > + $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> > > +
> > > # We don't currently support the multiarch system tests
> > > undefine MULTIARCH_TESTS
> > > diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
> > > new file mode 100644
> > > index 0000000000..7f1eb30023
> > > --- /dev/null
> > > +++ b/tests/tcg/riscv64/misa-ialign.S
> > > @@ -0,0 +1,88 @@
> > > +/*
> > > + * Test for MISA changing C and related IALIGN alignment cases
> > > + *
> > > + * This test verifies that the "C" extension can be cleared and set in MISA,
> > > + * that a branch to 2-byte aligned instructions can be executed when "C" is
> > > + * enabled, and that a write to MISA which would increase IALIGN and cause
> > > + * the next instruction to be unaligned is ignored.
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + */
> > > +
> > > +#define RVC (1 << ('C'-'A'))
> > > +#define RVV (1 << ('V'-'A'))
> > > +
> > > +.option norvc
> > > + .text
> > > + .global _start
> > > +_start:
> > > + lla t0, trap
> > > + csrw mtvec, t0
> > > +
> > > + csrr t0, misa
> > > + li t1, RVC
> > > + not t1, t1
> > > + and t0, t0, t1
> > > + csrw misa, t0
> > > + csrr t1, misa
> > > + li a0, 2 # fail code
> > > + bne t0, t1, _exit # Could not clear RVC in MISA
> > > +
> > > + li t1, RVC
> > > + or t0, t0, t1
> > > + csrw misa, t0
> > > + csrr t1, misa
> > > + li a0, 3 # fail code
> > > + bne t0, t1, _exit # Could not set RVC in MISA
> > > +
> > > + j unalign
> > > +. = . + 2
> > > +unalign:
> > > +
> > > + li t1, RVC
> > > + not t1, t1
> > > + and t0, t0, t1
> > > + csrw misa, t0
> > > + csrr t1, misa
> > > + li a0, 4 # fail code
> > > + beq t0, t1, _exit # Was able to clear RVC in MISA
> > > +
> > > + li t0, (RVC|RVV)
> > > + not t0, t0
> > > + and t0, t0, t1
> > > + csrw misa, t0
> > > + csrr t0, misa
> > > + li a0, 5 # fail code
> > > + bne t0, t1, _exit # MISA write was not ignored (RVV was cleared)
> > > +
> > > + j realign
> > > +. = . + 2
> > > +realign:
> > > +
> > > + # Success!
> > > + li a0, 0
> > > + j _exit
> > > +
> > > +trap:
> > > + # Any trap is a fail code 1
> > > + li a0, 1
> > > +
> > > +# Exit code in a0
> > > +_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
> > > +
> > > + # Semihosting call sequence
> > > + .balign 16
> > > + slli zero, zero, 0x1f
> > > + ebreak
> > > + srai zero, zero, 0x7
> > > + j .
> > > +
> > > + .data
> > > + .balign 16
> > > +semiargs:
> > > + .space 16
> > > --
> > > 2.51.0
> > >
> > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write
2026-03-25 3:40 ` Chao Liu
@ 2026-03-26 6:29 ` Nicholas Piggin
0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-26 6:29 UTC (permalink / raw)
To: Chao Liu
Cc: Alistair Francis, qemu-riscv, Laurent Vivier, Palmer Dabbelt,
Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
qemu-devel, Joel Stanley, Nicholas Joaquin, Ganesh Valliappan
On Wed, Mar 25, 2026 at 11:40:18AM +0800, Chao Liu wrote:
> On Wed, Mar 25, 2026 at 01:26:56PM +1000, Alistair Francis wrote:
> > On Wed, Mar 25, 2026 at 1:09 PM Chao Liu <chao.liu.zevorn@gmail.com> wrote:
> > >
> > > On Sun, Mar 22, 2026 at 12:45:52AM +1000, Nicholas Piggin wrote:
> > > > The instruction alignment check for the C extension was inverted.
> > > > The new value should be checked for C bit clear (thus increasing
> > > > IALIGN). If IALIGN is incompatible, then the write to misa should
> > > > be suppressed, not just ignoring the update to the C bit.
> > > >
> > > > From the ISA:
> > > >
> > > > Writing misa may increase IALIGN, e.g., by disabling the "C"
> > > > extension. If an instruction that would write misa increases IALIGN,
> > > > and the subsequent instruction’s address is not IALIGN-bit aligned,
> > > > the write to misa is suppressed, leaving misa unchanged.
> > > >
> > > > This was found with a verification test generator based on RiESCUE.
> > > >
> > > > Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> > > > Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > > ---
> > > > target/riscv/csr.c | 16 ++++-
> > > > tests/tcg/riscv64/Makefile.softmmu-target | 5 ++
> > > > tests/tcg/riscv64/misa-ialign.S | 88 +++++++++++++++++++++++
> > > > 3 files changed, 106 insertions(+), 3 deletions(-)
> > > > create mode 100644 tests/tcg/riscv64/misa-ialign.S
> > > >
> > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > > index 5064483917..91421a2dd8 100644
> > > > --- a/target/riscv/csr.c
> > > > +++ b/target/riscv/csr.c
> > > > @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> > > > /* Mask extensions that are not supported by this hart */
> > > > val &= env->misa_ext_mask;
> > > >
> > > > - /* Suppress 'C' if next instruction is not aligned. */
> > > > - if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > > > - val &= ~RVC;
> > > > + /*
> > > > + * misa writes that increase IALIGN beyond alignment of the next
> > > > + * instruction cause the write to misa to be suppressed. Clearing
> > > > + * "C" extension increases IALIGN.
> > > > + */
> > > > + if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> > > > + /*
> > > > + * If the next instruction is unaligned mod 4 then "C" must be
> > > > + * set or this instruction could not be executing, so we know
> > > > + * this is is clearing "C" (and not just keeping it clear).
> > > "this is is clearing" — double "is"
> > >
> > > > + */
> > > > + g_assert(env->misa_ext & RVC);
> > > > + return RISCV_EXCP_NONE;
> > > write_misa() is also reachable via riscv_csrrw_debug()
> > > with ra=0, where get_next_pc() falls back to env->pc.
> >
> > Ah good catch
> >
> > > A debugger can set PC to a 2-byte-aligned address while C is
> > > already disabled, then write misa keeping C=0. This hits
> > > the condition and fires the g_assert.
> >
> > I'm not convinced that that's necessarily bad, as that's an odd and
> > invalid thing to be writing. But we probably shouldn't assert
> >
> > >
> > > The ISA spec language:
> > >
> > > "if an instruction that would write misa..."
> > >
> > > does not cover debug writes, so the IALIGN suppression
> > > arguably should not apply in that case at all.
> > >
> > > We can:
> > >
> > > if (ra && !(val & RVC)
> > > && (get_next_pc(env, ra) & 3) != 0) {
> > > g_assert(env->misa_ext & RVC);
> > > return RISCV_EXCP_NONE;
> > > }
> >
> > Maybe it's best to change the `g_assert()` to a log GUEST_ERROR
> > instead. That way we flag that something fishy is going on, but don't
> > exit QEMU
> >
> Agreed!
>
> Replacing the g_assert() with qemu_log_mask(LOG_GUEST_ERROR, ...)
> sounds like the right balance — it still surfaces the anomaly for
> anyone debugging, without taking down QEMU over what is ultimately
> a debugger-induced corner case.
>
> The IALIGN suppression logic itself is still correct for the normal
> execution path, so there's no reason to skip it entirely; just don't
> crash on the weird one.
Okay good feedback, thanks to you both. I agree, I'll make the changes
and resend.
Thanks,
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops
2026-03-25 3:19 ` Chao Liu
@ 2026-03-26 6:32 ` Nicholas Piggin
0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2026-03-26 6:32 UTC (permalink / raw)
To: Chao Liu
Cc: qemu-riscv, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel,
Joel Stanley
On Wed, Mar 25, 2026 at 11:19:26AM +0800, Chao Liu wrote:
> On Sun, Mar 22, 2026 at 12:45:54AM +1000, Nicholas Piggin wrote:
> > riscv vector instructions can be interrupted with a trap, and partial
> > completion is recorded in the vstart register. Some causes are
> > implementation dependent, for example an asynchronous interrupt (which I
> > don't think TCG allows). Others are architectural, typically memory
> > access faults on vector load/store instructions.
> >
> > Add some TCG tests for interrupting vector load instructions and
> > resuming partially completed ones.
> >
> > This would have caught a recent (now reverted) regression in vector
> > stride load implementation, commit 28c12c1f2f50d ("Generate strided
> > vector loads/stores with tcg nodes.")
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > tests/tcg/riscv64/Makefile.target | 11 +
> > tests/tcg/riscv64/test-interrupted-v.c | 329 +++++++++++++++++++++++++
> > 2 files changed, 340 insertions(+)
> > create mode 100644 tests/tcg/riscv64/test-interrupted-v.c
> >
> > diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> > index 19a49b6467..b2b2325843 100644
> > --- a/tests/tcg/riscv64/Makefile.target
> > +++ b/tests/tcg/riscv64/Makefile.target
> > @@ -1,6 +1,10 @@
> > # -*- Mode: makefile -*-
> > # RISC-V specific tweaks
> >
> > +# Not all environments have compilers with vector intrinsics yet.
> > +HAVE_RISCV_VECTOR_INTRINSICS := $(shell echo '#ifndef __riscv_v_intrinsic\n#error\n#endif' | \
> > + $(CC) -march=rv64gcv -E -x c - >/dev/null 2>&1 && echo y)
> The feature probe relies on echo interpreting '\n'
> as newlines, but this is not portable.
>
> POSIX leaves echo's handling of backslash sequences
> implementation-defined.
>
> On bash (the default /bin/sh on many systems including
> Fedora, Arch, and some CI images), echo outputs literal
> '\n', so the preprocessor sees a malformed single-line input
> and always fails. This silently disables test-interrupted-v
> even when the compiler supports vector intrinsics.
>
> I suggest using printf for portability:
>
> HAVE_RISCV_VECTOR_INTRINSICS := $(shell printf \
> '#ifndef __riscv_v_intrinsic\n#error\n#endif\n' | \
> $(CC) -march=rv64gcv -E -x c - \
> >/dev/null 2>&1 && echo y)
Wow nice catch. My shell skills amount to pressing random buttons
until it works, so I appreciate the help :P
[...]
> > +static volatile int nr_segv;
> > +static volatile unsigned long fault_start, fault_end;
> > +static volatile bool fault_write;
> checkpatch errors:
>
> ERROR: Use of volatile is usually wrong, please add a comment
> #84: FILE: tests/tcg/riscv64/test-interrupted-v.c:26:
> +static volatile int nr_segv;
>
> ERROR: Use of volatile is usually wrong, please add a comment
> #85: FILE: tests/tcg/riscv64/test-interrupted-v.c:27:
> +static volatile unsigned long fault_start, fault_end;
>
> ERROR: Use of volatile is usually wrong, please add a comment
> #86: FILE: tests/tcg/riscv64/test-interrupted-v.c:28:
> +static volatile bool fault_write;
>
> Please fix it.
Yeah I guess they were missed in the rest of the checkpatch
errors, you're right the volatiles should be commented.
Thanks,
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-03-26 6:33 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 14:45 [PATCH v3 0/3] target/riscv: corner case fixes Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write Nicholas Piggin
2026-03-25 1:35 ` Alistair Francis
2026-03-25 3:08 ` Chao Liu
2026-03-25 3:26 ` Alistair Francis
2026-03-25 3:40 ` Chao Liu
2026-03-26 6:29 ` Nicholas Piggin
2026-03-21 14:45 ` [PATCH v3 2/3] target/riscv: Fix vector whole ldst vstart check Nicholas Piggin
2026-03-25 1:57 ` Alistair Francis
2026-03-25 2:10 ` Chao Liu
2026-03-21 14:45 ` [PATCH v3 3/3] tests/tcg: Add riscv test for interrupted vector ops Nicholas Piggin
2026-03-25 2:08 ` Alistair Francis
2026-03-25 3:19 ` Chao Liu
2026-03-26 6:32 ` Nicholas Piggin
2026-03-25 2:20 ` [PATCH v3 0/3] target/riscv: corner case fixes Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox