* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.