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 <npig...@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~
