Patchwork [6/6] aarch64: Add hwcap string routines

login
register
mail settings
Submitter Siddhesh Poyarekar
Date June 1, 2017, 8:12 p.m.
Message ID <1496347928-19432-7-git-send-email-siddhesh@sourceware.org>
Download mbox | patch
Permalink /patch/20689/
State New
Headers show

Comments

Siddhesh Poyarekar - June 1, 2017, 8:12 p.m.
Add support for routines in dl-procinfo.h to show string versions of
HWCAP entries when a program is invoked with the LD_SHOW_AUXV
environment variable set and also to aid in path resolution for
ldconfig.

	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
	(_dl_aarch64_cap_flags): New array.
	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
	(_dl_hwcap_string, _dl_string_hwcap, _dl_procinfo): Implement
	functions.
---
 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c | 15 +++++++
 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 58 +++++++++++++++++++++++----
 2 files changed, 65 insertions(+), 8 deletions(-)
Adhemerval Zanella Netto - June 6, 2017, 7:32 p.m.
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> Add support for routines in dl-procinfo.h to show string versions of
> HWCAP entries when a program is invoked with the LD_SHOW_AUXV
> environment variable set and also to aid in path resolution for
> ldconfig.
> 
> 	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> 	(_dl_aarch64_cap_flags): New array.
> 	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> 	(_dl_hwcap_string, _dl_string_hwcap, _dl_procinfo): Implement
> 	functions.

LGTM.  On the subject, I think current dl-procinfo.h macro quite messy 
and error prone, so I think a future cleanup would be worth it.

Since you are there, it would be good to sync with kernel recent updates
for v8.3 [1] [2] [3] which addes HWCAP_JSCVT, HWCAP_FCMA, and HWCAP_LRCPC.

[1] c8c3798d2369e4285da44b244638eafe446a8f8a
[2] cb567e79fa504575cb97fb2f866d2040ed1c92e7
[3] c651aae5a7732287c1c9bc974ece4ed798780544

> ---
>  sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c | 15 +++++++
>  sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 58 +++++++++++++++++++++++----
>  2 files changed, 65 insertions(+), 8 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> index 438046a..bc37bad 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> @@ -56,5 +56,20 @@ PROCINFO_CLASS struct cpu_features _dl_aarch64_cpu_features
>  # endif
>  #endif
>  
> +#if !defined PROCINFO_DECL && defined SHARED
> +  ._dl_aarch64_cap_flags
> +#else
> +PROCINFO_CLASS const char _dl_aarch64_cap_flags[13][10]
> +#endif
> +#ifndef PROCINFO_DECL
> += { "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2", "crc32",
> +    "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm"}
> +#endif
> +#if !defined SHARED || defined PROCINFO_DECL
> +;
> +#else
> +,
> +#endif
> +
>  #undef PROCINFO_DECL
>  #undef PROCINFO_CLASS
> diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> index 7a60d72..cdb36d3 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> @@ -20,25 +20,67 @@
>  #define _DL_PROCINFO_H	1
>  
>  #include <sys/auxv.h>
> +#include <unistd.h>
> +#include <ldsodefs.h>
> +#include <sysdep.h>
>  
>  /* We cannot provide a general printing function.  */
> -#define _dl_procinfo(type, word) -1
> +static inline int
> +__attribute__ ((unused))
> +_dl_procinfo (unsigned int type, unsigned long int word)
> +{
> +  /* This table should match the information from arch/arm64/kernel/cpuinfo.c
> +     in the kernel sources.  */
> +  int i;
>  
> -/* There are no hardware capabilities defined.  */
> -#define _dl_hwcap_string(idx) ""
> +  /* Fallback to unknown output mechanism.  */
> +  if (type == AT_HWCAP2)
> +    return -1;
> +
> +  _dl_printf ("AT_HWCAP:   ");
> +
> +  for (i = 0; i < 32; ++i)
> +    if (word & (1 << i))
> +      _dl_printf (" %s", GLRO(dl_aarch64_cap_flags)[i]);
> +
> +  _dl_printf ("\n");
> +
> +  return 0;
> +}
> +
> +static inline const char *
> +__attribute__ ((unused))
> +_dl_hwcap_string (int idx)
> +{
> +  return GLRO(dl_aarch64_cap_flags)[idx];
> +};
> +
> +
> +/* 13 HWCAP bits set.  */
> +#define _DL_HWCAP_COUNT 13
> +
> +/* Low 13 bits are allocated in HWCAP.  */
> +#define _DL_HWCAP_LAST 12
>  
>  /* HWCAP_CPUID should be available by default to influence IFUNC as well as
>     library search.  */
>  #define HWCAP_IMPORTANT HWCAP_CPUID
>  
> +static inline int
> +__attribute__ ((unused))
> +_dl_string_hwcap (const char *str)
> +{
> +  for (int i = 0; i < _DL_HWCAP_COUNT; i++)
> +    {
> +      if (strcmp (str, _dl_hwcap_string (i)) == 0)
> +	return i;
> +    }
> +  return -1;
> +};
> +
>  /* There're no platforms to filter out.  */
>  #define _DL_HWCAP_PLATFORM 0
>  
> -/* We don't have any hardware capabilities.  */
> -#define _DL_HWCAP_COUNT 0
> -
> -#define _dl_string_hwcap(str) (-1)
> -
>  #define _dl_string_platform(str) (-1)
>  
>  #endif /* dl-procinfo.h */
>
Szabolcs Nagy - June 7, 2017, 3:18 p.m.
On 01/06/17 21:12, Siddhesh Poyarekar wrote:
> Add support for routines in dl-procinfo.h to show string versions of
> HWCAP entries when a program is invoked with the LD_SHOW_AUXV
> environment variable set and also to aid in path resolution for
> ldconfig.
> 
> 	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> 	(_dl_aarch64_cap_flags): New array.
> 	* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> 	(_dl_hwcap_string, _dl_string_hwcap, _dl_procinfo): Implement
> 	functions.
> ---
>  sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c | 15 +++++++
>  sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 58 +++++++++++++++++++++++----
>  2 files changed, 65 insertions(+), 8 deletions(-)
> 

