Re: [PATCH v2 3/4] arm64/ptrace: Support access to TPIDR2_EL0

[Luis Machado <luis.machado@arm.com>]

On 8/15/22 14:30, Mark Brown wrote:
> SME introduces an additional EL0 register, TPIDR2_EL0, intended for use
> by userspace as part of the SME. Provide ptrace access to it through the
> existing NT_ARM_TLS regset used for TPIDR_EL0 by expanding it to two
> registers with TPIDR2_EL0 being the second one.
> 
> Existing programs that query the size of the register set will be able
> to observe the increased size of the register set. Programs that assume
> the register set is single register will see no change. On systems that
> do not support SME TPIDR2_EL0 will read as 0 and writes will be ignored,
> support for SME should be queried via hwcaps as normal.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/arm64/kernel/ptrace.c | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 21da83187a60..82feabba3911 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -666,10 +666,18 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
>   static int tls_get(struct task_struct *target, const struct user_regset *regset,
>   		   struct membuf to)
>   {
> +	int ret;
> +
>   	if (target == current)
>   		tls_preserve_current_state();
>   
> -	return membuf_store(&to, target->thread.uw.tp_value);
> +	ret = membuf_store(&to, target->thread.uw.tp_value);
> +	if (system_supports_tpidr2())
> +		ret = membuf_store(&to, target->thread.tpidr2_el0);
> +	else
> +		ret = membuf_zero(&to, sizeof(u64));
> +
> +	return ret;
>   }
>   
>   static int tls_set(struct task_struct *target, const struct user_regset *regset,
> @@ -677,13 +685,20 @@ static int tls_set(struct task_struct *target, const struct user_regset *regset,
>   		   const void *kbuf, const void __user *ubuf)
>   {
>   	int ret;
> -	unsigned long tls = target->thread.uw.tp_value;
> +	unsigned long tls[2];
>   
> -	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &tls, 0, -1);
> +	tls[0] = target->thread.uw.tp_value;
> +	if (system_supports_sme())
> +		tls[1] = target->thread.tpidr2_el0;
> +
> +	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, tls, 0, count);
>   	if (ret)
>   		return ret;
>   
> -	target->thread.uw.tp_value = tls;
> +	target->thread.uw.tp_value = tls[0];
> +	if (system_supports_sme())
> +		target->thread.tpidr2_el0 = tls[1];
> +
>   	return ret;
>   }
>   
> @@ -1392,7 +1407,7 @@ static const struct user_regset aarch64_regsets[] = {
>   	},
>   	[REGSET_TLS] = {
>   		.core_note_type = NT_ARM_TLS,
> -		.n = 1,
> +		.n = 2,
>   		.size = sizeof(void *),
>   		.align = sizeof(void *),
>   		.regset_get = tls_get,

This looks good from GDB's perspective. I tried it with an unpatched GDB and it still works as it should.

TPIDR can be read correctly. I'll check TPIDR2 once it gets implemented.

Thanks,
Luis