* [PATCH 0/4] linux-user/riscv: add vector state to signal
@ 2025-09-03 4:25 Nicholas Piggin
2025-09-03 4:25 ` [PATCH 1/4] tests/tcg/riscv64: Add a user signal handling test Nicholas Piggin
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Nicholas Piggin @ 2025-09-03 4:25 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel
This series adds vector state to the linux-user signal handler,
and adds a basic signal handling test case. As a sanity check, I
also verified the signal handling test works in the same way when
run under a real Linux kernel.
The signal handler test has some gross header hacks in it to make
it work for me (debian arm64->riscv64 cross compile environment),
I would not be surprised if it breaks in other environments, any
ideas or breakages let me know. May just have to define the types
by hand for now if it becomes intractable.
I couldn't find much in the way of previous discussion or work on
this, forgive me if I've missed it.
Thanks,
Nick
Nicholas Piggin (4):
tests/tcg/riscv64: Add a user signal handling test
linux-user/riscv: Add extended state to sigcontext
linux-user/riscv: Add vector state to signal context
tests/tcg/riscv64: Add vector state to signal test
linux-user/riscv/signal.c | 195 ++++++++-
linux-user/riscv/vdso-asmoffset.h | 2 +-
tests/tcg/riscv64/Makefile.target | 5 +
tests/tcg/riscv64/test-signal-handling.c | 506 +++++++++++++++++++++++
4 files changed, 696 insertions(+), 12 deletions(-)
create mode 100644 tests/tcg/riscv64/test-signal-handling.c
--
2.51.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] tests/tcg/riscv64: Add a user signal handling test
2025-09-03 4:25 [PATCH 0/4] linux-user/riscv: add vector state to signal Nicholas Piggin
@ 2025-09-03 4:25 ` Nicholas Piggin
2025-09-03 4:25 ` [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2025-09-03 4:25 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel
Add a few basic signal handling tests for user emulation.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/tcg/riscv64/Makefile.target | 5 +
tests/tcg/riscv64/test-signal-handling.c | 303 +++++++++++++++++++++++
2 files changed, 308 insertions(+)
create mode 100644 tests/tcg/riscv64/test-signal-handling.c
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index 8f4690ac57..0c89c46c4f 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -5,6 +5,11 @@ VPATH += $(SRC_PATH)/tests/tcg/riscv64
TESTS += test-div
TESTS += noexec
+# Test signal handling.
+TESTS += test-signal-handling
+test-signal-handling: CFLAGS += -march=rv64gcv
+run-test-signal-handling: QEMU_OPTS += -cpu rv64,v=on
+
# Disable compressed instructions for test-noc
TESTS += test-noc
test-noc: LDFLAGS = -nostdlib -static
diff --git a/tests/tcg/riscv64/test-signal-handling.c b/tests/tcg/riscv64/test-signal-handling.c
new file mode 100644
index 0000000000..e9c0170c74
--- /dev/null
+++ b/tests/tcg/riscv64/test-signal-handling.c
@@ -0,0 +1,303 @@
+/*
+ * Test for linux-user signal handling.
+ *
+ * This ensures that integer and fp register values are
+ * saved as expected in the sigcontext, created by a SIGILL.
+ *
+ * TODO: Register restore is not explicitly verified, except
+ * for advancing pc, and the restoring of registers that were
+ * clobbered by the compiler in the signal handler.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <execinfo.h>
+#include <unistd.h>
+#include <assert.h>
+#include <sys/mman.h>
+#include <ucontext.h>
+#include <asm/sigcontext.h>
+
+/*
+ * This horrible hack seems to be required when including
+ * signal.h and asm/sigcontext.h, to prevent sigcontext
+ * redefinition by bits/sigcontext.h :(
+ *
+ * bits/sigcontext.h does not have the extended state or
+ * RISCV_V_MAGIC, etc. It could have just been introduced
+ * as a new type.
+ */
+#define _BITS_SIGCONTEXT_H 1
+#include <signal.h>
+
+static uint64_t *initial_gvalues;
+static uint64_t *final_gvalues;
+static uint64_t *signal_gvalues;
+static double *initial_fvalues;
+static double *final_fvalues;
+static double *signal_fvalues;
+
+extern unsigned long unimp_addr[];
+
+static bool got_signal = false;
+
+#define BT_BUF_SIZE 100
+
+static void *find_callchain_root(void)
+{
+ int nptrs;
+ void *buffer[BT_BUF_SIZE];
+
+ nptrs = backtrace(buffer, BT_BUF_SIZE);
+
+ return buffer[nptrs - 1];
+}
+
+static void *callchain_root;
+
+static void ILL_handler(int signo, siginfo_t *info, void *context)
+{
+ ucontext_t *uc = context;
+ struct sigcontext *sc = (struct sigcontext *)&uc->uc_mcontext;
+
+ got_signal = true;
+
+ assert(unimp_addr == info->si_addr);
+ assert(sc->sc_regs.pc == (unsigned long)info->si_addr);
+
+ /* Ensure stack unwind through the signal frame is not broken */
+ assert(callchain_root == find_callchain_root());
+
+ for (int i = 0; i < 31; i++) {
+ ((uint64_t *)signal_gvalues)[i] = ((unsigned long *)&sc->sc_regs.ra)[i];
+ }
+
+ for (int i = 0; i < 32; i++) {
+ ((uint64_t *)signal_fvalues)[i] = sc->sc_fpregs.d.f[i];
+ }
+ /* Test sc->sc_fpregs.d.fcsr ? */
+
+ sc->sc_regs.pc += 4;
+}
+
+static void init_test(void)
+{
+ int i;
+
+ callchain_root = find_callchain_root();
+
+ initial_gvalues = malloc(8 * 31);
+ memset(initial_gvalues, 0, 8 * 31);
+ final_gvalues = malloc(8 * 31);
+ memset(final_gvalues, 0, 8 * 31);
+ signal_gvalues = malloc(8 * 31);
+ memset(signal_gvalues, 0, 8 * 31);
+
+ initial_fvalues = malloc(8 * 32);
+ memset(initial_fvalues, 0, 8 * 32);
+ for (i = 0; i < 32 ; i++) {
+ initial_fvalues[i] = 3.142 * (i + 1);
+ }
+ final_fvalues = malloc(8 * 32);
+ memset(final_fvalues, 0, 8 * 32);
+ signal_fvalues = malloc(8 * 32);
+ memset(signal_fvalues, 0, 8 * 32);
+}
+
+static void run_test(void)
+{
+ asm volatile(
+ /* Save initial values from gp registers */
+" mv t0, %[initial_gvalues] \n"
+" sd x1, 0x0(t0) \n"
+" sd x2, 0x8(t0) \n"
+" sd x3, 0x10(t0) \n"
+" sd x4, 0x18(t0) \n"
+" sd x5, 0x20(t0) \n"
+" sd x6, 0x28(t0) \n"
+" sd x7, 0x30(t0) \n"
+" sd x8, 0x38(t0) \n"
+" sd x9, 0x40(t0) \n"
+" sd x10, 0x48(t0) \n"
+" sd x11, 0x50(t0) \n"
+" sd x12, 0x58(t0) \n"
+" sd x13, 0x60(t0) \n"
+" sd x14, 0x68(t0) \n"
+" sd x15, 0x70(t0) \n"
+" sd x16, 0x78(t0) \n"
+" sd x17, 0x80(t0) \n"
+" sd x18, 0x88(t0) \n"
+" sd x19, 0x90(t0) \n"
+" sd x20, 0x98(t0) \n"
+" sd x21, 0xa0(t0) \n"
+" sd x22, 0xa8(t0) \n"
+" sd x23, 0xb0(t0) \n"
+" sd x24, 0xb8(t0) \n"
+" sd x25, 0xc0(t0) \n"
+" sd x26, 0xc8(t0) \n"
+" sd x27, 0xd0(t0) \n"
+" sd x28, 0xd8(t0) \n"
+" sd x29, 0xe0(t0) \n"
+" sd x30, 0xe8(t0) \n"
+" sd x31, 0xf0(t0) \n"
+ /* Load initial values into float registers */
+" mv t0, %[initial_fvalues] \n"
+" fld f0, 0x0(t0) \n"
+" fld f1, 0x8(t0) \n"
+" fld f2, 0x10(t0) \n"
+" fld f3, 0x18(t0) \n"
+" fld f4, 0x20(t0) \n"
+" fld f5, 0x28(t0) \n"
+" fld f6, 0x30(t0) \n"
+" fld f7, 0x38(t0) \n"
+" fld f8, 0x40(t0) \n"
+" fld f9, 0x48(t0) \n"
+" fld f10, 0x50(t0) \n"
+" fld f11, 0x58(t0) \n"
+" fld f12, 0x60(t0) \n"
+" fld f13, 0x68(t0) \n"
+" fld f14, 0x70(t0) \n"
+" fld f15, 0x78(t0) \n"
+" fld f16, 0x80(t0) \n"
+" fld f17, 0x88(t0) \n"
+" fld f18, 0x90(t0) \n"
+" fld f19, 0x98(t0) \n"
+" fld f20, 0xa0(t0) \n"
+" fld f21, 0xa8(t0) \n"
+" fld f22, 0xb0(t0) \n"
+" fld f23, 0xb8(t0) \n"
+" fld f24, 0xc0(t0) \n"
+" fld f25, 0xc8(t0) \n"
+" fld f26, 0xd0(t0) \n"
+" fld f27, 0xd8(t0) \n"
+" fld f28, 0xe0(t0) \n"
+" fld f29, 0xe8(t0) \n"
+" fld f30, 0xf0(t0) \n"
+" fld f31, 0xf8(t0) \n"
+ /* Trigger the SIGILL */
+".global unimp_addr \n"
+"unimp_addr: \n"
+" unimp \n"
+" nop \n"
+ /* Save final values from gp registers */
+" mv t0, %[final_gvalues] \n"
+" sd x1, 0x0(t0) \n"
+" sd x2, 0x8(t0) \n"
+" sd x3, 0x10(t0) \n"
+" sd x4, 0x18(t0) \n"
+" sd x5, 0x20(t0) \n"
+" sd x6, 0x28(t0) \n"
+" sd x7, 0x30(t0) \n"
+" sd x8, 0x38(t0) \n"
+" sd x9, 0x40(t0) \n"
+" sd x10, 0x48(t0) \n"
+" sd x11, 0x50(t0) \n"
+" sd x12, 0x58(t0) \n"
+" sd x13, 0x60(t0) \n"
+" sd x14, 0x68(t0) \n"
+" sd x15, 0x70(t0) \n"
+" sd x16, 0x78(t0) \n"
+" sd x17, 0x80(t0) \n"
+" sd x18, 0x88(t0) \n"
+" sd x19, 0x90(t0) \n"
+" sd x20, 0x98(t0) \n"
+" sd x21, 0xa0(t0) \n"
+" sd x22, 0xa8(t0) \n"
+" sd x23, 0xb0(t0) \n"
+" sd x24, 0xb8(t0) \n"
+" sd x25, 0xc0(t0) \n"
+" sd x26, 0xc8(t0) \n"
+" sd x27, 0xd0(t0) \n"
+" sd x28, 0xd8(t0) \n"
+" sd x29, 0xe0(t0) \n"
+" sd x30, 0xe8(t0) \n"
+" sd x31, 0xf0(t0) \n"
+ /* Save final values from float registers */
+" mv t0, %[final_fvalues] \n"
+" fsd f0, 0x0(t0) \n"
+" fsd f1, 0x8(t0) \n"
+" fsd f2, 0x10(t0) \n"
+" fsd f3, 0x18(t0) \n"
+" fsd f4, 0x20(t0) \n"
+" fsd f5, 0x28(t0) \n"
+" fsd f6, 0x30(t0) \n"
+" fsd f7, 0x38(t0) \n"
+" fsd f8, 0x40(t0) \n"
+" fsd f9, 0x48(t0) \n"
+" fsd f10, 0x50(t0) \n"
+" fsd f11, 0x58(t0) \n"
+" fsd f12, 0x60(t0) \n"
+" fsd f13, 0x68(t0) \n"
+" fsd f14, 0x70(t0) \n"
+" fsd f15, 0x78(t0) \n"
+" fsd f16, 0x80(t0) \n"
+" fsd f17, 0x88(t0) \n"
+" fsd f18, 0x90(t0) \n"
+" fsd f19, 0x98(t0) \n"
+" fsd f20, 0xa0(t0) \n"
+" fsd f21, 0xa8(t0) \n"
+" fsd f22, 0xb0(t0) \n"
+" fsd f23, 0xb8(t0) \n"
+" fsd f24, 0xc0(t0) \n"
+" fsd f25, 0xc8(t0) \n"
+" fsd f26, 0xd0(t0) \n"
+" fsd f27, 0xd8(t0) \n"
+" fsd f28, 0xe0(t0) \n"
+" fsd f29, 0xe8(t0) \n"
+" fsd f30, 0xf0(t0) \n"
+" fsd f31, 0xf8(t0) \n"
+ : "=m" (initial_gvalues),
+ "=m" (final_gvalues),
+ "=m" (final_fvalues)
+ : "m" (initial_fvalues),
+ [initial_gvalues] "r" (initial_gvalues),
+ [initial_fvalues] "r" (initial_fvalues),
+ [final_gvalues] "r" (final_gvalues),
+ [final_fvalues] "r" (final_fvalues)
+ : "t0",
+ "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
+ "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
+ "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
+ "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31");
+
+ assert(got_signal);
+
+ /*
+ * x4 / t0 is used in the asm so it has to be handled specially
+ * and is not a simple equality.
+ */
+ assert(initial_gvalues[4] == (unsigned long)initial_gvalues);
+ assert(signal_gvalues[4] == (unsigned long)initial_fvalues);
+ assert(final_gvalues[4] == (unsigned long)final_gvalues);
+ initial_gvalues[4] = final_gvalues[4] = signal_gvalues[4] = 0;
+
+ /*
+ * Ensure registers match before, inside, and after signal
+ * handler.
+ */
+ assert(!memcmp(initial_gvalues, final_gvalues, 8 * 31));
+ assert(!memcmp(initial_gvalues, signal_gvalues, 8 * 31));
+ assert(!memcmp(initial_fvalues, final_fvalues, 8 * 32));
+ assert(!memcmp(initial_fvalues, signal_fvalues, 8 * 32));
+}
+
+int main(void)
+{
+ struct sigaction act = { 0 };
+
+ act.sa_flags = SA_SIGINFO;
+ act.sa_sigaction = &ILL_handler;
+ if (sigaction(SIGILL, &act, NULL) == -1) {
+ perror("sigaction");
+ exit(EXIT_FAILURE);
+ }
+
+ init_test();
+
+ run_test();
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext
2025-09-03 4:25 [PATCH 0/4] linux-user/riscv: add vector state to signal Nicholas Piggin
2025-09-03 4:25 ` [PATCH 1/4] tests/tcg/riscv64: Add a user signal handling test Nicholas Piggin
@ 2025-09-03 4:25 ` Nicholas Piggin
2025-09-03 5:31 ` Richard Henderson
2025-09-03 4:25 ` [PATCH 3/4] linux-user/riscv: Add vector state to signal context Nicholas Piggin
2025-09-03 4:25 ` [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test Nicholas Piggin
3 siblings, 1 reply; 8+ messages in thread
From: Nicholas Piggin @ 2025-09-03 4:25 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel
Linux/riscv has extended the sigcontext with padding and an
extended state structure that can save various optional
features like vector in a flexible format. Update the
linux-user signal handling to this new structure.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
linux-user/riscv/signal.c | 71 ++++++++++++++++++++++++++-----
linux-user/riscv/vdso-asmoffset.h | 2 +-
2 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 358fa1d82d..4ef55d0848 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -31,14 +31,43 @@
The code below is qemu re-implementation of arch/riscv/kernel/signal.c */
-struct target_sigcontext {
+struct target_gp_state {
abi_long pc;
abi_long gpr[31]; /* x0 is not present, so all offsets must be -1 */
+};
+
+struct target_fp_state {
uint64_t fpr[32];
uint32_t fcsr;
+};
+
+/* The Magic number for signal context frame header. */
+#define END_MAGIC 0x0
+
+/* The size of END signal context header. */
+#define END_HDR_SIZE 0x0
+
+struct target_ctx_hdr {
+ uint32_t magic;
+ uint32_t size;
+};
+
+struct target_extra_ext_header {
+ uint32_t __padding[129] __attribute__((aligned(16)));
+ uint32_t reserved;
+ struct target_ctx_hdr hdr;
+};
+
+struct target_sigcontext {
+ struct target_gp_state sc_regs;
+ union {
+ struct target_fp_state sc_fpregs;
+ struct target_extra_ext_header sc_extdesc;
+ };
}; /* cf. riscv-linux:arch/riscv/include/uapi/asm/ptrace.h */
-QEMU_BUILD_BUG_ON(offsetof(struct target_sigcontext, fpr) != offsetof_freg0);
+QEMU_BUILD_BUG_ON(offsetof(struct target_sigcontext, sc_fpregs.fpr) !=
+ offsetof_freg0);
struct target_ucontext {
abi_ulong uc_flags;
@@ -79,19 +108,25 @@ static abi_ulong get_sigframe(struct target_sigaction *ka,
static void setup_sigcontext(struct target_sigcontext *sc, CPURISCVState *env)
{
+ struct target_ctx_hdr *hdr;
int i;
- __put_user(env->pc, &sc->pc);
+ __put_user(env->pc, &sc->sc_regs.pc);
for (i = 1; i < 32; i++) {
- __put_user(env->gpr[i], &sc->gpr[i - 1]);
+ __put_user(env->gpr[i], &sc->sc_regs.gpr[i - 1]);
}
for (i = 0; i < 32; i++) {
- __put_user(env->fpr[i], &sc->fpr[i]);
+ __put_user(env->fpr[i], &sc->sc_fpregs.fpr[i]);
}
uint32_t fcsr = riscv_csr_read(env, CSR_FCSR);
- __put_user(fcsr, &sc->fcsr);
+ __put_user(fcsr, &sc->sc_fpregs.fcsr);
+
+ __put_user(0, &sc->sc_extdesc.reserved);
+ hdr = &sc->sc_extdesc.hdr;
+ __put_user(END_MAGIC, &hdr->magic);
+ __put_user(END_HDR_SIZE, &hdr->size);
}
static void setup_ucontext(struct target_ucontext *uc,
@@ -146,20 +181,36 @@ badframe:
static void restore_sigcontext(CPURISCVState *env, struct target_sigcontext *sc)
{
+ struct target_ctx_hdr *hdr;
int i;
- __get_user(env->pc, &sc->pc);
+ __get_user(env->pc, &sc->sc_regs.pc);
for (i = 1; i < 32; ++i) {
- __get_user(env->gpr[i], &sc->gpr[i - 1]);
+ __get_user(env->gpr[i], &sc->sc_regs.gpr[i - 1]);
}
for (i = 0; i < 32; ++i) {
- __get_user(env->fpr[i], &sc->fpr[i]);
+ __get_user(env->fpr[i], &sc->sc_fpregs.fpr[i]);
}
uint32_t fcsr;
- __get_user(fcsr, &sc->fcsr);
+ __get_user(fcsr, &sc->sc_fpregs.fcsr);
riscv_csr_write(env, CSR_FCSR, fcsr);
+
+ hdr = &sc->sc_extdesc.hdr;
+ uint32_t rsv;
+ __get_user(rsv, &sc->sc_extdesc.reserved);
+ if (rsv != 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "signal: sigcontext reserved field is "
+ "non-zero. Attempting restore anyway.");
+ }
+
+ uint32_t magic;
+ __get_user(magic, &hdr->magic);
+ if (magic != END_MAGIC) {
+ qemu_log_mask(LOG_UNIMP, "signal: unknown extended context header: "
+ "0x%08x, ignoring", magic);
+ }
}
static void restore_ucontext(CPURISCVState *env, struct target_ucontext *uc)
diff --git a/linux-user/riscv/vdso-asmoffset.h b/linux-user/riscv/vdso-asmoffset.h
index 123902ef61..7d14228fb3 100644
--- a/linux-user/riscv/vdso-asmoffset.h
+++ b/linux-user/riscv/vdso-asmoffset.h
@@ -3,7 +3,7 @@
# define offsetof_uc_mcontext 0x120
# define offsetof_freg0 0x80
#else
-# define sizeof_rt_sigframe 0x340
+# define sizeof_rt_sigframe 0x440
# define offsetof_uc_mcontext 0x130
# define offsetof_freg0 0x100
#endif
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] linux-user/riscv: Add vector state to signal context
2025-09-03 4:25 [PATCH 0/4] linux-user/riscv: add vector state to signal Nicholas Piggin
2025-09-03 4:25 ` [PATCH 1/4] tests/tcg/riscv64: Add a user signal handling test Nicholas Piggin
2025-09-03 4:25 ` [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext Nicholas Piggin
@ 2025-09-03 4:25 ` Nicholas Piggin
2025-09-03 9:14 ` Richard Henderson
2025-09-03 4:25 ` [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test Nicholas Piggin
3 siblings, 1 reply; 8+ messages in thread
From: Nicholas Piggin @ 2025-09-03 4:25 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel
This enables vector state to be saved and restored across signals.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
linux-user/riscv/signal.c | 130 ++++++++++++++++++++++++++++++++++++--
1 file changed, 126 insertions(+), 4 deletions(-)
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 4ef55d0848..4acbabcbc9 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -41,7 +41,17 @@ struct target_fp_state {
uint32_t fcsr;
};
+struct target_v_ext_state {
+ target_ulong vstart;
+ target_ulong vl;
+ target_ulong vtype;
+ target_ulong vcsr;
+ target_ulong vlenb;
+ target_ulong datap;
+} __attribute__((aligned(16)));
+
/* The Magic number for signal context frame header. */
+#define RISCV_V_MAGIC 0x53465457
#define END_MAGIC 0x0
/* The size of END signal context header. */
@@ -106,6 +116,88 @@ static abi_ulong get_sigframe(struct target_sigaction *ka,
return sp;
}
+static unsigned int get_v_state_size(CPURISCVState *env)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ return sizeof(struct target_ctx_hdr) +
+ sizeof(struct target_v_ext_state) +
+ cpu->cfg.vlenb * 32;
+}
+
+static struct target_ctx_hdr *save_v_state(CPURISCVState *env,
+ struct target_ctx_hdr *hdr)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+ target_ulong vlenb = cpu->cfg.vlenb;
+ uint32_t riscv_v_sc_size = get_v_state_size(env);
+ struct target_v_ext_state *vs;
+ uint64_t *vdatap;
+ int i;
+
+ __put_user(RISCV_V_MAGIC, &hdr->magic);
+ __put_user(riscv_v_sc_size, &hdr->size);
+
+ vs = (struct target_v_ext_state *)(hdr + 1);
+ vdatap = (uint64_t *)(vs + 1);
+
+ __put_user(env->vstart, &vs->vstart);
+ __put_user(env->vl, &vs->vl);
+ __put_user(env->vtype, &vs->vtype);
+ target_ulong vcsr = riscv_csr_read(env, CSR_VCSR);
+ __put_user(vcsr, &vs->vcsr);
+ __put_user(vlenb, &vs->vlenb);
+ __put_user((target_ulong)vdatap, &vs->datap);
+
+ for (i = 0; i < 32; i++) {
+ int j;
+ for (j = 0; j < vlenb; j += 8) {
+ size_t idx = (i * vlenb + j) / 8;
+ __put_user(env->vreg[idx], vdatap + idx);
+ }
+ }
+
+ return (void *)hdr + riscv_v_sc_size;
+}
+
+static void restore_v_state(CPURISCVState *env,
+ struct target_ctx_hdr *hdr)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+ target_ulong vlenb = cpu->cfg.vlenb;
+ struct target_v_ext_state *vs;
+ uint64_t *vdatap;
+ int i;
+
+ uint32_t size;
+ __get_user(size, &hdr->size);
+ if (size != get_v_state_size(env)) {
+ g_assert_not_reached();
+ /* XXX: warn, bail */
+ }
+
+ vs = (struct target_v_ext_state *)(hdr + 1);
+
+ __get_user(env->vstart, &vs->vstart);
+ __get_user(env->vl, &vs->vl);
+ __get_user(env->vtype, &vs->vtype);
+ target_ulong vcsr;
+ __get_user(vcsr, &vs->vcsr);
+ riscv_csr_write(env, CSR_VCSR, vcsr);
+ __get_user(vlenb, &vs->vlenb);
+ target_ulong __vdatap;
+ __get_user(__vdatap, &vs->datap);
+ vdatap = (uint64_t *)__vdatap;
+
+ for (i = 0; i < 32; i++) {
+ int j;
+ for (j = 0; j < vlenb; j += 8) {
+ size_t idx = (i * vlenb + j) / 8;
+ __get_user(env->vreg[idx], vdatap + idx);
+ }
+ }
+}
+
static void setup_sigcontext(struct target_sigcontext *sc, CPURISCVState *env)
{
struct target_ctx_hdr *hdr;
@@ -124,7 +216,11 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPURISCVState *env)
__put_user(fcsr, &sc->sc_fpregs.fcsr);
__put_user(0, &sc->sc_extdesc.reserved);
+
hdr = &sc->sc_extdesc.hdr;
+ if (riscv_has_ext(env, RVV)) {
+ hdr = save_v_state(env, hdr);
+ }
__put_user(END_MAGIC, &hdr->magic);
__put_user(END_HDR_SIZE, &hdr->size);
}
@@ -151,8 +247,13 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
{
abi_ulong frame_addr;
struct target_rt_sigframe *frame;
+ size_t frame_size = sizeof(*frame);
- frame_addr = get_sigframe(ka, env, sizeof(*frame));
+ if (riscv_has_ext(env, RVV)) {
+ frame_size += get_v_state_size(env);
+ }
+
+ frame_addr = get_sigframe(ka, env, frame_size);
trace_user_setup_rt_frame(env, frame_addr);
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
@@ -207,9 +308,30 @@ static void restore_sigcontext(CPURISCVState *env, struct target_sigcontext *sc)
uint32_t magic;
__get_user(magic, &hdr->magic);
- if (magic != END_MAGIC) {
- qemu_log_mask(LOG_UNIMP, "signal: unknown extended context header: "
- "0x%08x, ignoring", magic);
+ while (magic != END_MAGIC) {
+ if (magic == RISCV_V_MAGIC) {
+ if (riscv_has_ext(env, RVV)) {
+ restore_v_state(env, hdr);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "signal: sigcontext has V state "
+ "but CPU does not.");
+ }
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "signal: unknown extended state in "
+ "sigcontext magic=0x%08x", magic);
+ }
+
+ if (hdr->size == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "signal: extended state in sigcontext "
+ "has size 0");
+ }
+ hdr = (void *)hdr + hdr->size;
+ __get_user(magic, &hdr->magic);
+ }
+
+ if (hdr->size != END_HDR_SIZE) {
+ qemu_log_mask(LOG_GUEST_ERROR, "signal: extended state end header has "
+ "size=%u (should be 0)", hdr->size);
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test
2025-09-03 4:25 [PATCH 0/4] linux-user/riscv: add vector state to signal Nicholas Piggin
` (2 preceding siblings ...)
2025-09-03 4:25 ` [PATCH 3/4] linux-user/riscv: Add vector state to signal context Nicholas Piggin
@ 2025-09-03 4:25 ` Nicholas Piggin
2025-09-03 20:15 ` Daniel Henrique Barboza
3 siblings, 1 reply; 8+ messages in thread
From: Nicholas Piggin @ 2025-09-03 4:25 UTC (permalink / raw)
To: qemu-riscv
Cc: Nicholas Piggin, Laurent Vivier, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-devel
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/tcg/riscv64/test-signal-handling.c | 215 ++++++++++++++++++++++-
1 file changed, 209 insertions(+), 6 deletions(-)
diff --git a/tests/tcg/riscv64/test-signal-handling.c b/tests/tcg/riscv64/test-signal-handling.c
index e9c0170c74..29b2fe169d 100644
--- a/tests/tcg/riscv64/test-signal-handling.c
+++ b/tests/tcg/riscv64/test-signal-handling.c
@@ -19,9 +19,16 @@
#include <execinfo.h>
#include <unistd.h>
#include <assert.h>
+#include <sys/auxv.h>
+#include <elf.h>
#include <sys/mman.h>
#include <ucontext.h>
#include <asm/sigcontext.h>
+#include "riscv_vector.h"
+
+#ifndef COMPAT_HWCAP_ISA_V
+#define COMPAT_HWCAP_ISA_V (1 << ('V' - 'A'))
+#endif
/*
* This horrible hack seems to be required when including
@@ -41,6 +48,10 @@ static uint64_t *signal_gvalues;
static double *initial_fvalues;
static double *final_fvalues;
static double *signal_fvalues;
+static size_t vlenb;
+static uint8_t *initial_vvalues;
+static uint8_t *final_vvalues;
+static uint8_t *signal_vvalues;
extern unsigned long unimp_addr[];
@@ -64,6 +75,8 @@ static void ILL_handler(int signo, siginfo_t *info, void *context)
{
ucontext_t *uc = context;
struct sigcontext *sc = (struct sigcontext *)&uc->uc_mcontext;
+ struct __riscv_ctx_hdr *sc_ext = &sc->sc_extdesc.hdr;
+ bool found_v = false;
got_signal = true;
@@ -82,12 +95,47 @@ static void ILL_handler(int signo, siginfo_t *info, void *context)
}
/* Test sc->sc_fpregs.d.fcsr ? */
+ assert(sc->sc_extdesc.reserved == 0);
+ while (sc_ext->magic != END_MAGIC) {
+ assert(sc_ext->size != 0);
+
+ if (sc_ext->magic == RISCV_V_MAGIC) {
+ struct __sc_riscv_v_state *sc_v_state = (struct __sc_riscv_v_state *)(sc_ext + 1);
+ struct __riscv_v_ext_state *v_state = &sc_v_state->v_state;
+
+ found_v = true;
+
+ assert(getauxval(AT_HWCAP) & COMPAT_HWCAP_ISA_V);
+
+ assert(v_state->vlenb == vlenb);
+ assert(v_state->vtype == 0xc0); /* vma, vta */
+ assert(v_state->vl == vlenb);
+ assert(v_state->vstart == 0);
+ assert(v_state->vcsr == 0);
+
+ uint64_t *vregs = v_state->datap;
+ for (int i = 0; i < 32; i++) {
+ for (int j = 0; j < vlenb; j += 8) {
+ size_t idx = (i * vlenb + j) / 8;
+ ((uint64_t *)signal_vvalues)[idx] = vregs[idx];
+ }
+ }
+ }
+
+ sc_ext = (void *)sc_ext + sc_ext->size;
+ }
+
+ assert(sc_ext->size == 0);
+ if (getauxval(AT_HWCAP) & COMPAT_HWCAP_ISA_V) {
+ assert(found_v);
+ }
+
sc->sc_regs.pc += 4;
}
static void init_test(void)
{
- int i;
+ int i, j;
callchain_root = find_callchain_root();
@@ -107,6 +155,19 @@ static void init_test(void)
memset(final_fvalues, 0, 8 * 32);
signal_fvalues = malloc(8 * 32);
memset(signal_fvalues, 0, 8 * 32);
+
+ vlenb = __riscv_vlenb();
+ initial_vvalues = malloc(vlenb * 32);
+ memset(initial_vvalues, 0, vlenb * 32);
+ for (i = 0; i < 32 ; i++) {
+ for (j = 0; j < vlenb; j++) {
+ initial_vvalues[i * vlenb + j] = i * vlenb + j;
+ }
+ }
+ final_vvalues = malloc(vlenb * 32);
+ memset(final_vvalues, 0, vlenb * 32);
+ signal_vvalues = malloc(vlenb * 32);
+ memset(signal_vvalues, 0, vlenb * 32);
}
static void run_test(void)
@@ -179,6 +240,72 @@ static void run_test(void)
" fld f29, 0xe8(t0) \n"
" fld f30, 0xf0(t0) \n"
" fld f31, 0xf8(t0) \n"
+ /* Load initial values into vector registers */
+" mv t0, %[initial_vvalues] \n"
+" vsetvli x0,%[vlenb],e8,m1,ta,ma \n"
+" vle8.v v0, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v1, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v2, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v3, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v4, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v5, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v6, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v7, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v8, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v9, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v10, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v11, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v12, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v13, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v14, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v15, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v16, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v17, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v18, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v19, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v20, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v21, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v22, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v23, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v24, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v25, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v26, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v27, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v28, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v29, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v30, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vle8.v v31, (t0) \n"
/* Trigger the SIGILL */
".global unimp_addr \n"
"unimp_addr: \n"
@@ -251,19 +378,93 @@ static void run_test(void)
" fsd f29, 0xe8(t0) \n"
" fsd f30, 0xf0(t0) \n"
" fsd f31, 0xf8(t0) \n"
+ /* Save final values from vector registers */
+" mv t0, %[final_vvalues] \n"
+" vse8.v v0, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v1, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v2, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v3, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v4, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v5, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v6, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v7, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v8, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v9, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v10, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v11, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v12, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v13, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v14, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v15, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v16, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v17, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v18, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v19, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v20, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v21, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v22, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v23, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v24, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v25, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v26, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v27, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v28, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v29, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v30, (t0) \n"
+" add t0, t0, %[vlenb] \n"
+" vse8.v v31, (t0) \n"
: "=m" (initial_gvalues),
"=m" (final_gvalues),
- "=m" (final_fvalues)
- : "m" (initial_fvalues),
+ "=m" (final_fvalues),
+ "=m" (final_vvalues)
+ : [vlenb] "r" (vlenb),
+ "m" (initial_fvalues),
+ "m" (initial_vvalues),
[initial_gvalues] "r" (initial_gvalues),
[initial_fvalues] "r" (initial_fvalues),
+ [initial_vvalues] "r" (initial_vvalues),
[final_gvalues] "r" (final_gvalues),
- [final_fvalues] "r" (final_fvalues)
+ [final_fvalues] "r" (final_fvalues),
+ [final_vvalues] "r" (final_vvalues)
: "t0",
"f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
"f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
"f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
- "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31");
+ "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
+ "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7",
+ "v8", "v9", "v10", "v11", "v12", "v13", "v14", "v15",
+ "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
+ "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
assert(got_signal);
@@ -272,7 +473,7 @@ static void run_test(void)
* and is not a simple equality.
*/
assert(initial_gvalues[4] == (unsigned long)initial_gvalues);
- assert(signal_gvalues[4] == (unsigned long)initial_fvalues);
+ assert(signal_gvalues[4] == (unsigned long)initial_vvalues + 31 * vlenb);
assert(final_gvalues[4] == (unsigned long)final_gvalues);
initial_gvalues[4] = final_gvalues[4] = signal_gvalues[4] = 0;
@@ -284,6 +485,8 @@ static void run_test(void)
assert(!memcmp(initial_gvalues, signal_gvalues, 8 * 31));
assert(!memcmp(initial_fvalues, final_fvalues, 8 * 32));
assert(!memcmp(initial_fvalues, signal_fvalues, 8 * 32));
+ assert(!memcmp(initial_vvalues, signal_vvalues, vlenb * 32));
+ assert(!memcmp(initial_vvalues, final_vvalues, vlenb * 32));
}
int main(void)
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext
2025-09-03 4:25 ` [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext Nicholas Piggin
@ 2025-09-03 5:31 ` Richard Henderson
0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2025-09-03 5:31 UTC (permalink / raw)
To: qemu-devel
On 9/3/25 06:25, Nicholas Piggin wrote:
> static void restore_sigcontext(CPURISCVState *env, struct target_sigcontext *sc)
> {
> + struct target_ctx_hdr *hdr;
> int i;
>
> - __get_user(env->pc, &sc->pc);
> + __get_user(env->pc, &sc->sc_regs.pc);
>
> for (i = 1; i < 32; ++i) {
> - __get_user(env->gpr[i], &sc->gpr[i - 1]);
> + __get_user(env->gpr[i], &sc->sc_regs.gpr[i - 1]);
> }
> for (i = 0; i < 32; ++i) {
> - __get_user(env->fpr[i], &sc->fpr[i]);
> + __get_user(env->fpr[i], &sc->sc_fpregs.fpr[i]);
> }
>
> uint32_t fcsr;
> - __get_user(fcsr, &sc->fcsr);
> + __get_user(fcsr, &sc->sc_fpregs.fcsr);
> riscv_csr_write(env, CSR_FCSR, fcsr);
> +
> + hdr = &sc->sc_extdesc.hdr;
> + uint32_t rsv;
> + __get_user(rsv, &sc->sc_extdesc.reserved);
> + if (rsv != 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "signal: sigcontext reserved field is "
> + "non-zero. Attempting restore anyway.");
> + }
The kernel returns -EINVAL from restore_sigcontext, which causes rt_sigreturn to
force_sig(SIGSEGV). We don't need -ERRNO here, but returning bool success would be proper.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] linux-user/riscv: Add vector state to signal context
2025-09-03 4:25 ` [PATCH 3/4] linux-user/riscv: Add vector state to signal context Nicholas Piggin
@ 2025-09-03 9:14 ` Richard Henderson
0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2025-09-03 9:14 UTC (permalink / raw)
To: qemu-devel
On 9/3/25 06:25, Nicholas Piggin wrote:
> This enables vector state to be saved and restored across signals.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> linux-user/riscv/signal.c | 130 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 126 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
> index 4ef55d0848..4acbabcbc9 100644
> --- a/linux-user/riscv/signal.c
> +++ b/linux-user/riscv/signal.c
> @@ -41,7 +41,17 @@ struct target_fp_state {
> uint32_t fcsr;
> };
>
> +struct target_v_ext_state {
> + target_ulong vstart;
> + target_ulong vl;
> + target_ulong vtype;
> + target_ulong vcsr;
> + target_ulong vlenb;
All abi_ulong.
> + target_ulong datap;
abi_ptr.
> +} __attribute__((aligned(16)));
Where does this come from?
As it happens, sizeof(struct target_v_ext_state) will be a multiple of 16 for riscv64.
however...
> +
> /* The Magic number for signal context frame header. */
> +#define RISCV_V_MAGIC 0x53465457
> #define END_MAGIC 0x0
>
> /* The size of END signal context header. */
> @@ -106,6 +116,88 @@ static abi_ulong get_sigframe(struct target_sigaction *ka,
> return sp;
> }
>
> +static unsigned int get_v_state_size(CPURISCVState *env)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> +
> + return sizeof(struct target_ctx_hdr) +
> + sizeof(struct target_v_ext_state) +
> + cpu->cfg.vlenb * 32;
> +}
> +
> +static struct target_ctx_hdr *save_v_state(CPURISCVState *env,
> + struct target_ctx_hdr *hdr)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + target_ulong vlenb = cpu->cfg.vlenb;
> + uint32_t riscv_v_sc_size = get_v_state_size(env);
> + struct target_v_ext_state *vs;
> + uint64_t *vdatap;
> + int i;
> +
> + __put_user(RISCV_V_MAGIC, &hdr->magic);
> + __put_user(riscv_v_sc_size, &hdr->size);
> +
> + vs = (struct target_v_ext_state *)(hdr + 1);
> + vdatap = (uint64_t *)(vs + 1);
... if you wanted to ensure 16-byte alignment of the data for riscv32, you'd do it here.
I don't believe there's anything about RVV that would require 16-byte aligned data though.
> +
> + __put_user(env->vstart, &vs->vstart);
> + __put_user(env->vl, &vs->vl);
> + __put_user(env->vtype, &vs->vtype);
> + target_ulong vcsr = riscv_csr_read(env, CSR_VCSR);
> + __put_user(vcsr, &vs->vcsr);
> + __put_user(vlenb, &vs->vlenb);
> + __put_user((target_ulong)vdatap, &vs->datap);
h2g(vdatap)
> +static void restore_v_state(CPURISCVState *env,
> + struct target_ctx_hdr *hdr)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + target_ulong vlenb = cpu->cfg.vlenb;
> + struct target_v_ext_state *vs;
> + uint64_t *vdatap;
> + int i;
> +
> + uint32_t size;
> + __get_user(size, &hdr->size);
> + if (size != get_v_state_size(env)) {
> + g_assert_not_reached();
> + /* XXX: warn, bail */
> + }
Do not assert. The kernel simply fails the restore,
if (!(has_vector() || has_xtheadvector()) ||
!riscv_v_vstate_query(regs) ||
size != riscv_v_sc_size)
return -EINVAL;
leading to SIGSEGV.
> +
> + vs = (struct target_v_ext_state *)(hdr + 1);
> +
> + __get_user(env->vstart, &vs->vstart);
> + __get_user(env->vl, &vs->vl);
> + __get_user(env->vtype, &vs->vtype);
You're missing a step here. The unsanitized values from vs should be processed as if
"vsetvl x0, vtype, vl". In particular, if either input is garbage, vill will be set. You
probably need a small wrapper around helper_vsetvl().
Similarly vstart should be written with riscv_csr_write because it gets masked.
> + target_ulong vcsr;
> + __get_user(vcsr, &vs->vcsr);
Do not intersperse declarations. As much as I like them, qemu has denied them in
docs/devel/style.rst.
> + riscv_csr_write(env, CSR_VCSR, vcsr);
> + __get_user(vlenb, &vs->vlenb);
> + target_ulong __vdatap;
No __, ever. It's a reserved namespace in user-land.
> + __get_user(__vdatap, &vs->datap);
> + vdatap = (uint64_t *)__vdatap;
Bad cast. Since vdatap may be discontiguous, you need
host_vdatap = lock_user(VERIFY_READ, guest_vdatap, env->vlenb * 32);
if (!host_vdatap) {
goto badaddr;
}
> +
> + for (i = 0; i < 32; i++) {
> + int j;
> + for (j = 0; j < vlenb; j += 8) {
Feel free to use
for (int i = 0;
for (int j = 0;
etc.
> @@ -207,9 +308,30 @@ static void restore_sigcontext(CPURISCVState *env, struct target_sigcontext *sc)
>
> uint32_t magic;
> __get_user(magic, &hdr->magic);
> - if (magic != END_MAGIC) {
> - qemu_log_mask(LOG_UNIMP, "signal: unknown extended context header: "
> - "0x%08x, ignoring", magic);
> + while (magic != END_MAGIC) {
> + if (magic == RISCV_V_MAGIC) {
> + if (riscv_has_ext(env, RVV)) {
> + restore_v_state(env, hdr);
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "signal: sigcontext has V state "
> + "but CPU does not.");
> + }
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "signal: unknown extended state in "
> + "sigcontext magic=0x%08x", magic);
> + }
> +
> + if (hdr->size == 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "signal: extended state in sigcontext "
> + "has size 0");
> + }
> + hdr = (void *)hdr + hdr->size;
> + __get_user(magic, &hdr->magic);
> + }
> +
> + if (hdr->size != END_HDR_SIZE) {
> + qemu_log_mask(LOG_GUEST_ERROR, "signal: extended state end header has "
> + "size=%u (should be 0)", hdr->size);
All of these GUEST_ERROR should goto badaddr. We haven't generally logged the reason for
this, though I don't have any particular objection to doing so.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test
2025-09-03 4:25 ` [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test Nicholas Piggin
@ 2025-09-03 20:15 ` Daniel Henrique Barboza
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2025-09-03 20:15 UTC (permalink / raw)
To: Nicholas Piggin, qemu-riscv
Cc: Laurent Vivier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Liu Zhiwei, qemu-devel
On 9/3/25 1:25 AM, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> tests/tcg/riscv64/test-signal-handling.c | 215 ++++++++++++++++++++++-
> 1 file changed, 209 insertions(+), 6 deletions(-)
>
> diff --git a/tests/tcg/riscv64/test-signal-handling.c b/tests/tcg/riscv64/test-signal-handling.c
> index e9c0170c74..29b2fe169d 100644
> --- a/tests/tcg/riscv64/test-signal-handling.c
> +++ b/tests/tcg/riscv64/test-signal-handling.c
> @@ -19,9 +19,16 @@
> #include <execinfo.h>
> #include <unistd.h>
> #include <assert.h>
> +#include <sys/auxv.h>
> +#include <elf.h>
> #include <sys/mman.h>
> #include <ucontext.h>
> #include <asm/sigcontext.h>
> +#include "riscv_vector.h"
As I said in a review in the "[PATCH 0/3] target/riscv: corner case fixes" series,
this header will break 'make check-tcg'. We need extra changes in the Docker image
to recognize this header.
Thanks,
Daniel
> +
> +#ifndef COMPAT_HWCAP_ISA_V
> +#define COMPAT_HWCAP_ISA_V (1 << ('V' - 'A'))
> +#endif
>
> /*
> * This horrible hack seems to be required when including
> @@ -41,6 +48,10 @@ static uint64_t *signal_gvalues;
> static double *initial_fvalues;
> static double *final_fvalues;
> static double *signal_fvalues;
> +static size_t vlenb;
> +static uint8_t *initial_vvalues;
> +static uint8_t *final_vvalues;
> +static uint8_t *signal_vvalues;
>
> extern unsigned long unimp_addr[];
>
> @@ -64,6 +75,8 @@ static void ILL_handler(int signo, siginfo_t *info, void *context)
> {
> ucontext_t *uc = context;
> struct sigcontext *sc = (struct sigcontext *)&uc->uc_mcontext;
> + struct __riscv_ctx_hdr *sc_ext = &sc->sc_extdesc.hdr;
> + bool found_v = false;
>
> got_signal = true;
>
> @@ -82,12 +95,47 @@ static void ILL_handler(int signo, siginfo_t *info, void *context)
> }
> /* Test sc->sc_fpregs.d.fcsr ? */
>
> + assert(sc->sc_extdesc.reserved == 0);
> + while (sc_ext->magic != END_MAGIC) {
> + assert(sc_ext->size != 0);
> +
> + if (sc_ext->magic == RISCV_V_MAGIC) {
> + struct __sc_riscv_v_state *sc_v_state = (struct __sc_riscv_v_state *)(sc_ext + 1);
> + struct __riscv_v_ext_state *v_state = &sc_v_state->v_state;
> +
> + found_v = true;
> +
> + assert(getauxval(AT_HWCAP) & COMPAT_HWCAP_ISA_V);
> +
> + assert(v_state->vlenb == vlenb);
> + assert(v_state->vtype == 0xc0); /* vma, vta */
> + assert(v_state->vl == vlenb);
> + assert(v_state->vstart == 0);
> + assert(v_state->vcsr == 0);
> +
> + uint64_t *vregs = v_state->datap;
> + for (int i = 0; i < 32; i++) {
> + for (int j = 0; j < vlenb; j += 8) {
> + size_t idx = (i * vlenb + j) / 8;
> + ((uint64_t *)signal_vvalues)[idx] = vregs[idx];
> + }
> + }
> + }
> +
> + sc_ext = (void *)sc_ext + sc_ext->size;
> + }
> +
> + assert(sc_ext->size == 0);
> + if (getauxval(AT_HWCAP) & COMPAT_HWCAP_ISA_V) {
> + assert(found_v);
> + }
> +
> sc->sc_regs.pc += 4;
> }
>
> static void init_test(void)
> {
> - int i;
> + int i, j;
>
> callchain_root = find_callchain_root();
>
> @@ -107,6 +155,19 @@ static void init_test(void)
> memset(final_fvalues, 0, 8 * 32);
> signal_fvalues = malloc(8 * 32);
> memset(signal_fvalues, 0, 8 * 32);
> +
> + vlenb = __riscv_vlenb();
> + initial_vvalues = malloc(vlenb * 32);
> + memset(initial_vvalues, 0, vlenb * 32);
> + for (i = 0; i < 32 ; i++) {
> + for (j = 0; j < vlenb; j++) {
> + initial_vvalues[i * vlenb + j] = i * vlenb + j;
> + }
> + }
> + final_vvalues = malloc(vlenb * 32);
> + memset(final_vvalues, 0, vlenb * 32);
> + signal_vvalues = malloc(vlenb * 32);
> + memset(signal_vvalues, 0, vlenb * 32);
> }
>
> static void run_test(void)
> @@ -179,6 +240,72 @@ static void run_test(void)
> " fld f29, 0xe8(t0) \n"
> " fld f30, 0xf0(t0) \n"
> " fld f31, 0xf8(t0) \n"
> + /* Load initial values into vector registers */
> +" mv t0, %[initial_vvalues] \n"
> +" vsetvli x0,%[vlenb],e8,m1,ta,ma \n"
> +" vle8.v v0, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v1, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v2, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v3, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v4, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v5, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v6, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v7, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v8, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v9, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v10, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v11, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v12, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v13, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v14, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v15, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v16, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v17, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v18, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v19, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v20, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v21, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v22, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v23, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v24, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v25, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v26, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v27, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v28, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v29, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v30, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vle8.v v31, (t0) \n"
> /* Trigger the SIGILL */
> ".global unimp_addr \n"
> "unimp_addr: \n"
> @@ -251,19 +378,93 @@ static void run_test(void)
> " fsd f29, 0xe8(t0) \n"
> " fsd f30, 0xf0(t0) \n"
> " fsd f31, 0xf8(t0) \n"
> + /* Save final values from vector registers */
> +" mv t0, %[final_vvalues] \n"
> +" vse8.v v0, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v1, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v2, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v3, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v4, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v5, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v6, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v7, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v8, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v9, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v10, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v11, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v12, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v13, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v14, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v15, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v16, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v17, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v18, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v19, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v20, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v21, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v22, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v23, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v24, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v25, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v26, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v27, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v28, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v29, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v30, (t0) \n"
> +" add t0, t0, %[vlenb] \n"
> +" vse8.v v31, (t0) \n"
> : "=m" (initial_gvalues),
> "=m" (final_gvalues),
> - "=m" (final_fvalues)
> - : "m" (initial_fvalues),
> + "=m" (final_fvalues),
> + "=m" (final_vvalues)
> + : [vlenb] "r" (vlenb),
> + "m" (initial_fvalues),
> + "m" (initial_vvalues),
> [initial_gvalues] "r" (initial_gvalues),
> [initial_fvalues] "r" (initial_fvalues),
> + [initial_vvalues] "r" (initial_vvalues),
> [final_gvalues] "r" (final_gvalues),
> - [final_fvalues] "r" (final_fvalues)
> + [final_fvalues] "r" (final_fvalues),
> + [final_vvalues] "r" (final_vvalues)
> : "t0",
> "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
> "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
> "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
> - "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31");
> + "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
> + "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7",
> + "v8", "v9", "v10", "v11", "v12", "v13", "v14", "v15",
> + "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
> + "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
>
> assert(got_signal);
>
> @@ -272,7 +473,7 @@ static void run_test(void)
> * and is not a simple equality.
> */
> assert(initial_gvalues[4] == (unsigned long)initial_gvalues);
> - assert(signal_gvalues[4] == (unsigned long)initial_fvalues);
> + assert(signal_gvalues[4] == (unsigned long)initial_vvalues + 31 * vlenb);
> assert(final_gvalues[4] == (unsigned long)final_gvalues);
> initial_gvalues[4] = final_gvalues[4] = signal_gvalues[4] = 0;
>
> @@ -284,6 +485,8 @@ static void run_test(void)
> assert(!memcmp(initial_gvalues, signal_gvalues, 8 * 31));
> assert(!memcmp(initial_fvalues, final_fvalues, 8 * 32));
> assert(!memcmp(initial_fvalues, signal_fvalues, 8 * 32));
> + assert(!memcmp(initial_vvalues, signal_vvalues, vlenb * 32));
> + assert(!memcmp(initial_vvalues, final_vvalues, vlenb * 32));
> }
>
> int main(void)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-03 20:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-03 4:25 [PATCH 0/4] linux-user/riscv: add vector state to signal Nicholas Piggin
2025-09-03 4:25 ` [PATCH 1/4] tests/tcg/riscv64: Add a user signal handling test Nicholas Piggin
2025-09-03 4:25 ` [PATCH 2/4] linux-user/riscv: Add extended state to sigcontext Nicholas Piggin
2025-09-03 5:31 ` Richard Henderson
2025-09-03 4:25 ` [PATCH 3/4] linux-user/riscv: Add vector state to signal context Nicholas Piggin
2025-09-03 9:14 ` Richard Henderson
2025-09-03 4:25 ` [PATCH 4/4] tests/tcg/riscv64: Add vector state to signal test Nicholas Piggin
2025-09-03 20:15 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).