On Fri, 25 Jul 2025 at 17:39, Richard Henderson <[email protected]> wrote: > > On 7/25/25 04:22, 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: [email protected] > > Fixes: 78011586b90d1 ("target/arm: Enable SME for user-only") > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > linux-user/aarch64/signal.c | 46 +++++++++++++++++++++++++++++++++++-- > > 1 file changed, 44 insertions(+), 2 deletions(-) > > Oh my. I spent yesterday working on this and other signal handling fixes. > Though from a > FEAT_GCS starting point, so I still hadn't seen the clear tpidr2 on signal > delivery change. > > > > +static void target_setup_tpidr2_record(struct target_tpidr2_context > > *tpidr2, > > + CPUARMState *env, int size) > > +{ > > + __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); > > +} > > Drop the unused size argument. > > > +static bool target_restore_tpidr2_record(CPUARMState *env, > > + struct target_tpidr2_context > > *tpidr2) > > +{ > > + if (!cpu_isar_feature(aa64_sme, env_archcpu(env))) { > > + return false; > > + } > > + __get_user(env->cp15.tpidr2_el0, &tpidr2->tpidr2); > > + return true; > > +} > > The sme check is better placed in target_restore_sigframe ... > > > + case TARGET_TPIDR2_MAGIC: > > + if (tpidr2 || size != sizeof(struct target_tpidr2_context)) { > > + goto err; > > + } > > ... here. Then target_restore_tpidr2_record has no failure modes and can > return void ... > > > @@ -497,6 +530,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)) { > > + goto err; > > + } > > ... which simplifies this.
Mmm. I wasn't sure to what extent we were trying to follow the kernel signal.c (which defers essentially all error checks including sizes to its restore_*_context functions, so that the "switch on type of record" function is pretty much the same code for each block, whether it's a complicated variable size one or a simple fixed record. -- PMM
