| Message ID | 1496347928-19432-7-git-send-email-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
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.
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 */