Re: [PATCH v2 2/8] AArch64: Use HWCAP to detect pauth feature

[Simon Marchi <simark at simark dot ca>]

On 2019-03-06 8:33 a.m., Alan Hayward wrote:
> Add aarch64_get_hwcap functions for reading the HWCAP.
>  From this extract the PACA value and use this to enable pauth.
>
> gdb/ChangeLog:
>
> 2019-03-06  Alan Hayward  <alan.hayward@arm.com>
> 	    Jiong Wang  <jiong.wang@arm.com>
>
> 	* aarch64-linux-nat.c
> 	(aarch64_linux_nat_target::read_description): Read PACA hwcap.
> 	* aarch64-linux-tdep.c
> 	(aarch64_linux_core_read_description): Likewise.
> 	(aarch64_linux_get_hwcap): New function.
> 	* aarch64-linux-tdep.h (AARCH64_HWCAP_PACA): New define.
> 	(aarch64_linux_get_hwcap): New declaration.
>
> gdb/gdbserver/ChangeLog:
>
> 2019-03-06  Alan Hayward  <alan.hayward@arm.com>
> 	    Jiong Wang  <jiong.wang@arm.com>
>
> 	* linux-aarch64-low.c (AARCH64_HWCAP_PACA): New define.
> 	(aarch64_get_hwcap): New function.
> 	(aarch64_arch_setup): Read APIA hwcap.
> ---
>   gdb/aarch64-linux-nat.c           |  7 +++++--
>   gdb/aarch64-linux-tdep.c          | 16 +++++++++++----
>   gdb/aarch64-linux-tdep.h          |  6 ++++++
>   gdb/gdbserver/linux-aarch64-low.c | 33 +++++++++++++++++++++++++++++--
>   4 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index f58a41e195..8a7006165e 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -606,8 +606,11 @@ aarch64_linux_nat_target::read_description ()
>     if (ret == 0)
>       return tdesc_arm_with_neon;
>   
> -  /* pauth not yet supported.  */
> -  return aarch64_read_description (aarch64_sve_get_vq (tid), false);
> +  CORE_ADDR hwcap = 0;
> +  bool pauth_p = aarch64_linux_get_hwcap (&hwcap)
> +		 && (hwcap & AARCH64_HWCAP_PACA);
> +
> +  return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p);
>   }
>   
>   /* Convert a native/host siginfo object, into/from the siginfo in the
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 445019accc..5eeafa456c 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -637,12 +637,11 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>   {
>     CORE_ADDR aarch64_hwcap = 0;
>   
> -  if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
> -    return NULL;
> +  if (!aarch64_linux_get_hwcap (&aarch64_hwcap))
> +    return nullptr;
>   
> -  /* pauth not yet supported.  */
>     return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
> -				   false);
> +				   aarch64_hwcap & AARCH64_HWCAP_PACA);
>   }
>   
>   /* Implementation of `gdbarch_stap_is_single_operand', as defined in
> @@ -1420,6 +1419,15 @@ aarch64_linux_gcc_target_options (struct gdbarch *gdbarch)
>     return NULL;
>   }
>   
> +/* See aarch64-linux-tdep.h.  */
> +
> +bool
> +aarch64_linux_get_hwcap (CORE_ADDR *hwcap)
> +{
> +  *hwcap = 0;
> +  return target_auxv_search (current_top_target (), AT_HWCAP, hwcap) == 1;
> +}
> +

I don't know if it actually matters, but when the target_aux_search call 
was in aarch64_linux_core_read_description, it used the TARGET 
parameter, rather than using current_top_target.  So, perhaps that new 
function should take a target_ops parameter and use it?  The other call 
in aarch64_linux_nat_target::read_description would pass "this".

And this function does nothing AArch64-specific, so I think it would be 
nice to have it in linux-tdep.{h,c} and update other arches to use it. 
But I would say, don't bother with that in this series, we can do it after.

Simon