qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 for-10.1 0/3] linux-user/aarch64: Fix SME/SME2 signal frame handling
@ 2025-07-25 17:55 Peter Maydell
  2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Peter Maydell @ 2025-07-25 17:55 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Richard Henderson

This patchset fixes some bugs in our SME-related signal frame
handling for linux-user:

 * we need to make the equivalent change to a recent kernel
   bugfix/ABI change: TPIDR2_EL0 should be cleared when delivering
   a signal
 * we forgot the TPIDR2_MAGIC signal frame record (which is necessary
   for SME v1)
 * we forgot the ZT_MAGIC signal frame record when implementing SME2

These bugs generally only surface when guest code attempts to
unwind an exception from inside a signal handler and SME is
involved. Discovered (and the fixes tested) by some new gcc
test cases which implement their part of the bugfix/ABI change
 https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b5ffc8e75a8

v2: quick respin to address RTH's code review comments. I know
I only sent v1 earlier this evening but I wanted to get this
out of the door as I'm not going to be working the first half
of next week (back Thurs).

v1->v2:
 * patch 1: drop unnecessary if(), update comment
 * patch 2: do "has sme" check in better place, drop unneeded
   argument, return type
 * patch 3: do "has sme" check in better place, check the
   incoming SVCR value, not the old one

thanks
-- PMM


Peter Maydell (3):
  linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals
  linux-user/aarch64: Support TPIDR2_MAGIC signal frame record
  linux-user/aarch64: Support ZT_MAGIC signal frame record

 linux-user/aarch64/signal.c | 139 +++++++++++++++++++++++++++++++++++-
 1 file changed, 136 insertions(+), 3 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals
  2025-07-25 17:55 [PATCH v2 for-10.1 0/3] linux-user/aarch64: Fix SME/SME2 signal frame handling Peter Maydell
