[6/6] aarch64: Add hwcap string routines
Commit Message
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
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 */
>
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.
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.
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
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.
@@ -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
@@ -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 */