[PATCH v2 2/4] arm64/signal: Include TPIDR2 in the signal context

[Will Deacon]

On Mon, Oct 31, 2022 at 08:17:34PM +0000, Mark Brown wrote:
> Add a new signal frame record for TPIDR2 using the same format as we
> already use for ESR with different magic, a header with the value from the
> register appended as the only data. If SME is supported then this record is
> always included.
> 
> Signed-off-by: Mark Brown <broonie at kernel.org>
> Reviewed-by: Szabolcs Nagy <szabolcs.nagy at arm.com>
> ---
>  arch/arm64/include/uapi/asm/sigcontext.h |  8 ++++
>  arch/arm64/kernel/signal.c               | 59 ++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
> index 4aaf31e3bf16..46ce097ca8c0 100644
> --- a/arch/arm64/include/uapi/asm/sigcontext.h
> +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> @@ -140,6 +140,14 @@ struct sve_context {
>  
>  #define SVE_SIG_FLAG_SM	0x1	/* Context describes streaming mode */
>  
> +/* TPIDR2_EL0 context */
> +#define TPIDR2_MAGIC	0x54504902
> +
> +struct tpidr2_context {
> +	struct _aarch64_ctx head;
> +	__u64 tpidr2;
> +};
> +
>  #define ZA_MAGIC	0x54366345
>  
>  struct za_context {
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 9ad911f1647c..ac4fb42a9613 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -56,6 +56,7 @@ struct rt_sigframe_user_layout {
>  	unsigned long fpsimd_offset;
>  	unsigned long esr_offset;
>  	unsigned long sve_offset;
> +	unsigned long tpidr2_offset;
>  	unsigned long za_offset;
>  	unsigned long extra_offset;
>  	unsigned long end_offset;
> @@ -219,6 +220,7 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
>  struct user_ctxs {
>  	struct fpsimd_context __user *fpsimd;
>  	struct sve_context __user *sve;
> +	struct tpidr2_context __user *tpidr2;
>  	struct za_context __user *za;
>  };
>  
> @@ -358,6 +360,32 @@ extern int preserve_sve_context(void __user *ctx);
>  
>  #ifdef CONFIG_ARM64_SME
>  
> +static int preserve_tpidr2_context(struct tpidr2_context __user *ctx)
> +{
> +	int err = 0;
> +
> +	current->thread.tpidr2_el0 = read_sysreg_s(SYS_TPIDR2_EL0);

This is half of tls_preserve_current_state() and may be worth factoring out
so that ptrace and signal delivery can use the same sets of helpers.

> +
> +	__put_user_error(TPIDR2_MAGIC, &ctx->head.magic, err);
> +	__put_user_error(sizeof(*ctx), &ctx->head.size, err);
> +	__put_user_error(current->thread.tpidr2_el0, &ctx->tpidr2, err);
> +
> +	return err;
> +}
> +
> +static int restore_tpidr2_context(struct user_ctxs *user)
> +{
> +	u64 tpidr2_el0;
> +	int err = 0;
> +
> +	/* Magic and size were validated deciding to call this function */

typo: before deciding

> +	__get_user_error(tpidr2_el0, &user->tpidr2->tpidr2, err);
> +	if (!err)
> +		current->thread.tpidr2_el0 = tpidr2_el0;

What guarantees this makes its way into the hardware register before we
return to userspace, context switch or deliver another signal?

> +
> +	return err;
> +}
> +
>  static int preserve_za_context(struct za_context __user *ctx)
>  {
>  	int err = 0;
> @@ -447,6 +475,8 @@ static int restore_za_context(struct user_ctxs *user)
>  #else /* ! CONFIG_ARM64_SME */
>  
>  /* Turn any non-optimised out attempts to use these into a link error: */
> +extern int preserve_tpidr2_context(void __user *ctx);
> +extern int restore_tpidr2_context(struct user_ctxs *user);
>  extern int preserve_za_context(void __user *ctx);
>  extern int restore_za_context(struct user_ctxs *user);
>  
> @@ -465,6 +495,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
>  
>  	user->fpsimd = NULL;
>  	user->sve = NULL;
> +	user->tpidr2 = NULL;
>  	user->za = NULL;
>  
>  	if (!IS_ALIGNED((unsigned long)base, 16))
> @@ -531,6 +562,19 @@ static int parse_user_sigframe(struct user_ctxs *user,
>  			user->sve = (struct sve_context __user *)head;
>  			break;
>  
> +		case TPIDR2_MAGIC:
> +			if (!system_supports_sme())

We seem to be inconsistent between using system_supports_sme() and
system_supports_tpidr2() for tpidr2 checks (see tls_get() and tls_set() for
a glaring example). Please can you tidy this up as a preliminary patch?

> +				goto invalid;
> +
> +			if (user->tpidr2)
> +				goto invalid;
> +
> +			if (size != sizeof(*user->tpidr2))

Why are you requiring an exact match here? Won't that hinder any future
extension of the structure?

Will