[PATCH 0/3] target/riscv: corner case fixes

[PATCH 0/3] target/riscv: corner case fixes

[Nicholas Piggin]

There is ongoing effort to run generated test verification on the 
QEMU riscv CPU which has turned out a few corner cases.

I added some fixes for these, as well as tcg tests. The
interrupted vector test also catches a bug in
"Generate strided vector loads/stores with tcg nodes." that
I referred to in the v5 thread for that series.

Thanks,
Nick

Nicholas Piggin (3):
  target/riscv: Fix IALIGN check in misa write
  target/risvc: 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         |  10 ++
 tests/tcg/riscv64/misa-ialign.S           |  88 +++++++++
 tests/tcg/riscv64/test-interrupted-v.c    | 208 ++++++++++++++++++++++
 tests/tcg/riscv64/test-vstart-overflow.c  |  75 ++++++++
 7 files changed, 401 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

[PATCH 1/3] target/riscv: Fix IALIGN check in misa write

[Nicholas Piggin]

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 8842e07a73..64b55b7add 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2140,9 +2140,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 3ca595335d..6e470a028f 100644
--- a/tests/tcg/riscv64/Makefile.softmmu-target
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -24,5 +24,10 @@ EXTRA_RUNS += run-test-mepc-masking
 run-test-mepc-masking: test-mepc-masking
 	$(call run-test, $<, $(QEMU) $(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

[PATCH 2/3] target/risvc: Fix vector whole ldst vstart check

[Nicholas Piggin]

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 | 75 ++++++++++++++++++++++++
 3 files changed, 82 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 fc85a34a84..e0e8735000 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..72999f2c8a
--- /dev/null
+++ b/tests/tcg/riscv64/test-vstart-overflow.c
@@ -0,0 +1,75 @@
+/*
+ * 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>
+#include <riscv_vector.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("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

[PATCH 3/3] tests/tcg: Add riscv test for interrupted vector ops

[Nicholas Piggin]

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      |   5 +
 tests/tcg/riscv64/test-interrupted-v.c | 208 +++++++++++++++++++++++++
 2 files changed, 213 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..8f4690ac57 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -23,3 +23,8 @@ 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
+
+# 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
diff --git a/tests/tcg/riscv64/test-interrupted-v.c b/tests/tcg/riscv64/test-interrupted-v.c
new file mode 100644
index 0000000000..db4fb6092f
--- /dev/null
+++ b/tests/tcg/riscv64/test-interrupted-v.c
@@ -0,0 +1,208 @@
+/*
+ * 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;
+
+/*
+ * 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);
+    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);
+}
+
+/* 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);
+
+    /* 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);
+
+    munmap(mem, NR_ELEMS * page_size);
+
+    return 0;
+}
+
+int main(void)
+{
+    return run_interrupted_v_tests();
+}
-- 
2.51.0

Re: [PATCH 1/3] target/riscv: Fix IALIGN check in misa write

[Daniel Henrique Barboza]

On 9/3/25 12:01 AM, 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>

Fixes: f18637cd61 ("RISC-V: Add misa runtime write support")

> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.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 8842e07a73..64b55b7add 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2140,9 +2140,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 3ca595335d..6e470a028f 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -24,5 +24,10 @@ EXTRA_RUNS += run-test-mepc-masking
>   run-test-mepc-masking: test-mepc-masking
>   	$(call run-test, $<, $(QEMU) $(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

Re: [PATCH 2/3] target/risvc: Fix vector whole ldst vstart check

[Daniel Henrique Barboza]

Hi Nick,

^ typo in the patch subject: s/risvc/riscv

On 9/3/25 12:01 AM, 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.
> 
> 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 | 75 ++++++++++++++++++++++++
>   3 files changed, 82 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 fc85a34a84..e0e8735000 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..72999f2c8a
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-vstart-overflow.c
> @@ -0,0 +1,75 @@
> +/*
> + * 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>
> +#include <riscv_vector.h>

The fix in vector_helper.c is fine but this patch (and patch 3) won't execute
'make check-tcg'. It complains about this header being missing in the docker
env.

To eliminate the possibility of my env being the problem I ran this series in
Gitlab. Same error:


https://gitlab.com/danielhb/qemu/-/jobs/11236091281

/builds/danielhb/qemu/tests/tcg/riscv64/test-vstart-overflow.c:13:10: fatal error: riscv_vector.h: No such file or directory
3899
    13 | #include <riscv_vector.h>
3900
       |          ^~~~~~~~~~~~~~~~
3901
compilation terminated.
3902
make[1]: *** [Makefile:122: test-vstart-overflow] Error 1


I believe you need to add the Docker changes you made in this patch. Same
thing for patch 3. And same thing for patch 4 of:

[PATCH 0/4] linux-user/riscv: add vector state to signal

Given that you're also using riscv_vector.h in there too. Thanks,


Daniel



> +
> +#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("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();
> +}

Re: [PATCH 2/3] target/risvc: Fix vector whole ldst vstart check

[Nicholas Piggin]

On Wed, Sep 03, 2025 at 05:13:36PM -0300, Daniel Henrique Barboza wrote:
> Hi Nick,
> 
> ^ typo in the patch subject: s/risvc/riscv

Well I'm off to a fine start :/

> 
> On 9/3/25 12:01 AM, 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.
> > 
> > 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 | 75 ++++++++++++++++++++++++
> >   3 files changed, 82 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 fc85a34a84..e0e8735000 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..72999f2c8a
> > --- /dev/null
> > +++ b/tests/tcg/riscv64/test-vstart-overflow.c
> > @@ -0,0 +1,75 @@
> > +/*
> > + * 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>
> > +#include <riscv_vector.h>
> 
> The fix in vector_helper.c is fine but this patch (and patch 3) won't execute
> 'make check-tcg'. It complains about this header being missing in the docker
> env.
> 
> To eliminate the possibility of my env being the problem I ran this series in
> Gitlab. Same error:
> 
> 
> https://gitlab.com/danielhb/qemu/-/jobs/11236091281
> 
> /builds/danielhb/qemu/tests/tcg/riscv64/test-vstart-overflow.c:13:10: fatal error: riscv_vector.h: No such file or directory
> 3899
>    13 | #include <riscv_vector.h>
> 3900
>       |          ^~~~~~~~~~~~~~~~
> 3901
> compilation terminated.
> 3902
> make[1]: *** [Makefile:122: test-vstart-overflow] Error 1
> 
> 
> I believe you need to add the Docker changes you made in this patch. Same
> thing for patch 3. And same thing for patch 4 of:
> 
> [PATCH 0/4] linux-user/riscv: add vector state to signal
> 
> Given that you're also using riscv_vector.h in there too. Thanks,

Hmm, thanks. It did work for my local build.

I think the header is provided by the compiler, so I might have
to work out a way to skip the test if the compiler is too old.
GCC13 might have been the first one to support.

I was considering writing .S files for these. Should have done so
if I realized, but nevermind.

Thanks,
Nick

Re: [PATCH 2/3] target/risvc: Fix vector whole ldst vstart check

[Daniel Henrique Barboza]

Alex, Richard,

On 9/4/25 2:16 AM, Nicholas Piggin wrote:
> On Wed, Sep 03, 2025 at 05:13:36PM -0300, Daniel Henrique Barboza wrote:
>> Hi Nick,
>>
>> ^ typo in the patch subject: s/risvc/riscv
> 
> Well I'm off to a fine start :/
> 
>>
>> On 9/3/25 12:01 AM, 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.
>>>
>>> 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 | 75 ++++++++++++++++++++++++
>>>    3 files changed, 82 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 fc85a34a84..e0e8735000 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..72999f2c8a
>>> --- /dev/null
>>> +++ b/tests/tcg/riscv64/test-vstart-overflow.c
>>> @@ -0,0 +1,75 @@
>>> +/*
>>> + * 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>
>>> +#include <riscv_vector.h>
>>
>> The fix in vector_helper.c is fine but this patch (and patch 3) won't execute
>> 'make check-tcg'. It complains about this header being missing in the docker
>> env.
>>
>> To eliminate the possibility of my env being the problem I ran this series in
>> Gitlab. Same error:
>>
>>
>> https://gitlab.com/danielhb/qemu/-/jobs/11236091281
>>
>> /builds/danielhb/qemu/tests/tcg/riscv64/test-vstart-overflow.c:13:10: fatal error: riscv_vector.h: No such file or directory
>> 3899
>>     13 | #include <riscv_vector.h>
>> 3900
>>        |          ^~~~~~~~~~~~~~~~
>> 3901
>> compilation terminated.
>> 3902
>> make[1]: *** [Makefile:122: test-vstart-overflow] Error 1
>>
>>
>> I believe you need to add the Docker changes you made in this patch. Same
>> thing for patch 3. And same thing for patch 4 of:
>>
>> [PATCH 0/4] linux-user/riscv: add vector state to signal
>>
>> Given that you're also using riscv_vector.h in there too. Thanks,
> 
> Hmm, thanks. It did work for my local build.
> 
> I think the header is provided by the compiler, so I might have
> to work out a way to skip the test if the compiler is too old.
> GCC13 might have been the first one to support.

How hard it is to update the GCC version we're running in the docker images for
"check-tcg"? We would like to use a RISC-V vector header that isn't supported
ATM.


Thanks,

Daniel


> 
> I was considering writing .S files for these. Should have done so
> if I realized, but nevermind.
> 
> Thanks,
> Nick