diff mbox

[6/6] aarch64: Add hwcap string routines

Message ID 1496347928-19432-7-git-send-email-siddhesh@sourceware.org
State New, archived
Headers

Commit Message

Siddhesh Poyarekar June 1, 2017, 8:12 p.m. UTC 
  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(-)

Comments

Adhemerval Zanella June 6, 2017, 7:32 p.m. UTC | #1 
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. UTC | #2 
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. UTC | #3 
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. UTC | #4 
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. UTC | #5 
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.
diff mbox

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 */