@ 2025-07-25 17:55 ` Peter Maydell
  2025-07-25 17:57   ` Pierrick Bouvier
  2025-07-28 16:10   ` Michael Tokarev
  2025-07-25 17:55 ` [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record Peter Maydell
  2025-07-25 17:55 ` [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC " Peter Maydell
  2 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2025-07-25 17:55 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Richard Henderson

A recent change to the kernel (Linux commit b376108e1f88
"arm64/fpsimd: signal: Clear TPIDR2 when delivering signals") updated
the signal-handler entry code to always clear TPIDR2_EL0.

This is necessary for the userspace ZA lazy saving scheme to work
correctly when unwinding exceptions across a signal boundary.
(For the essay-length description of the incorrect behaviour and
why this is the correct fix, see the commit message for the
kernel commit.)

Make QEMU also clear TPIDR2_EL0 on signal entry, applying the
equivalent bugfix to our implementation.

Note that getting this unwinding to work correctly also requires
changes to the userspace code, e.g.  as implemented in gcc in
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b5ffc8e75a8

This change is technically an ABI change; from the kernel's
point of view SME was never enabled (it was hidden behind
CONFIG_BROKEN) before the change. From QEMU's point of view
our SME-related signal handling was broken anyway as we weren't
saving and restoring TPIDR2_EL0.

Cc: qemu-stable@nongnu.org
Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/signal.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index d50cab78d83..6514b73ad98 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -666,8 +666,12 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
         env->btype = 2;
     }
 
-    /* Invoke the signal handler with both SM and ZA disabled. */
+    /*
+     * Invoke the signal handler with a clean SME state: both SM and ZA
+     * disabled and TPIDR2_EL0 cleared.
+     */
     aarch64_set_svcr(env, 0, R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
+    env->cp15.tpidr2_el0 = 0;
 
     if (info) {
         frame->info = *info;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record
  2025-07-25 17:55 [PATCH v2 for-10.1 0/3] linux-user/aarch64: Fix SME/SME2 signal frame handling Peter Maydell
  2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
@ 2025-07-25 17:55 ` Peter Maydell
  2025-07-25 17:57   ` Pierrick Bouvier
  2025-07-25 20:20   ` Richard Henderson
  2025-07-25 17:55 ` [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC " Peter Maydell
  2 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2025-07-25 17:55 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Richard Henderson

FEAT_SME adds the TPIDR2 userspace-accessible system register, which
is used as part of the procedure calling standard's lazy saving
scheme for the ZA registers:
 https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#66the-za-lazy-saving-scheme

The Linux kernel has a signal frame record for saving
and restoring this value when calling signal handlers, but
we forgot to implement this. The result is that code which
tries to unwind an exception out of a signal handler will
not work correctly.

Add support for the missing record.

Cc: qemu-stable@nongnu.org
Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/aarch64/signal.c | 42 +++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index 6514b73ad98..f28ba807549 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -121,6 +121,13 @@ struct target_za_context {
 #define TARGET_ZA_SIG_CONTEXT_SIZE(VQ) \
     TARGET_ZA_SIG_ZAV_OFFSET(VQ, VQ * TARGET_SVE_VQ_BYTES)
 
+#define TARGET_TPIDR2_MAGIC 0x54504902
+
+struct target_tpidr2_context {
+    struct target_aarch64_ctx head;
+    uint64_t tpidr2;
+};
+
 struct target_rt_sigframe {
     struct target_siginfo info;
     struct target_ucontext uc;
@@ -253,6 +260,14 @@ static void target_setup_za_record(struct target_za_context *za,
     }
 }
 
+static void target_setup_tpidr2_record(struct target_tpidr2_context *tpidr2,
+                                       CPUARMState *env)
+{
+    __put_user(TARGET_TPIDR2_MAGIC, &tpidr2->head.magic);
+    __put_user(sizeof(struct target_tpidr2_context), &tpidr2->head.size);
+    __put_user(env->cp15.tpidr2_el0, &tpidr2->tpidr2);
+}
+
 static void target_restore_general_frame(CPUARMState *env,
                                          struct target_rt_sigframe *sf)
 {
@@ -403,6 +418,12 @@ static bool target_restore_za_record(CPUARMState *env,
     return true;
 }
 
+static void target_restore_tpidr2_record(CPUARMState *env,
+                                         struct target_tpidr2_context *tpidr2)
+{
+    __get_user(env->cp15.tpidr2_el0, &tpidr2->tpidr2);
+}
+
 static int target_restore_sigframe(CPUARMState *env,
                                    struct target_rt_sigframe *sf)
 {
@@ -410,6 +431,7 @@ static int target_restore_sigframe(CPUARMState *env,
     struct target_fpsimd_context *fpsimd = NULL;
     struct target_sve_context *sve = NULL;
     struct target_za_context *za = NULL;
+    struct target_tpidr2_context *tpidr2 = NULL;
     uint64_t extra_datap = 0;
     bool used_extra = false;
     int sve_size = 0;
@@ -460,6 +482,14 @@ static int target_restore_sigframe(CPUARMState *env,
             za_size = size;
             break;
 
+        case TARGET_TPIDR2_MAGIC:
+            if (tpidr2 || size != sizeof(struct target_tpidr2_context) ||
+                !cpu_isar_feature(aa64_sme, env_archcpu(env))) {
+                goto err;
+            }
+            tpidr2 = (struct target_tpidr2_context *)ctx;
+            break;
+
         case TARGET_EXTRA_MAGIC:
             if (extra || size != sizeof(struct target_extra_context)) {
                 goto err;
@@ -497,6 +527,9 @@ static int target_restore_sigframe(CPUARMState *env,
     if (za && !target_restore_za_record(env, za, za_size, &svcr)) {
         goto err;
     }
+    if (tpidr2) {
+        target_restore_tpidr2_record(env, tpidr2);
+    }
     if (env->svcr != svcr) {
         env->svcr = svcr;
         arm_rebuild_hflags(env);
@@ -568,8 +601,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
         .total_size = offsetof(struct target_rt_sigframe,
                                uc.tuc_mcontext.__reserved),
     };
-    int fpsimd_ofs, fr_ofs, sve_ofs = 0, za_ofs = 0;
-    int sve_size = 0, za_size = 0;
+    int fpsimd_ofs, fr_ofs, sve_ofs = 0, za_ofs = 0, tpidr2_ofs = 0;
+    int sve_size = 0, za_size = 0, tpidr2_size = 0;
     struct target_rt_sigframe *frame;
     struct target_rt_frame_record *fr;
     abi_ulong frame_addr, return_addr;
@@ -585,6 +618,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
         sve_ofs = alloc_sigframe_space(sve_size, &layout);
     }
     if (cpu_isar_feature(aa64_sme, env_archcpu(env))) {
+        tpidr2_size = sizeof(struct target_tpidr2_context);
+        tpidr2_ofs = alloc_sigframe_space(tpidr2_size, &layout);
         /* ZA state needs saving only if it is enabled.  */
         if (FIELD_EX64(env->svcr, SVCR, ZA)) {
             za_size = TARGET_ZA_SIG_CONTEXT_SIZE(sme_vq(env));
@@ -644,6 +679,9 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
     if (za_ofs) {
         target_setup_za_record((void *)frame + za_ofs, env, za_size);
     }
+    if (tpidr2_ofs) {
+        target_setup_tpidr2_record((void *)frame + tpidr2_ofs, env);
+    }
 
     /* Set up the stack frame for unwinding.  */
     fr = (void *)frame + fr_ofs;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC signal frame record
  2025-07-25 17:55 [PATCH v2 for-10.1 0/3] linux-user/aarch64: Fix SME/SME2 signal frame handling Peter Maydell
  2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
  2025-07-25 17:55 ` [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record Peter Maydell
@ 2025-07-25 17:55 ` Peter Maydell
  2025-07-25 17:59   ` Pierrick Bouvier
  2025-07-25 20:23   ` Richard Henderson
  2 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2025-07-25 17:55 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Richard Henderson

FEAT_SME2 adds the ZT0 register, whose contents may need to be
preserved and restored on signal handler entry and exit.  This is
done with a new ZT_MAGIC record.  We forgot to implement support for
this in our linux-user code before enabling the SME2p1 emulation,
which meant that a signal handler using SME would corrupt the ZT0
register value, and code that attempted to unwind an exception from
inside a signal handler would not work.

Add the missing record handling.

Fixes: 7b1613a1020d2942 ("target/arm: Enable FEAT_SME2p1 on -cpu max")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/aarch64/signal.c | 93 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index f28ba807549..668353bbda4 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -128,6 +128,23 @@ struct target_tpidr2_context {
     uint64_t tpidr2;
 };
 
+#define TARGET_ZT_MAGIC 0x5a544e01
+
+struct target_zt_context {
+    struct target_aarch64_ctx head;
+    uint16_t nregs;
+    uint16_t reserved[3];
+    /* ZTn register data immediately follows */
+};
+
+#define TARGET_ZT_SIG_REG_BYTES (512 / 8)
+#define TARGET_ZT_SIG_REGS_SIZE(n) (TARGET_ZT_SIG_REG_BYTES * (n))
+#define TARGET_ZT_SIG_CONTEXT_SIZE(n) (sizeof(struct target_zt_context) + \
+                                       TARGET_ZT_SIG_REGS_SIZE(n))
+#define TARGET_ZT_SIG_REGS_OFFSET sizeof(struct target_zt_context)
+QEMU_BUILD_BUG_ON(TARGET_ZT_SIG_REG_BYTES != \
+                  sizeof_field(CPUARMState, za_state.zt0));
+
 struct target_rt_sigframe {
     struct target_siginfo info;
     struct target_ucontext uc;
@@ -268,6 +285,28 @@ static void target_setup_tpidr2_record(struct target_tpidr2_context *tpidr2,
     __put_user(env->cp15.tpidr2_el0, &tpidr2->tpidr2);
 }
 
+static void target_setup_zt_record(struct target_zt_context *zt,
+                                   CPUARMState *env, int size)
+{
+    uint64_t *z;
+
+    memset(zt, 0, sizeof(*zt));
+    __put_user(TARGET_ZT_MAGIC, &zt->head.magic);
+    __put_user(size, &zt->head.size);
+    /*
+     * The record format allows for multiple ZT regs, but
+     * currently there is only one, ZT0.
+     */
+    __put_user(1, &zt->nregs);
+    assert(size == TARGET_ZT_SIG_CONTEXT_SIZE(1));
+
+    /* ZT0 is the same byte-stream format as SVE regs and ZA */
+    z = (void *)zt + TARGET_ZT_SIG_REGS_OFFSET;
+    for (int i = 0; i < ARRAY_SIZE(env->za_state.zt0); i++) {
+        __put_user_e(env->za_state.zt0[i], z + i, le);
+    }
+}
+
 static void target_restore_general_frame(CPUARMState *env,
                                          struct target_rt_sigframe *sf)
 {
@@ -424,6 +463,30 @@ static void target_restore_tpidr2_record(CPUARMState *env,
     __get_user(env->cp15.tpidr2_el0, &tpidr2->tpidr2);
 }
 
+static bool target_restore_zt_record(CPUARMState *env,
+                                     struct target_zt_context *zt, int size,
+                                     int svcr)
+{
+    uint16_t nregs;
+    uint64_t *z;
+
+    if (!(FIELD_EX64(svcr, SVCR, ZA))) {
+        return false;
+    }
+
+    __get_user(nregs, &zt->nregs);
+
+    if (nregs != 1) {
+        return false;
+    }
+
+    z = (void *)zt + TARGET_ZT_SIG_REGS_OFFSET;
+    for (int i = 0; i < ARRAY_SIZE(env->za_state.zt0); i++) {
+        __get_user_e(env->za_state.zt0[i], z + i, le);
+    }
+    return true;
+}
+
 static int target_restore_sigframe(CPUARMState *env,
                                    struct target_rt_sigframe *sf)
 {
@@ -432,10 +495,12 @@ static int target_restore_sigframe(CPUARMState *env,
     struct target_sve_context *sve = NULL;
     struct target_za_context *za = NULL;
     struct target_tpidr2_context *tpidr2 = NULL;
+    struct target_zt_context *zt = NULL;
     uint64_t extra_datap = 0;
     bool used_extra = false;
     int sve_size = 0;
     int za_size = 0;
+    int zt_size = 0;
     int svcr = 0;
 
     target_restore_general_frame(env, sf);
@@ -490,6 +555,15 @@ static int target_restore_sigframe(CPUARMState *env,
             tpidr2 = (struct target_tpidr2_context *)ctx;
             break;
 
+        case TARGET_ZT_MAGIC:
+            if (zt || size != TARGET_ZT_SIG_CONTEXT_SIZE(1) ||
+                !cpu_isar_feature(aa64_sme2, env_archcpu(env))) {
+                goto err;
+            }
+            zt = (struct target_zt_context *)ctx;
+            zt_size = size;
+            break;
+
         case TARGET_EXTRA_MAGIC:
             if (extra || size != sizeof(struct target_extra_context)) {
                 goto err;
@@ -530,6 +604,13 @@ static int target_restore_sigframe(CPUARMState *env,
     if (tpidr2) {
         target_restore_tpidr2_record(env, tpidr2);
     }
+    /*
+     * NB that we must restore ZT after ZA so the check that there's
+     * no ZT record if SVCR.ZA is 0 gets the right value of SVCR.
+     */
+    if (zt && !target_restore_zt_record(env, zt, zt_size, svcr)) {
+        goto err;
+    }
     if (env->svcr != svcr) {
         env->svcr = svcr;
         arm_rebuild_hflags(env);
@@ -602,7 +683,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
                                uc.tuc_mcontext.__reserved),
     };
     int fpsimd_ofs, fr_ofs, sve_ofs = 0, za_ofs = 0, tpidr2_ofs = 0;
-    int sve_size = 0, za_size = 0, tpidr2_size = 0;
+    int zt_ofs = 0;
+    int sve_size = 0, za_size = 0, tpidr2_size = 0, zt_size = 0;
     struct target_rt_sigframe *frame;
     struct target_rt_frame_record *fr;
     abi_ulong frame_addr, return_addr;
@@ -628,6 +710,12 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
         }
         za_ofs = alloc_sigframe_space(za_size, &layout);
     }
+    if (cpu_isar_feature(aa64_sme2, env_archcpu(env)) &&
+        FIELD_EX64(env->svcr, SVCR, ZA)) {
+        /* If SME ZA storage is enabled, we must also save SME2 ZT0 */
+        zt_size = TARGET_ZT_SIG_CONTEXT_SIZE(1);
+        zt_ofs = alloc_sigframe_space(zt_size, &layout);
+    }
 
     if (layout.extra_ofs) {
         /* Reserve space for the extra end marker.  The standard end marker
@@ -682,6 +770,9 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
     if (tpidr2_ofs) {
         target_setup_tpidr2_record((void *)frame + tpidr2_ofs, env);
     }
+    if (zt_ofs) {
+        target_setup_zt_record((void *)frame + zt_ofs, env, zt_size);
+    }
 
     /* Set up the stack frame for unwinding.  */
     fr = (void *)frame + fr_ofs;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals
  2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
@ 2025-07-25 17:57   ` Pierrick Bouvier
  2025-07-28 16:10   ` Michael Tokarev
  1 sibling, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2025-07-25 17:57 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Richard Henderson

On 7/25/25 10:55 AM, Peter Maydell wrote:
> A recent change to the kernel (Linux commit b376108e1f88
> "arm64/fpsimd: signal: Clear TPIDR2 when delivering signals") updated
> the signal-handler entry code to always clear TPIDR2_EL0.
> 
> This is necessary for the userspace ZA lazy saving scheme to work
> correctly when unwinding exceptions across a signal boundary.
> (For the essay-length description of the incorrect behaviour and
> why this is the correct fix, see the commit message for the
> kernel commit.)
> 
> Make QEMU also clear TPIDR2_EL0 on signal entry, applying the
> equivalent bugfix to our implementation.
> 
> Note that getting this unwinding to work correctly also requires
> changes to the userspace code, e.g.  as implemented in gcc in
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b5ffc8e75a8
> 
> This change is technically an ABI change; from the kernel's
> point of view SME was never enabled (it was hidden behind
> CONFIG_BROKEN) before the change. From QEMU's point of view
> our SME-related signal handling was broken anyway as we weren't
> saving and restoring TPIDR2_EL0.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/aarch64/signal.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record
  2025-07-25 17:55 ` [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record Peter Maydell
@ 2025-07-25 17:57   ` Pierrick Bouvier
  2025-07-25 20:20   ` Richard Henderson
  1 sibling, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2025-07-25 17:57 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Richard Henderson

On 7/25/25 10:55 AM, Peter Maydell wrote:
> FEAT_SME adds the TPIDR2 userspace-accessible system register, which
> is used as part of the procedure calling standard's lazy saving
> scheme for the ZA registers:
>   https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#66the-za-lazy-saving-scheme
> 
> The Linux kernel has a signal frame record for saving
> and restoring this value when calling signal handlers, but
> we forgot to implement this. The result is that code which
> tries to unwind an exception out of a signal handler will
> not work correctly.
> 
> Add support for the missing record.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/aarch64/signal.c | 42 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 40 insertions(+), 2 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC signal frame record
  2025-07-25 17:55 ` [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC " Peter Maydell
@ 2025-07-25 17:59   ` Pierrick Bouvier
  2025-07-25 20:23   ` Richard Henderson
  1 sibling, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2025-07-25 17:59 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Richard Henderson

On 7/25/25 10:55 AM, Peter Maydell wrote:
> FEAT_SME2 adds the ZT0 register, whose contents may need to be
> preserved and restored on signal handler entry and exit.  This is
> done with a new ZT_MAGIC record.  We forgot to implement support for
> this in our linux-user code before enabling the SME2p1 emulation,
> which meant that a signal handler using SME would corrupt the ZT0
> register value, and code that attempted to unwind an exception from
> inside a signal handler would not work.
> 
> Add the missing record handling.
> 
> Fixes: 7b1613a1020d2942 ("target/arm: Enable FEAT_SME2p1 on -cpu max")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/aarch64/signal.c | 93 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 92 insertions(+), 1 deletion(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record
  2025-07-25 17:55 ` [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record Peter Maydell
  2025-07-25 17:57   ` Pierrick Bouvier
@ 2025-07-25 20:20   ` Richard Henderson
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2025-07-25 20:20 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel

On 7/25/25 07:55, Peter Maydell wrote:
> FEAT_SME adds the TPIDR2 userspace-accessible system register, which
> is used as part of the procedure calling standard's lazy saving
> scheme for the ZA registers:
>   https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#66the-za-lazy-saving- 
> scheme
> 
> The Linux kernel has a signal frame record for saving
> and restoring this value when calling signal handlers, but
> we forgot to implement this. The result is that code which
> tries to unwind an exception out of a signal handler will
> not work correctly.
> 
> Add support for the missing record.
> 
> Cc:qemu-stable@nongnu.org
> Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   linux-user/aarch64/signal.c | 42 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 40 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC signal frame record
  2025-07-25 17:55 ` [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC " Peter Maydell
  2025-07-25 17:59   ` Pierrick Bouvier
@ 2025-07-25 20:23   ` Richard Henderson
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2025-07-25 20:23 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel

On 7/25/25 07:55, Peter Maydell wrote:
> FEAT_SME2 adds the ZT0 register, whose contents may need to be
> preserved and restored on signal handler entry and exit.  This is
> done with a new ZT_MAGIC record.  We forgot to implement support for
> this in our linux-user code before enabling the SME2p1 emulation,
> which meant that a signal handler using SME would corrupt the ZT0
> register value, and code that attempted to unwind an exception from
> inside a signal handler would not work.
> 
> Add the missing record handling.
> 
> Fixes: 7b1613a1020d2942 ("target/arm: Enable FEAT_SME2p1 on -cpu max")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   linux-user/aarch64/signal.c | 93 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 92 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals
  2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
  2025-07-25 17:57   ` Pierrick Bouvier
@ 2025-07-28 16:10   ` Michael Tokarev
  2025-07-31  9:57     ` Peter Maydell
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2025-07-28 16:10 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Richard Henderson

On 25.07.2025 20:55, Peter Maydell wrote:
> A recent change to the kernel (Linux commit b376108e1f88
> "arm64/fpsimd: signal: Clear TPIDR2 when delivering signals") updated
> the signal-handler entry code to always clear TPIDR2_EL0.
> 
> This is necessary for the userspace ZA lazy saving scheme to work
> correctly when unwinding exceptions across a signal boundary.
> (For the essay-length description of the incorrect behaviour and
> why this is the correct fix, see the commit message for the
> kernel commit.)
> 
> Make QEMU also clear TPIDR2_EL0 on signal entry, applying the
> equivalent bugfix to our implementation.
> 
> Note that getting this unwinding to work correctly also requires
> changes to the userspace code, e.g.  as implemented in gcc in
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b5ffc8e75a8
> 
> This change is technically an ABI change; from the kernel's
> point of view SME was never enabled (it was hidden behind
> CONFIG_BROKEN) before the change. From QEMU's point of view
> our SME-related signal handling was broken anyway as we weren't
> saving and restoring TPIDR2_EL0.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")

Is it worth the efforts to apply this one to qemu 7.2.x branch?
If yes, I'd love to have it back-ported to that one :)

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals
  2025-07-28 16:10   ` Michael Tokarev
@ 2025-07-31  9:57     ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2025-07-31  9:57 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-arm, qemu-devel, Richard Henderson

On Mon, 28 Jul 2025 at 17:10, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 25.07.2025 20:55, Peter Maydell wrote:
> > A recent change to the kernel (Linux commit b376108e1f88
> > "arm64/fpsimd: signal: Clear TPIDR2 when delivering signals") updated
> > the signal-handler entry code to always clear TPIDR2_EL0.
> >
> > This is necessary for the userspace ZA lazy saving scheme to work
> > correctly when unwinding exceptions across a signal boundary.
> > (For the essay-length description of the incorrect behaviour and
> > why this is the correct fix, see the commit message for the
> > kernel commit.)
> >
> > Make QEMU also clear TPIDR2_EL0 on signal entry, applying the
> > equivalent bugfix to our implementation.
> >
> > Note that getting this unwinding to work correctly also requires
> > changes to the userspace code, e.g.  as implemented in gcc in
> > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b5ffc8e75a8
> >
> > This change is technically an ABI change; from the kernel's
> > point of view SME was never enabled (it was hidden behind
> > CONFIG_BROKEN) before the change. From QEMU's point of view
> > our SME-related signal handling was broken anyway as we weren't
> > saving and restoring TPIDR2_EL0.
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only")
>
> Is it worth the efforts to apply this one to qemu 7.2.x branch?

Well, it's an easy backport since it's a one-liner, and
it's not a complicated change so it's pretty safe. There's
no point unless you're also backporting patch 2 of this series,
though.

In the 7.2.x version of target_setup_frame() the "clear
SVCR bits" code is a little different, but it's still there;
the change should go after this part:

    if (env->svcr) {
        env->svcr = 0;
        arm_rebuild_hflags(env);
    }

(there isn't actually an ordering requirement, so it's
just neater to put it in the same logical place)

-- PMM


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-07-31  9:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-25 17:55 [PATCH v2 for-10.1 0/3] linux-user/aarch64: Fix SME/SME2 signal frame handling Peter Maydell
2025-07-25 17:55 ` [PATCH v2 for-10.1 1/3] linux-user/aarch64: Clear TPIDR2_EL0 when delivering signals Peter Maydell
2025-07-25 17:57   ` Pierrick Bouvier
2025-07-28 16:10   ` Michael Tokarev
2025-07-31  9:57     ` Peter Maydell
2025-07-25 17:55 ` [PATCH v2 for-10.1 2/3] linux-user/aarch64: Support TPIDR2_MAGIC signal frame record Peter Maydell
2025-07-25 17:57   ` Pierrick Bouvier
2025-07-25 20:20   ` Richard Henderson
2025-07-25 17:55 ` [PATCH v2 for-10.1 3/3] linux-user/aarch64: Support ZT_MAGIC " Peter Maydell
2025-07-25 17:59   ` Pierrick Bouvier
2025-07-25 20:23   ` Richard Henderson

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).