i'm not a fan of magic numbers and strings that
make hwcap flag updates harder.

may be add a note in bits/hwcap.h so whoever
touches that file does not forget to update
dl-procinfo.c ?

> diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> index 438046a..bc37bad 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
> @@ -56,5 +56,20 @@ PROCINFO_CLASS struct cpu_features _dl_aarch64_cpu_features
>  # endif
>  #endif
>  
> +#if !defined PROCINFO_DECL && defined SHARED
> +  ._dl_aarch64_cap_flags
> +#else
> +PROCINFO_CLASS const char _dl_aarch64_cap_flags[13][10]

magic numbers

> +#endif
> +#ifndef PROCINFO_DECL
> += { "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2", "crc32",
> +    "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm"}

strings

...
> diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> index 7a60d72..cdb36d3 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
...
> +static inline int
> +__attribute__ ((unused))
> +_dl_procinfo (unsigned int type, unsigned long int word)
> +{
> +  /* This table should match the information from arch/arm64/kernel/cpuinfo.c
> +     in the kernel sources.  */
> +  int i;
>  
> -/* There are no hardware capabilities defined.  */
> -#define _dl_hwcap_string(idx) ""
> +  /* Fallback to unknown output mechanism.  */
> +  if (type == AT_HWCAP2)
> +    return -1;
> +
> +  _dl_printf ("AT_HWCAP:   ");
> +
> +  for (i = 0; i < 32; ++i)
> +    if (word & (1 << i))
> +      _dl_printf (" %s", GLRO(dl_aarch64_cap_flags)[i]);
> +

1<<31 is ub, use 1UL << i or something

i think word can be proper 64bit number on lp64
so may be the loop should stop at 8 * sizeof word
instead of 32 (or _DL_HWCAP_COUNT or...).

> +  _dl_printf ("\n");
> +
> +  return 0;
> +}
...
> +/* 13 HWCAP bits set.  */
> +#define _DL_HWCAP_COUNT 13
> +
> +/* Low 13 bits are allocated in HWCAP.  */
> +#define _DL_HWCAP_LAST 12
>  

magic numbers.
Andreas Schwab - June 7, 2017, 3:27 p.m.
On Jun 07 2017, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:

>> diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
>> index 7a60d72..cdb36d3 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
> ...
>> +static inline int
>> +__attribute__ ((unused))
>> +_dl_procinfo (unsigned int type, unsigned long int word)
>> +{
>> +  /* This table should match the information from arch/arm64/kernel/cpuinfo.c
>> +     in the kernel sources.  */
>> +  int i;
>>  
>> -/* There are no hardware capabilities defined.  */
>> -#define _dl_hwcap_string(idx) ""
>> +  /* Fallback to unknown output mechanism.  */
>> +  if (type == AT_HWCAP2)
>> +    return -1;
>> +
>> +  _dl_printf ("AT_HWCAP:   ");
>> +
>> +  for (i = 0; i < 32; ++i)
>> +    if (word & (1 << i))
>> +      _dl_printf (" %s", GLRO(dl_aarch64_cap_flags)[i]);
>> +
>
> 1<<31 is ub, use 1UL << i or something

Or (word >> i) & 1.

Andreas.
Siddhesh Poyarekar - June 7, 2017, 4:38 p.m.
On Wednesday 07 June 2017 08:48 PM, Szabolcs Nagy wrote:
> On 01/06/17 21:12, Siddhesh Poyarekar wrote:
> i'm not a fan of magic numbers and strings that
> make hwcap flag updates harder.
> 
> may be add a note in bits/hwcap.h so whoever
> touches that file does not forget to update
> dl-procinfo.c ?

I've already committed this series and will be working on making the
procinfo bits a bit simpler so that they don't have to be maintained as
a separate list like this.

Siddhesh
Szabolcs Nagy - June 9, 2017, 8:24 a.m.
On 07/06/17 17:38, Siddhesh Poyarekar wrote:
> On Wednesday 07 June 2017 08:48 PM, Szabolcs Nagy wrote:
>> On 01/06/17 21:12, Siddhesh Poyarekar wrote:
>> i'm not a fan of magic numbers and strings that
>> make hwcap flag updates harder.
>>
>> may be add a note in bits/hwcap.h so whoever
>> touches that file does not forget to update
>> dl-procinfo.c ?
> 
> I've already committed this series and will be working on making the
> procinfo bits a bit simpler so that they don't have to be maintained as
> a separate list like this.

please fix the undefined behaviour you introduced.

and remove the comments with magic numbers so
on hwcap update comments do not need changes.

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
index 438046a..bc37bad 100644
--- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
+++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c
@@ -56,5 +56,20 @@  PROCINFO_CLASS struct cpu_features _dl_aarch64_cpu_features
 # endif
 #endif
 
+#if !defined PROCINFO_DECL && defined SHARED
+  ._dl_aarch64_cap_flags
+#else
+PROCINFO_CLASS const char _dl_aarch64_cap_flags[13][10]
+#endif
+#ifndef PROCINFO_DECL
+= { "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2", "crc32",
+    "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm"}
+#endif
+#if !defined SHARED || defined PROCINFO_DECL
+;
+#else
+,
+#endif
+
 #undef PROCINFO_DECL
 #undef PROCINFO_CLASS
diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
index 7a60d72..cdb36d3 100644
--- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
@@ -20,25 +20,67 @@ 
 #define _DL_PROCINFO_H	1
 
 #include <sys/auxv.h>
+#include <unistd.h>
+#include <ldsodefs.h>
+#include <sysdep.h>
 
 /* We cannot provide a general printing function.  */
-#define _dl_procinfo(type, word) -1
+static inline int
+__attribute__ ((unused))
+_dl_procinfo (unsigned int type, unsigned long int word)
+{
+  /* This table should match the information from arch/arm64/kernel/cpuinfo.c
+     in the kernel sources.  */
+  int i;
 
-/* There are no hardware capabilities defined.  */
-#define _dl_hwcap_string(idx) ""
+  /* Fallback to unknown output mechanism.  */
+  if (type == AT_HWCAP2)
+    return -1;
+
+  _dl_printf ("AT_HWCAP:   ");
+
+  for (i = 0; i < 32; ++i)
+    if (word & (1 << i))
+      _dl_printf (" %s", GLRO(dl_aarch64_cap_flags)[i]);
+
+  _dl_printf ("\n");
+
+  return 0;
+}
+
+static inline const char *
+__attribute__ ((unused))
+_dl_hwcap_string (int idx)
+{
+  return GLRO(dl_aarch64_cap_flags)[idx];
+};
+
+
+/* 13 HWCAP bits set.  */
+#define _DL_HWCAP_COUNT 13
+
+/* Low 13 bits are allocated in HWCAP.  */
+#define _DL_HWCAP_LAST 12
 
 /* HWCAP_CPUID should be available by default to influence IFUNC as well as
    library search.  */
 #define HWCAP_IMPORTANT HWCAP_CPUID
 
+static inline int
+__attribute__ ((unused))
+_dl_string_hwcap (const char *str)
+{
+  for (int i = 0; i < _DL_HWCAP_COUNT; i++)
+    {
+      if (strcmp (str, _dl_hwcap_string (i)) == 0)
+	return i;
+    }
+  return -1;
+};
+
 /* There're no platforms to filter out.  */
 #define _DL_HWCAP_PLATFORM 0
 
-/* We don't have any hardware capabilities.  */
-#define _DL_HWCAP_COUNT 0
-
-#define _dl_string_hwcap(str) (-1)
-
 #define _dl_string_platform(str) (-1)
 
 #endif /* dl-procinfo.h */