* [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
* 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 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 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
* [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
* 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 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
* [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 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 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 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
* 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
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