[v2,3/6] gdbserver/linux-aarch64: Factor out function to get aarch64_features

  It will be used in a subsequent commit.  There's no functional change.
---
 gdbserver/linux-aarch64-low.cc | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
  

On 11/26/22 02:04, Thiago Jung Bauermann wrote:
> It will be used in a subsequent commit.  There's no functional change.
> ---
>   gdbserver/linux-aarch64-low.cc | 33 +++++++++++++++++++++------------
>   1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index a6ed68f93029..cab4fc0a4674 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -652,6 +652,25 @@ aarch64_target::low_delete_process (arch_process_info *info)
>     xfree (info);
>   }
>   
> +/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> +#define AARCH64_HWCAP_PACA (1 << 30)
> +
> +static struct aarch64_features
> +aarch64_get_arch_features (const thread_info *thread)
> +{
> +  struct aarch64_features features;
> +  int pid = pid_of (thread);
> +
> +  features.vq = aarch64_sve_get_vq (lwpid_of (thread));
> +  /* A-profile PAC is 64-bit only.  */
> +  features.pauth = linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA;
> +  /* A-profile MTE is 64-bit only.  */
> +  features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE;
> +  features.tls = true;
> +
> +  return features;
> +}
> +
>   void
>   aarch64_target::low_new_thread (lwp_info *lwp)
>   {
> @@ -804,9 +823,6 @@ aarch64_adjust_register_sets (const struct aarch64_features &features)
>       }
>   }
>   
> -/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> -#define AARCH64_HWCAP_PACA (1 << 30)
> -
>   /* Implementation of linux target ops method "low_arch_setup".  */
>   
>   void
> @@ -822,15 +838,8 @@ aarch64_target::low_arch_setup ()
>   
>     if (is_elf64)
>       {
> -      struct aarch64_features features;
> -      int pid = pid_of (current_thread);
> -
> -      features.vq = aarch64_sve_get_vq (tid);
> -      /* A-profile PAC is 64-bit only.  */
> -      features.pauth = (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA);
> -      /* A-profile MTE is 64-bit only.  */
> -      features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE;
> -      features.tls = true;
> +      struct aarch64_features features
> +	  = aarch64_get_arch_features (current_thread);
>   
>         current_process ()->tdesc = aarch64_linux_read_description (features);
>   

LGTM

Reviewed-by: Luis Machado <luis.machado@arm.com>
  

On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
> It will be used in a subsequent commit.  There's no functional change.
> ---
>  gdbserver/linux-aarch64-low.cc | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index a6ed68f93029..cab4fc0a4674 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -652,6 +652,25 @@ aarch64_target::low_delete_process (arch_process_info *info)
>    xfree (info);
>  }
>  
> +/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> +#define AARCH64_HWCAP_PACA (1 << 30)
> +
> +static struct aarch64_features
> +aarch64_get_arch_features (const thread_info *thread)

Please document the new function.

> +  struct aarch64_features features;
> +  int pid = pid_of (thread);
> +
> +  features.vq = aarch64_sve_get_vq (lwpid_of (thread));

Just a note, I feel like the pid_of / lwpid_of functions (they used to
be macros) are a vestige of when GDB was in C.  Today, I would have no
problem doing:

  thread->id.pid ()
  thread->id.lwp ()

With the doc change done,

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
  

Luis Machado <luis.machado@arm.com> writes:

> On 11/26/22 02:04, Thiago Jung Bauermann wrote:
>> It will be used in a subsequent commit.  There's no functional change.
>> ---
>>   gdbserver/linux-aarch64-low.cc | 33 +++++++++++++++++++++------------
>>   1 file changed, 21 insertions(+), 12 deletions(-)
>
> LGTM
>
> Reviewed-by: Luis Machado <luis.machado@arm.com>

Thanks! Added the Reviewed-by tag to the patch description for v3.
  

Simon Marchi <simark@simark.ca> writes:

> On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
>> It will be used in a subsequent commit.  There's no functional change.
>> ---
>>  gdbserver/linux-aarch64-low.cc | 33 +++++++++++++++++++++------------
>>  1 file changed, 21 insertions(+), 12 deletions(-)
>> 
>> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
>> index a6ed68f93029..cab4fc0a4674 100644
>> --- a/gdbserver/linux-aarch64-low.cc
>> +++ b/gdbserver/linux-aarch64-low.cc
>> @@ -652,6 +652,25 @@ aarch64_target::low_delete_process (arch_process_info *info)
>>    xfree (info);
>>  }
>>  
>> +/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
>> +#define AARCH64_HWCAP_PACA (1 << 30)
>> +
>> +static struct aarch64_features
>> +aarch64_get_arch_features (const thread_info *thread)
>
> Please document the new function.

Done for v3.

>> +  struct aarch64_features features;
>> +  int pid = pid_of (thread);
>> +
>> +  features.vq = aarch64_sve_get_vq (lwpid_of (thread));
>
> Just a note, I feel like the pid_of / lwpid_of functions (they used to
> be macros) are a vestige of when GDB was in C.  Today, I would have no
> problem doing:
>
>   thread->id.pid ()
>   thread->id.lwp ()

Ah, thanks for the note. I changed occurrences of pid_of and lwpid_of in
this patch and the previous one (which are the only patches in this
series where the functions were added) to the above.

> With the doc change done,
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>

Thanks! Added the Approved-by tag to the patch description for v3.