* [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
` (4 preceding siblings ...)
2017-06-01 20:12 ` [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 19:32 ` Adhemerval Zanella
2017-06-07 15:18 ` Szabolcs Nagy
2017-06-05 16:28 ` [Ping] [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
6 siblings, 2 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
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(-)
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 */
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
` (2 preceding siblings ...)
2017-06-01 20:12 ` [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 18:18 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK Siddhesh Poyarekar
` (2 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
Drop _dl_hwcap_mask when building with tunables. This completes the
transition of hwcap_mask reading from _dl_hwcap_mask to tunables.
* elf/dl-hwcaps.h: New file.
* elf/dl-hwcaps.c: Include it.
(_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update
glibc.tune.hwcap_mask.
* elf/dl-cache.c: Include dl-hwcaps.h.
(_dl_load_cache_lookup)[HAVE_TUNABLES]: Read
glibc.tune.hwcap_mask.
* sysdeps/sparc/sparc32/dl-machine.h: Likewise.
* elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop
_dl_hwcap_mask.
* elf/rtld.c (rtld_global_ro)[HAVE_TUNABLES]: Drop
_dl_hwcap_mask.
(process_envvars)[HAVE_TUNABLES]: Likewise.
* sysdeps/generic/ldsodefs.h (rtld_global_ro)[HAVE_TUNABLES]:
Likewise.
* sysdeps/x86/cpu-features.c (init_cpu_features): Don't
initialize dl_hwcap_mask when tunables are enabled.
---
elf/dl-cache.c | 5 ++++-
elf/dl-hwcaps.c | 11 +++++++++--
elf/dl-hwcaps.h | 31 +++++++++++++++++++++++++++++++
elf/dl-support.c | 2 ++
elf/rtld.c | 4 ++++
sysdeps/generic/ldsodefs.h | 2 ++
sysdeps/sparc/sparc32/dl-machine.h | 4 +++-
sysdeps/x86/cpu-features.c | 4 ++++
8 files changed, 59 insertions(+), 4 deletions(-)
create mode 100644 elf/dl-hwcaps.h
diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 017c78a..e9632da 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -24,6 +24,7 @@
#include <dl-procinfo.h>
#include <stdint.h>
#include <_itoa.h>
+#include <dl-hwcaps.h>
#ifndef _DL_PLATFORMS_COUNT
# define _DL_PLATFORMS_COUNT 0
@@ -258,8 +259,10 @@ _dl_load_cache_lookup (const char *name)
if (platform != (uint64_t) -1)
platform = 1ULL << platform;
+ uint64_t hwcap_mask = GET_HWCAP_MASK();
+
#define _DL_HWCAP_TLS_MASK (1LL << 63)
- uint64_t hwcap_exclude = ~((GLRO(dl_hwcap) & GLRO(dl_hwcap_mask))
+ uint64_t hwcap_exclude = ~((GLRO(dl_hwcap) & hwcap_mask)
| _DL_HWCAP_PLATFORM | _DL_HWCAP_TLS_MASK);
/* Only accept hwcap if it's for the right platform. */
diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c
index c437397..ac50fd2 100644
--- a/elf/dl-hwcaps.c
+++ b/elf/dl-hwcaps.c
@@ -24,6 +24,7 @@
#include <ldsodefs.h>
#include <dl-procinfo.h>
+#include <dl-hwcaps.h>
#ifdef _DL_FIRST_PLATFORM
# define _DL_FIRST_EXTRA (_DL_FIRST_PLATFORM + _DL_PLATFORMS_COUNT)
@@ -37,8 +38,9 @@ internal_function
_dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
size_t *max_capstrlen)
{
+ uint64_t hwcap_mask = GET_HWCAP_MASK();
/* Determine how many important bits are set. */
- uint64_t masked = GLRO(dl_hwcap) & GLRO(dl_hwcap_mask);
+ uint64_t masked = GLRO(dl_hwcap) & hwcap_mask;
size_t cnt = platform != NULL;
size_t n, m;
size_t total;
@@ -125,7 +127,12 @@ _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
LD_HWCAP_MASK environment variable (or default HWCAP_IMPORTANT).
So there is no way to request ignoring an OS-supplied dsocap
string and bit like you can ignore an OS-supplied HWCAP bit. */
- GLRO(dl_hwcap_mask) |= (uint64_t) mask << _DL_FIRST_EXTRA;
+ hwcap_mask |= (uint64_t) mask << _DL_FIRST_EXTRA;
+#if HAVE_TUNABLES
+ TUNABLE_SET (glibc, tune, hwcap_mask, uint64_t, hwcap_mask);
+#else
+ GLRO(dl_hwcap_mask) = hwcap_mask;
+#endif
size_t len;
for (const char *p = dsocaps; p < dsocaps + dsocapslen; p += len + 1)
{
diff --git a/elf/dl-hwcaps.h b/elf/dl-hwcaps.h
new file mode 100644
index 0000000..9ce3317
--- /dev/null
+++ b/elf/dl-hwcaps.h
@@ -0,0 +1,31 @@
+/* Hardware capability support for run-time dynamic loader.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <elf/dl-tunables.h>
+
+#ifdef SHARED
+# if HAVE_TUNABLES
+# define GET_HWCAP_MASK() \
+ TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
+# else
+# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
+# endif
+#else
+/* HWCAP_MASK is ignored in static binaries. */
+# define GET_HWCAP_MASK() (0)
+#endif
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 3c46a7a..c22be85 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -164,6 +164,7 @@ uint64_t _dl_hwcap2 __attribute__ ((nocommon));
/* The value of the FPU control word the kernel will preset in hardware. */
fpu_control_t _dl_fpu_control = _FPU_DEFAULT;
+#if !HAVE_TUNABLES
/* This is not initialized to HWCAP_IMPORTANT, matching the definition
of _dl_important_hwcaps, below, where no hwcap strings are ever
used. This mask is still used to mediate the lookups in the cache
@@ -171,6 +172,7 @@ fpu_control_t _dl_fpu_control = _FPU_DEFAULT;
LD_HWCAP_MASK environment variable here), there is no real point in
setting _dl_hwcap nonzero below, but we do anyway. */
uint64_t _dl_hwcap_mask __attribute__ ((nocommon));
+#endif
/* Prevailing state of the stack. Generally this includes PF_X, indicating it's
* executable but this isn't true for all platforms. */
diff --git a/elf/rtld.c b/elf/rtld.c
index 319ef06..3746653 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -159,7 +159,9 @@ struct rtld_global_ro _rtld_global_ro attribute_relro =
._dl_debug_fd = STDERR_FILENO,
._dl_use_load_bias = -2,
._dl_correct_cache_id = _DL_CACHE_DEFAULT_ID,
+#if !HAVE_TUNABLES
._dl_hwcap_mask = HWCAP_IMPORTANT,
+#endif
._dl_lazy = 1,
._dl_fpu_control = _FPU_DEFAULT,
._dl_pagesize = EXEC_PAGESIZE,
@@ -2402,6 +2404,7 @@ process_envvars (enum mode *modep)
_dl_show_auxv ();
break;
+#if !HAVE_TUNABLES
case 10:
/* Mask for the important hardware capabilities. */
if (!__libc_enable_secure
@@ -2409,6 +2412,7 @@ process_envvars (enum mode *modep)
GLRO(dl_hwcap_mask) = __strtoul_internal (&envline[11], NULL,
0, 0);
break;
+#endif
case 11:
/* Path where the binary is found. */
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index f26a8b2..695ac24 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -515,8 +515,10 @@ struct rtld_global_ro
/* Mask for hardware capabilities that are available. */
EXTERN uint64_t _dl_hwcap;
+#if !HAVE_TUNABLES
/* Mask for important hardware capabilities we honour. */
EXTERN uint64_t _dl_hwcap_mask;
+#endif
#ifdef HAVE_AUX_VECTOR
/* Pointer to the auxv list supplied to the program at startup. */
diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
index e17ac8e..f9ae133 100644
--- a/sysdeps/sparc/sparc32/dl-machine.h
+++ b/sysdeps/sparc/sparc32/dl-machine.h
@@ -27,6 +27,7 @@
#include <sysdep.h>
#include <tls.h>
#include <dl-plt.h>
+#include <elf/dl-hwcaps.h>
/* Return nonzero iff ELF header is compatible with the running host. */
static inline int
@@ -37,7 +38,8 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
else if (ehdr->e_machine == EM_SPARC32PLUS)
{
#ifdef SHARED
- return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9;
+ uint64_t hwcap_mask = GET_HWCAP_MASK();
+ return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9;
#else
return GLRO(dl_hwcap) & HWCAP_SPARC_V9;
#endif
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index b481f50..4fe58bf 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -316,7 +316,11 @@ no_cpuid:
/* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
GLRO(dl_platform) = NULL;
GLRO(dl_hwcap) = 0;
+#if !HAVE_TUNABLES
+ /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
+ this. */
GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
+#endif
# ifdef __x86_64__
if (cpu_features->kind == arch_kind_intel)
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check
@ 2017-06-01 20:12 Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries Siddhesh Poyarekar
` (6 more replies)
0 siblings, 7 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
Hi,
Here is another iteration of the patch to move LD_HWCAP_MASK into tunables
and then using it to disable HWCAP_CPUID on aarch64. There is just one
change from the previous version:
The first patch now reworks the internal tunables API to make it more
straightforward with just TUNABLE_GET and TUNABLE_SET instead of the
arbitrary TUNABLE_SET, TUNABLE_GET, TUNABLE_UPDATE, etc. The API
should now be more straightforward and predictable to use. This also
fixes the problem of hwcap_mask not being read when it is not set
externally.
Testing:
- The patches don't introduce any testsuite regressions on x86_64 and
aarch64
- I ran the dynamic linker under gdb and verified that the hwcap_mask is
read correctly and that MIDR is read correctly
- I ran 'LD_SHOW_AUXV=1 elf/ld.so --library-path .:elf:nptl /bin/true'
to verify that the hwcaps are read correctly.
Siddhesh Poyarekar (6):
tunables: Clean up hooks to get and set tunables
tunables: Add LD_HWCAP_MASK to tunables
tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask
aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
Make LD_HWCAP_MASK usable for static binaries
aarch64: Add hwcap string routines
README.tunables | 69 +++++++++++++++++----
elf/Versions | 2 +-
elf/dl-cache.c | 5 +-
elf/dl-hwcaps.c | 11 +++-
elf/dl-hwcaps.h | 30 +++++++++
elf/dl-support.c | 2 +
elf/dl-tunables.c | 58 ++++++++++-------
elf/dl-tunables.h | 53 ++++++++++------
elf/dl-tunables.list | 7 +++
elf/rtld.c | 4 ++
malloc/arena.c | 20 +++---
manual/tunables.texi | 23 +++++++
scripts/gen-tunables.awk | 1 +
sysdeps/generic/ldsodefs.h | 2 +
sysdeps/sparc/sparc32/dl-machine.h | 6 +-
sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 10 +--
sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c | 15 +++++
sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 86 ++++++++++++++++++++++++++
sysdeps/x86/cpu-features.c | 10 +--
19 files changed, 339 insertions(+), 75 deletions(-)
create mode 100644 elf/dl-hwcaps.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 19:06 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 1/6] tunables: Clean up hooks to get and set tunables Siddhesh Poyarekar
` (5 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
The LD_HWCAP_MASK environment variable was ignored in static binaries,
which is inconsistent with the behaviour of dynamically linked
binaries. This seems to have been because of the inability of
ld_hwcap_mask being read early enough to influence anything but now
that it is in tunables, the mask is usable in static binaries as well.
This feature is important for aarch64, which relies on HWCAP_CPUID
being masked out to disable multiarch. A sanity test on x86_64 shows
that there are no failures. Likewise for aarch64.
* elf/dl-hwcaps.h [HAVE_TUNABLES]: Always read hwcap_mask.
* sysdeps/sparc/sparc32/dl-machine.h [HAVE_TUNABLES]:
Likewise.
* sysdeps/x86/cpu-features.c (init_cpu_features): Always set
up hwcap and hwcap_mask.
---
elf/dl-hwcaps.h | 15 +++++++--------
sysdeps/sparc/sparc32/dl-machine.h | 2 +-
sysdeps/x86/cpu-features.c | 8 +++-----
3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/elf/dl-hwcaps.h b/elf/dl-hwcaps.h
index 9ce3317..2c4fa3d 100644
--- a/elf/dl-hwcaps.h
+++ b/elf/dl-hwcaps.h
@@ -18,14 +18,13 @@
#include <elf/dl-tunables.h>
-#ifdef SHARED
-# if HAVE_TUNABLES
-# define GET_HWCAP_MASK() \
- TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
+#if HAVE_TUNABLES
+# define GET_HWCAP_MASK() TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
+#else
+# ifdef SHARED
+# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
# else
-# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
+/* HWCAP_MASK is ignored in static binaries when built without tunables. */
+# define GET_HWCAP_MASK() (0)
# endif
-#else
-/* HWCAP_MASK is ignored in static binaries. */
-# define GET_HWCAP_MASK() (0)
#endif
diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
index f9ae133..95f6732 100644
--- a/sysdeps/sparc/sparc32/dl-machine.h
+++ b/sysdeps/sparc/sparc32/dl-machine.h
@@ -37,7 +37,7 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
return 1;
else if (ehdr->e_machine == EM_SPARC32PLUS)
{
-#ifdef SHARED
+#if HAVE_TUNABLES || defined SHARED
uint64_t hwcap_mask = GET_HWCAP_MASK();
return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9;
#else
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 4fe58bf..4288001 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -312,17 +312,16 @@ no_cpuid:
cpu_features->model = model;
cpu_features->kind = kind;
-#if IS_IN (rtld)
/* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
GLRO(dl_platform) = NULL;
GLRO(dl_hwcap) = 0;
-#if !HAVE_TUNABLES
+#if !HAVE_TUNABLES && defined SHARED
/* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
this. */
GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
#endif
-# ifdef __x86_64__
+#ifdef __x86_64__
if (cpu_features->kind == arch_kind_intel)
{
if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
@@ -352,7 +351,7 @@ no_cpuid:
&& CPU_FEATURES_CPU_P (cpu_features, POPCNT))
GLRO(dl_platform) = "haswell";
}
-# else
+#else
if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
GLRO(dl_hwcap) |= HWCAP_X86_SSE2;
@@ -360,6 +359,5 @@ no_cpuid:
GLRO(dl_platform) = "i686";
else if (CPU_FEATURES_ARCH_P (cpu_features, I586))
GLRO(dl_platform) = "i586";
-# endif
#endif
}
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
` (3 preceding siblings ...)
2017-06-01 20:12 ` [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 18:24 ` Adhemerval Zanella
2017-06-07 15:00 ` Szabolcs Nagy
2017-06-01 20:12 ` [PATCH 6/6] aarch64: Add hwcap string routines Siddhesh Poyarekar
2017-06-05 16:28 ` [Ping] [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
6 siblings, 2 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
Now that LD_HWCAP_MASK (or glibc.tune.hwcap_mask) is read early enough
to influence cpu feature check in aarch64, use it to influence
multiarch selection. Setting LD_HWCAP_MASK such that it clears
HWCAP_CPUID will now disable multiarch for the binary.
HWCAP_CPUID is also now set in HWCAP_IMPORTANT so that it is set by
default. With this patch, this feature is only usable with
dyanmically linked binaries because LD_HWCAP_MASK is not read for
static binaries. A future patch fixes that.
* sysdeps/unix/sysv/linux/aarch64/cpu-features.c
(init_cpu_features): Use glibc.tune.hwcap_mask.
* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h: New file.
---
sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 10 +++---
sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 44 ++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 4 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 7025062..ef6eecd 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -18,18 +18,20 @@
#include <cpu-features.h>
#include <sys/auxv.h>
+#include <elf/dl-hwcaps.h>
static inline void
init_cpu_features (struct cpu_features *cpu_features)
{
- if (GLRO(dl_hwcap) & HWCAP_CPUID)
+ uint64_t hwcap_mask = GET_HWCAP_MASK();
+ uint64_t hwcap = GLRO (dl_hwcap) & hwcap_mask;
+
+ if (hwcap & HWCAP_CPUID)
{
register uint64_t id = 0;
asm volatile ("mrs %0, midr_el1" : "=r"(id));
cpu_features->midr_el1 = id;
}
else
- {
- cpu_features->midr_el1 = 0;
- }
+ cpu_features->midr_el1 = 0;
}
diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
new file mode 100644
index 0000000..7a60d72
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
@@ -0,0 +1,44 @@
+/* Processor capability information handling macros - aarch64 version.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _DL_PROCINFO_H
+#define _DL_PROCINFO_H 1
+
+#include <sys/auxv.h>
+
+/* We cannot provide a general printing function. */
+#define _dl_procinfo(type, word) -1
+
+/* There are no hardware capabilities defined. */
+#define _dl_hwcap_string(idx) ""
+
+/* HWCAP_CPUID should be available by default to influence IFUNC as well as
+ library search. */
+#define HWCAP_IMPORTANT HWCAP_CPUID
+
+/* 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 */
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/6] tunables: Clean up hooks to get and set tunables
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 17:37 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables Siddhesh Poyarekar
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
The TUNABLE_SET_VALUE and family of macros (and my later attempt to
add a TUNABLE_GET) never quite went together very well because the
overall interface was not clearly defined. This patch is an attempt
to do just that.
This patch consolidates the API to two simple sets of macros,
TUNABLE_GET* and TUNABLE_SET*. If TUNABLE_NAMESPACE is defined,
TUNABLE_GET takes just the tunable name, type and a (optionally NULL)
callback function to get the value of the tunable. The callback
function, if non-NULL, is called if the tunable was externally set
(i.e. via GLIBC_TUNABLES or any future mechanism). For example:
val = TUNABLE_GET (check, int32_t, check_callback)
returns the value of the glibc.malloc.check tunable (assuming
TUNABLE_NAMESPACE is set to malloc) as an int32_t into VAL after
calling check_callback.
Likewise, TUNABLE_SET can be used to set the value of the tunable,
although this is currently possible only in the dynamic linker before
it relocates itself. For example:
TUNABLE_SET (check, int32_t, 2)
will set glibc.malloc.check to 2. Of course, this is not possible
since we set (or read) glibc.malloc.check long after it is relocated.
To access or set a tunable outside of TUNABLE_NAMESPACE, use the
TUNABLE_GET_FULL and TUNABLE_SET_FULL macros, which have the following
prototype:
TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, 0xffff)
In future the tunable list may get split into mutable and immutable
tunables where mutable tunables can be modified by the library and
userspace after relocation as well and TUNABLE_SET will be more useful
than it currently is. However whenever we actually do that split, we
will have to ensure that the mutable tunables are protected with
locks.
* elf/Versions (__tunable_set_val): Rename to __tunable_get_val.
* elf/dl-tunables.c: Likewise.
(do_tunable_update_val): New function.
(__tunable_set_val): New function.
(__tunable_get_val): Call CB only if the tunable was externally
initialized.
(tunables_strtoul): Replace strval with initialized.
* elf/dl-tunables.h (strval): Replace with a bool initialized.
(TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
prevent collision.
(__tunable_set_val): New function.
(TUNABLE_GET, TUNABLE_GET_FULL): New macros.
(TUNABLE_SET, TUNABLE_SET_FULL): Likewise.
(TUNABLE_SET_VAL): Remove.
(TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
* README.tunables: Document the new macros.
* malloc/arena.c (ptmalloc_init): Adjust.
---
README.tunables | 69 ++++++++++++++++++++++++++++++++++++++++++++++---------
elf/Versions | 2 +-
elf/dl-tunables.c | 58 +++++++++++++++++++++++++++++-----------------
elf/dl-tunables.h | 53 +++++++++++++++++++++++++++---------------
malloc/arena.c | 20 ++++++++--------
5 files changed, 140 insertions(+), 62 deletions(-)
diff --git a/README.tunables b/README.tunables
index 0e9b0d7..6fcfd2d 100644
--- a/README.tunables
+++ b/README.tunables
@@ -20,14 +20,12 @@ namespace by overriding the TOP_NAMESPACE macro for that tunable. Downstream
implementations are discouraged from using the 'glibc' top namespace for
tunables they don't already have consensus to push upstream.
-There are two steps to adding a tunable:
+There are three steps to adding a tunable:
-1. Add a tunable ID:
+1. Add a tunable to the list and fully specify its properties:
-Modules that wish to use the tunables interface must define the
-TUNABLE_NAMESPACE macro. Following this, for each tunable you want to
-add, make an entry in elf/dl-tunables.list. The format of the file is as
-follows:
+For each tunable you want to add, make an entry in elf/dl-tunables.list. The
+format of the file is as follows:
TOP_NAMESPACE {
NAMESPACE1 {
@@ -69,11 +67,60 @@ The list of allowed attributes are:
non-AT_SECURE subprocesses.
NONE: Read all the time.
-2. Call either the TUNABLE_SET_VALUE and pass into it the tunable name and a
- pointer to the variable that should be set with the tunable value.
- If additional work needs to be done after setting the value, use the
- TUNABLE_SET_VALUE_WITH_CALLBACK instead and additionally pass a pointer to
- the function that should be called if the tunable value has been set.
+2. Use TUNABLE_GET/TUNABLE_SET to get and set tunables.
+
+3. OPTIONAL: If tunables in a namespace are being used multiple times within a
+ specific module, set the TUNABLE_NAMESPACE macro to reduce the amount of
+ typing.
+
+GETTING AND SETTING TUNABLES
+----------------------------
+
+When the TUNABLE_NAMESPACE macro is defined, one may get tunables in that
+module using the TUNABLE_GET macro as follows:
+
+ val = TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (check_callback))
+
+where 'check' is the tunable name, 'int32_t' is the C type of the tunable and
+'check_callback' is the function to call if the tunable got initialized to a
+non-default value. The macro returns the value as type 'int32_t'.
+
+The callback function should be defined as follows:
+
+ void
+ DL_TUNABLE_CALLBACK (check_callback) (int32_t *valp)
+ {
+ ...
+ }
+
+where it can expect the tunable value to be passed in VALP.
+
+Tunables in the module can be updated using:
+
+ TUNABLE_SET (check, int32_t, val)
+
+where 'check' is the tunable name, 'int32_t' is the C type of the tunable and
+'val' is a value of same type.
+
+To get and set tunables in a different namespace from that module, use the full
+form of the macros as follows:
+
+ val = TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
+
+ TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, val)
+
+where 'glibc' is the top namespace, 'tune' is the tunable namespace and the
+remaining arguments are the same as the short form macros.
+
+When TUNABLE_NAMESPACE is not defined in a module, TUNABLE_GET is equivalent to
+TUNABLE_GET_FULL, so you will need to provide full namespace information for
+both macros. Likewise for TUNABLE_SET and TUNABLE_SET_FULL.
+
+** IMPORTANT NOTE **
+
+The tunable list is set as read-only after the dynamic linker relocates itself,
+so setting tunable values must be limited only to tunables within the dynamic
+linker, that too before relocation.
FUTURE WORK
-----------
diff --git a/elf/Versions b/elf/Versions
index 6abe9db..c59facd 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -72,6 +72,6 @@ ld {
_dl_signal_error; _dl_catch_error;
# Set value of a tunable.
- __tunable_set_val;
+ __tunable_get_val;
}
}
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index b6e6b3d..76e8c5c 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -184,19 +184,17 @@ tunables_strtoul (const char *nptr)
if ((__type) (__val) >= min && (__type) (val) <= max) \
{ \
(__cur)->val.numval = val; \
- (__cur)->strval = strval; \
+ (__cur)->initialized = true; \
} \
})
-/* Validate range of the input value and initialize the tunable CUR if it looks
- good. */
static void
-tunable_initialize (tunable_t *cur, const char *strval)
+do_tunable_update_val (tunable_t *cur, const void *valp)
{
uint64_t val;
if (cur->type.type_code != TUNABLE_TYPE_STRING)
- val = tunables_strtoul (strval);
+ val = *((int64_t *) valp);
switch (cur->type.type_code)
{
@@ -217,7 +215,7 @@ tunable_initialize (tunable_t *cur, const char *strval)
}
case TUNABLE_TYPE_STRING:
{
- cur->val.strval = cur->strval = strval;
+ cur->val.strval = valp;
break;
}
default:
@@ -225,6 +223,35 @@ tunable_initialize (tunable_t *cur, const char *strval)
}
}
+/* Validate range of the input value and initialize the tunable CUR if it looks
+ good. */
+static void
+tunable_initialize (tunable_t *cur, const char *strval)
+{
+ uint64_t val;
+ const void *valp;
+
+ if (cur->type.type_code != TUNABLE_TYPE_STRING)
+ {
+ val = tunables_strtoul (strval);
+ valp = &val;
+ }
+ else
+ {
+ cur->initialized = true;
+ valp = strval;
+ }
+ do_tunable_update_val (cur, valp);
+}
+
+void
+__tunable_set_val (tunable_id_t id, void *valp)
+{
+ tunable_t *cur = &tunable_list[id];
+
+ do_tunable_update_val (cur, valp);
+}
+
#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
/* Parse the tunable string TUNESTR and adjust it to drop any tunables that may
be unsafe for AT_SECURE processes so that it can be used as the new
@@ -375,7 +402,7 @@ __tunables_init (char **envp)
/* Skip over tunables that have either been set already or should be
skipped. */
- if (cur->strval != NULL || cur->env_alias == NULL)
+ if (cur->initialized || cur->env_alias == NULL)
continue;
const char *name = cur->env_alias;
@@ -426,20 +453,10 @@ __tunables_init (char **envp)
/* Set the tunable value. This is called by the module that the tunable exists
in. */
void
-__tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
+__tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
{
tunable_t *cur = &tunable_list[id];
- /* Don't do anything if our tunable was not set during initialization or if
- it failed validation. */
- if (cur->strval == NULL)
- return;
-
- /* Caller does not need the value, just call the callback with our tunable
- value. */
- if (valp == NULL)
- goto cb;
-
switch (cur->type.type_code)
{
case TUNABLE_TYPE_UINT_64:
@@ -466,9 +483,8 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
__builtin_unreachable ();
}
-cb:
- if (callback)
+ if (cur->initialized && callback != NULL)
callback (&cur->val);
}
-rtld_hidden_def (__tunable_set_val)
+rtld_hidden_def (__tunable_get_val)
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 20ee512..6c49dcb 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -39,8 +39,8 @@ struct _tunable
const char *name; /* Internal name of the tunable. */
tunable_type_t type; /* Data type of the tunable. */
tunable_val_t val; /* The value. */
- const char *strval; /* The string containing the value,
- points into envp. */
+ bool initialized; /* Flag to indicate that the tunable is
+ initialized. */
tunable_seclevel_t security_level; /* Specify the security level for the
tunable with respect to AT_SECURE
programs. See description of
@@ -61,37 +61,52 @@ typedef struct _tunable tunable_t;
/* Full name for a tunable is top_ns.tunable_ns.id. */
# define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id
-# define TUNABLE_ENUM_NAME(top,ns,id) TUNABLE_ENUM_NAME1 (top,ns,id)
-# define TUNABLE_ENUM_NAME1(top,ns,id) top ## _ ## ns ## _ ## id
+# define TUNABLE_ENUM_NAME(__top,__ns,__id) TUNABLE_ENUM_NAME1 (__top,__ns,__id)
+# define TUNABLE_ENUM_NAME1(__top,__ns,__id) __top ## _ ## __ns ## _ ## __id
# include "dl-tunable-list.h"
extern void __tunables_init (char **);
-extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
-
+extern void __tunable_get_val (tunable_id_t, void *, tunable_callback_t);
+extern void __tunable_set_val (tunable_id_t, void *);
rtld_hidden_proto (__tunables_init)
-rtld_hidden_proto (__tunable_set_val)
+rtld_hidden_proto (__tunable_get_val)
-/* Check if the tunable has been set to a non-default value and if it is, copy
- it over into __VAL. */
-# define TUNABLE_SET_VAL(__id,__val) \
+/* Define TUNABLE_GET and TUNABLE_SET in short form if TOP_NAMESPACE and
+ TUNABLE_NAMESPACE are defined. This is useful shorthand to get and set
+ tunables within a module. */
+#if defined TOP_NAMESPACE && defined TUNABLE_NAMESPACE
+# define TUNABLE_GET(__id, __type, __cb) \
+ TUNABLE_GET_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, __type, __cb)
+# define TUNABLE_SET(__id, __type, __val) \
+ TUNABLE_SET_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, __type, __val)
+#else
+# define TUNABLE_GET(__top, __ns, __id, __type, __cb) \
+ TUNABLE_GET_FULL (__top, __ns, __id, __type, __cb)
+# define TUNABLE_SET(__top, __ns, __id, __type, __val) \
+ TUNABLE_SET_FULL (__top, __ns, __id, __type, __val)
+#endif
+
+/* Get and return a tunable value. If the tunable was set externally and __CB
+ is defined then call __CB before returning the value. */
+# define TUNABLE_GET_FULL(__top, __ns, __id, __type, __cb) \
({ \
- __tunable_set_val \
- (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \
- NULL); \
+ tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id); \
+ __type ret; \
+ __tunable_get_val (id, &ret, __cb); \
+ ret; \
})
-/* Same as TUNABLE_SET_VAL, but also call the callback function __CB. */
-# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
+/* Set a tunable value. */
+# define TUNABLE_SET_FULL(__top, __ns, __id, __type, __val) \
({ \
- __tunable_set_val \
- (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \
- DL_TUNABLE_CALLBACK (__cb)); \
+ __tunable_set_val (TUNABLE_ENUM_NAME (__top, __ns, __id), \
+ & (__type) {__val}); \
})
/* Namespace sanity for callback functions. Use this macro to keep the
namespace of the modules clean. */
-# define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
+# define TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
# define TUNABLES_FRONTEND_valstring 1
/* The default value for TUNABLES_FRONTEND. */
diff --git a/malloc/arena.c b/malloc/arena.c
index d49e4a2..ae75c06 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -212,7 +212,7 @@ __malloc_fork_unlock_child (void)
#if HAVE_TUNABLES
static inline int do_set_mallopt_check (int32_t value);
void
-DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
+TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
{
int32_t value = (int32_t) valp->numval;
do_set_mallopt_check (value);
@@ -223,7 +223,7 @@ DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \
static inline int do_ ## __name (__type value); \
void \
-DL_TUNABLE_CALLBACK (__name) (tunable_val_t *valp) \
+TUNABLE_CALLBACK (__name) (tunable_val_t *valp) \
{ \
__type value = (__type) (valp)->numval; \
do_ ## __name (value); \
@@ -314,14 +314,14 @@ ptmalloc_init (void)
__libc_lock_lock (main_arena.mutex);
malloc_consolidate (&main_arena);
- TUNABLE_SET_VAL_WITH_CALLBACK (check, NULL, set_mallopt_check);
- TUNABLE_SET_VAL_WITH_CALLBACK (top_pad, NULL, set_top_pad);
- TUNABLE_SET_VAL_WITH_CALLBACK (perturb, NULL, set_perturb_byte);
- TUNABLE_SET_VAL_WITH_CALLBACK (mmap_threshold, NULL, set_mmap_threshold);
- TUNABLE_SET_VAL_WITH_CALLBACK (trim_threshold, NULL, set_trim_threshold);
- TUNABLE_SET_VAL_WITH_CALLBACK (mmap_max, NULL, set_mmaps_max);
- TUNABLE_SET_VAL_WITH_CALLBACK (arena_max, NULL, set_arena_max);
- TUNABLE_SET_VAL_WITH_CALLBACK (arena_test, NULL, set_arena_test);
+ TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
+ TUNABLE_GET (top_pad, size_t, TUNABLE_CALLBACK (set_top_pad));
+ TUNABLE_GET (perturb, int32_t, TUNABLE_CALLBACK (set_perturb_byte));
+ TUNABLE_GET (mmap_threshold, size_t, TUNABLE_CALLBACK (set_mmap_threshold));
+ TUNABLE_GET (trim_threshold, size_t, TUNABLE_CALLBACK (set_trim_threshold));
+ TUNABLE_GET (mmap_max, int32_t, TUNABLE_CALLBACK (set_mmaps_max));
+ TUNABLE_GET (arena_max, size_t, TUNABLE_CALLBACK (set_arena_max));
+ TUNABLE_GET (arena_test, size_t, TUNABLE_CALLBACK (set_arena_test));
__libc_lock_unlock (main_arena.mutex);
#else
const char *s = NULL;
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 1/6] tunables: Clean up hooks to get and set tunables Siddhesh Poyarekar
@ 2017-06-01 20:12 ` Siddhesh Poyarekar
2017-06-06 17:57 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask Siddhesh Poyarekar
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-01 20:12 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
Add LD_HWCAP_MASK to tunables in preparation of it being removed from
rtld.c. This allows us to read LD_HWCAP_MASK much earlier so that it
can influence IFUNC resolution in aarch64.
This patch does not actually do anything other than read the
LD_HWCAP_MASK variable and add the tunables way to set the
LD_HWCAP_MASK, i.e. via the glibc.tune.hwcap_mask tunable. In a
follow-up patch, the _dl_hwcap_mask will be replaced with
glibc.tune.hwcap_mask to complete the transition.
* elf/dl-tunables.list: Add glibc.tune.hwcap_mask.
* scripts/gen-tunables.awk: Include dl-procinfo.h.
* manual/tunables.texi: Document glibc.tune.hwcap_mask.
---
elf/dl-tunables.list | 7 +++++++
manual/tunables.texi | 23 +++++++++++++++++++++++
scripts/gen-tunables.awk | 1 +
3 files changed, 31 insertions(+)
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index b9f1488..41ce9af 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -77,4 +77,11 @@ glibc {
security_level: SXID_IGNORE
}
}
+ tune {
+ hwcap_mask {
+ type: UINT_64
+ env_alias: LD_HWCAP_MASK
+ default: HWCAP_IMPORTANT
+ }
+ }
}
diff --git a/manual/tunables.texi b/manual/tunables.texi
index ac8c38f..c9a4cb7 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -31,6 +31,8 @@ their own namespace.
@menu
* Tunable names:: The structure of a tunable name
* Memory Allocation Tunables:: Tunables in the memory allocation subsystem
+* Hardware Capability Tunables:: Tunables that modify the hardware
+ capabilities seen by @theglibc{}
@end menu
@node Tunable names
@@ -190,3 +192,24 @@ number of arenas is determined by the number of CPU cores online. For 32-bit
systems the limit is twice the number of cores online and on 64-bit systems, it
is 8 times the number of cores online.
@end deftp
+
+@node Hardware Capability Tunables
+@section Hardware Capability Tunables
+@cindex hardware capability tunables
+@cindex hwcap tunables
+@cindex tunables, hwcap
+
+@deftp {Tunable namespace} glibc.tune
+Behavior of @theglibc{} can be tuned to assume specific hardware capabilities
+by setting the following tunables in the @code{tune} namespace:
+@end deftp
+
+@deftp Tunable glibc.tune.hwcap_mask
+This tunable supersedes the @env{LD_HWCAP_MASK} environment variable and is
+identical in features.
+
+The @code{AT_HWCAP} key in the Auxilliary Vector specifies instruction set
+extensions available in the processor at runtime for some architectures. The
+@code{glibc.tune.hwcap_mask} tunable allows the user to mask out those
+capabilities at runtime, thus disabling use of those extensions.
+@end deftp
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index b10b00e..93e5aff 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -134,6 +134,7 @@ END {
print "# error \"Do not include this file directly.\""
print "# error \"Include tunables.h instead.\""
print "#endif"
+ print "#include <dl-procinfo.h>\n"
# Now, the enum names
print "\ntypedef enum"
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Ping] [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
` (5 preceding siblings ...)
2017-06-01 20:12 ` [PATCH 6/6] aarch64: Add hwcap string routines Siddhesh Poyarekar
@ 2017-06-05 16:28 ` Siddhesh Poyarekar
6 siblings, 0 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-05 16:28 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, sellcey
Ping!
On Friday 02 June 2017 01:42 AM, Siddhesh Poyarekar wrote:
> Hi,
>
> Here is another iteration of the patch to move LD_HWCAP_MASK into tunables
> and then using it to disable HWCAP_CPUID on aarch64. There is just one
> change from the previous version:
>
> The first patch now reworks the internal tunables API to make it more
> straightforward with just TUNABLE_GET and TUNABLE_SET instead of the
> arbitrary TUNABLE_SET, TUNABLE_GET, TUNABLE_UPDATE, etc. The API
> should now be more straightforward and predictable to use. This also
> fixes the problem of hwcap_mask not being read when it is not set
> externally.
>
> Testing:
>
> - The patches don't introduce any testsuite regressions on x86_64 and
> aarch64
> - I ran the dynamic linker under gdb and verified that the hwcap_mask is
> read correctly and that MIDR is read correctly
> - I ran 'LD_SHOW_AUXV=1 elf/ld.so --library-path .:elf:nptl /bin/true'
> to verify that the hwcaps are read correctly.
>
> Siddhesh Poyarekar (6):
> tunables: Clean up hooks to get and set tunables
> tunables: Add LD_HWCAP_MASK to tunables
> tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask
> aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
> Make LD_HWCAP_MASK usable for static binaries
> aarch64: Add hwcap string routines
>
> README.tunables | 69 +++++++++++++++++----
> elf/Versions | 2 +-
> elf/dl-cache.c | 5 +-
> elf/dl-hwcaps.c | 11 +++-
> elf/dl-hwcaps.h | 30 +++++++++
> elf/dl-support.c | 2 +
> elf/dl-tunables.c | 58 ++++++++++-------
> elf/dl-tunables.h | 53 ++++++++++------
> elf/dl-tunables.list | 7 +++
> elf/rtld.c | 4 ++
> malloc/arena.c | 20 +++---
> manual/tunables.texi | 23 +++++++
> scripts/gen-tunables.awk | 1 +
> sysdeps/generic/ldsodefs.h | 2 +
> sysdeps/sparc/sparc32/dl-machine.h | 6 +-
> sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 10 +--
> sysdeps/unix/sysv/linux/aarch64/dl-procinfo.c | 15 +++++
> sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 86 ++++++++++++++++++++++++++
> sysdeps/x86/cpu-features.c | 10 +--
> 19 files changed, 339 insertions(+), 75 deletions(-)
> create mode 100644 elf/dl-hwcaps.h
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] tunables: Clean up hooks to get and set tunables
2017-06-01 20:12 ` [PATCH 1/6] tunables: Clean up hooks to get and set tunables Siddhesh Poyarekar
@ 2017-06-06 17:37 ` Adhemerval Zanella
2017-06-06 17:52 ` Siddhesh Poyarekar
0 siblings, 1 reply; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 17:37 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> The TUNABLE_SET_VALUE and family of macros (and my later attempt to
> add a TUNABLE_GET) never quite went together very well because the
> overall interface was not clearly defined. This patch is an attempt
> to do just that.
>
> This patch consolidates the API to two simple sets of macros,
> TUNABLE_GET* and TUNABLE_SET*. If TUNABLE_NAMESPACE is defined,
> TUNABLE_GET takes just the tunable name, type and a (optionally NULL)
> callback function to get the value of the tunable. The callback
> function, if non-NULL, is called if the tunable was externally set
> (i.e. via GLIBC_TUNABLES or any future mechanism). For example:
>
> val = TUNABLE_GET (check, int32_t, check_callback)
>
> returns the value of the glibc.malloc.check tunable (assuming
> TUNABLE_NAMESPACE is set to malloc) as an int32_t into VAL after
> calling check_callback.
>
> Likewise, TUNABLE_SET can be used to set the value of the tunable,
> although this is currently possible only in the dynamic linker before
> it relocates itself. For example:
>
> TUNABLE_SET (check, int32_t, 2)
>
> will set glibc.malloc.check to 2. Of course, this is not possible
> since we set (or read) glibc.malloc.check long after it is relocated.
>
> To access or set a tunable outside of TUNABLE_NAMESPACE, use the
> TUNABLE_GET_FULL and TUNABLE_SET_FULL macros, which have the following
> prototype:
>
> TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
> TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, 0xffff)
>
> In future the tunable list may get split into mutable and immutable
> tunables where mutable tunables can be modified by the library and
> userspace after relocation as well and TUNABLE_SET will be more useful
> than it currently is. However whenever we actually do that split, we
> will have to ensure that the mutable tunables are protected with
> locks.
>
> * elf/Versions (__tunable_set_val): Rename to __tunable_get_val.
> * elf/dl-tunables.c: Likewise.
> (do_tunable_update_val): New function.
> (__tunable_set_val): New function.
> (__tunable_get_val): Call CB only if the tunable was externally
> initialized.
> (tunables_strtoul): Replace strval with initialized.
> * elf/dl-tunables.h (strval): Replace with a bool initialized.
> (TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
> prevent collision.
> (__tunable_set_val): New function.
> (TUNABLE_GET, TUNABLE_GET_FULL): New macros.
> (TUNABLE_SET, TUNABLE_SET_FULL): Likewise.
> (TUNABLE_SET_VAL): Remove.
> (TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
> * README.tunables: Document the new macros.
> * malloc/arena.c (ptmalloc_init): Adjust.
LGTM with just a clarification below.
> diff --git a/README.tunables b/README.tunables
> index 0e9b0d7..6fcfd2d 100644
> --- a/README.tunables
> +++ b/README.tunables
[...]
> +GETTING AND SETTING TUNABLES
> +----------------------------
> +
> +When the TUNABLE_NAMESPACE macro is defined, one may get tunables in that
> +module using the TUNABLE_GET macro as follows:
> +
> + val = TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (check_callback))
> +
> +where 'check' is the tunable name, 'int32_t' is the C type of the tunable and
> +'check_callback' is the function to call if the tunable got initialized to a
> +non-default value. The macro returns the value as type 'int32_t'.
> +
> +The callback function should be defined as follows:
> +
> + void
> + DL_TUNABLE_CALLBACK (check_callback) (int32_t *valp)
> + {
> + ...
> + }
Shouldn't it be just TUNABLE_CALLBACK? I noted that DL_TUNABLE_CALLBACK is just
define at malloc/arena.c.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] tunables: Clean up hooks to get and set tunables
2017-06-06 17:37 ` Adhemerval Zanella
@ 2017-06-06 17:52 ` Siddhesh Poyarekar
0 siblings, 0 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-06 17:52 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha; +Cc: sellcey
On Tuesday 06 June 2017 11:07 PM, Adhemerval Zanella wrote:
> Shouldn't it be just TUNABLE_CALLBACK? I noted that DL_TUNABLE_CALLBACK is just
> define at malloc/arena.c.
Yessir, I forgot that one during the refactoring, I'll fix it before
commit. I'll rename the DL_TUNABLE_CALLBACK_FNDECL in malloc/arena.c as
well to maintain consistency.
Siddhesh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables
2017-06-01 20:12 ` [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables Siddhesh Poyarekar
@ 2017-06-06 17:57 ` Adhemerval Zanella
2017-06-06 18:16 ` Adhemerval Zanella
2017-06-07 5:36 ` Siddhesh Poyarekar
0 siblings, 2 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 17:57 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> Add LD_HWCAP_MASK to tunables in preparation of it being removed from
> rtld.c. This allows us to read LD_HWCAP_MASK much earlier so that it
> can influence IFUNC resolution in aarch64.
>
> This patch does not actually do anything other than read the
> LD_HWCAP_MASK variable and add the tunables way to set the
> LD_HWCAP_MASK, i.e. via the glibc.tune.hwcap_mask tunable. In a
> follow-up patch, the _dl_hwcap_mask will be replaced with
> glibc.tune.hwcap_mask to complete the transition.
>
> * elf/dl-tunables.list: Add glibc.tune.hwcap_mask.
> * scripts/gen-tunables.awk: Include dl-procinfo.h.
> * manual/tunables.texi: Document glibc.tune.hwcap_mask.
LGTM with a remark below.
> ---
> elf/dl-tunables.list | 7 +++++++
> manual/tunables.texi | 23 +++++++++++++++++++++++
> scripts/gen-tunables.awk | 1 +
> 3 files changed, 31 insertions(+)
>
> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> index b9f1488..41ce9af 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -77,4 +77,11 @@ glibc {
> security_level: SXID_IGNORE
> }
> }
> + tune {
> + hwcap_mask {
> + type: UINT_64
> + env_alias: LD_HWCAP_MASK
> + default: HWCAP_IMPORTANT
> + }
> + }
LD_HWCAP_MASK is on UNSECURE_ENVVARS at sysdeps/generic/unsecvars.h, so I think
we should add a security_level: SXID_IGNORE for hwcap_mask as well.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables
2017-06-06 17:57 ` Adhemerval Zanella
@ 2017-06-06 18:16 ` Adhemerval Zanella
2017-06-07 5:36 ` Siddhesh Poyarekar
1 sibling, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 18:16 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 06/06/2017 14:57, Adhemerval Zanella wrote:
>
>
> On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
>> Add LD_HWCAP_MASK to tunables in preparation of it being removed from
>> rtld.c. This allows us to read LD_HWCAP_MASK much earlier so that it
>> can influence IFUNC resolution in aarch64.
>>
>> This patch does not actually do anything other than read the
>> LD_HWCAP_MASK variable and add the tunables way to set the
>> LD_HWCAP_MASK, i.e. via the glibc.tune.hwcap_mask tunable. In a
>> follow-up patch, the _dl_hwcap_mask will be replaced with
>> glibc.tune.hwcap_mask to complete the transition.
>>
>> * elf/dl-tunables.list: Add glibc.tune.hwcap_mask.
>> * scripts/gen-tunables.awk: Include dl-procinfo.h.
>> * manual/tunables.texi: Document glibc.tune.hwcap_mask.
>
> LGTM with a remark below.
>
>> ---
>> elf/dl-tunables.list | 7 +++++++
>> manual/tunables.texi | 23 +++++++++++++++++++++++
>> scripts/gen-tunables.awk | 1 +
>> 3 files changed, 31 insertions(+)
>>
>> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
>> index b9f1488..41ce9af 100644
>> --- a/elf/dl-tunables.list
>> +++ b/elf/dl-tunables.list
>> @@ -77,4 +77,11 @@ glibc {
>> security_level: SXID_IGNORE
>> }
>> }
>> + tune {
>> + hwcap_mask {
>> + type: UINT_64
>> + env_alias: LD_HWCAP_MASK
>> + default: HWCAP_IMPORTANT
>> + }
>> + }
>
> LD_HWCAP_MASK is on UNSECURE_ENVVARS at sysdeps/generic/unsecvars.h, so I think
> we should add a security_level: SXID_IGNORE for hwcap_mask as well.
>
I am getting build errors for i686 and s390{x} with --enable-tunables:
In file included from /home/azanella/Projects/glibc/build/i686-linux-gnu/dl-tunable-list.h:6:0,
from ./dl-tunables.h:67,
from ../elf/dl-sysdep.c:47,
from ../sysdeps/unix/sysv/linux/dl-sysdep.c:38:
../sysdeps/unix/sysv/linux/i386/dl-procinfo.h:25:1: error: redefinition of â_dl_procinfoâ
_dl_procinfo (unsigned int type, unsigned long int word)
^~~~~~~~~~~~
In file included from ../elf/dl-sysdep.c:42:0,
from ../sysdeps/unix/sysv/linux/dl-sysdep.c:38:
../sysdeps/unix/sysv/linux/i386/dl-procinfo.h:25:1: note: previous definition of â_dl_procinfoâ was here
_dl_procinfo (unsigned int type, unsigned long int word)
^~~~~~~~~~~~
It because both Linux i686 and s390 dl-procinfo.h do not have include guard (as
for the other dl-procinfo.h implementation). I think you should include them
on this patch.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask
2017-06-01 20:12 ` [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask Siddhesh Poyarekar
@ 2017-06-06 18:18 ` Adhemerval Zanella
0 siblings, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 18:18 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> Drop _dl_hwcap_mask when building with tunables. This completes the
> transition of hwcap_mask reading from _dl_hwcap_mask to tunables.
>
> * elf/dl-hwcaps.h: New file.
> * elf/dl-hwcaps.c: Include it.
> (_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update
> glibc.tune.hwcap_mask.
> * elf/dl-cache.c: Include dl-hwcaps.h.
> (_dl_load_cache_lookup)[HAVE_TUNABLES]: Read
> glibc.tune.hwcap_mask.
> * sysdeps/sparc/sparc32/dl-machine.h: Likewise.
> * elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop
> _dl_hwcap_mask.
> * elf/rtld.c (rtld_global_ro)[HAVE_TUNABLES]: Drop
> _dl_hwcap_mask.
> (process_envvars)[HAVE_TUNABLES]: Likewise.
> * sysdeps/generic/ldsodefs.h (rtld_global_ro)[HAVE_TUNABLES]:
> Likewise.
> * sysdeps/x86/cpu-features.c (init_cpu_features): Don't
> initialize dl_hwcap_mask when tunables are enabled.
LGTM.
> diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c
> @@ -125,7 +127,12 @@ _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
> LD_HWCAP_MASK environment variable (or default HWCAP_IMPORTANT).
> So there is no way to request ignoring an OS-supplied dsocap
> string and bit like you can ignore an OS-supplied HWCAP bit. */
> - GLRO(dl_hwcap_mask) |= (uint64_t) mask << _DL_FIRST_EXTRA;
> + hwcap_mask |= (uint64_t) mask << _DL_FIRST_EXTRA;
> +#if HAVE_TUNABLES
> + TUNABLE_SET (glibc, tune, hwcap_mask, uint64_t, hwcap_mask);
> +#else
> + GLRO(dl_hwcap_mask) = hwcap_mask;
> +#endif
I would add macro for both get/set, as for GET_HWCAP_MASK, but we can live with it.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
2017-06-01 20:12 ` [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK Siddhesh Poyarekar
@ 2017-06-06 18:24 ` Adhemerval Zanella
2017-06-07 15:00 ` Szabolcs Nagy
1 sibling, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 18:24 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> Now that LD_HWCAP_MASK (or glibc.tune.hwcap_mask) is read early enough
> to influence cpu feature check in aarch64, use it to influence
> multiarch selection. Setting LD_HWCAP_MASK such that it clears
> HWCAP_CPUID will now disable multiarch for the binary.
>
> HWCAP_CPUID is also now set in HWCAP_IMPORTANT so that it is set by
> default. With this patch, this feature is only usable with
> dyanmically linked binaries because LD_HWCAP_MASK is not read for
> static binaries. A future patch fixes that.
>
> * sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> (init_cpu_features): Use glibc.tune.hwcap_mask.
> * sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h: New file.
LGTM.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries
2017-06-01 20:12 ` [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries Siddhesh Poyarekar
@ 2017-06-06 19:06 ` Adhemerval Zanella
0 siblings, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 19:06 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> The LD_HWCAP_MASK environment variable was ignored in static binaries,
> which is inconsistent with the behaviour of dynamically linked
> binaries. This seems to have been because of the inability of
> ld_hwcap_mask being read early enough to influence anything but now
> that it is in tunables, the mask is usable in static binaries as well.
>
> This feature is important for aarch64, which relies on HWCAP_CPUID
> being masked out to disable multiarch. A sanity test on x86_64 shows
> that there are no failures. Likewise for aarch64.
>
> * elf/dl-hwcaps.h [HAVE_TUNABLES]: Always read hwcap_mask.
> * sysdeps/sparc/sparc32/dl-machine.h [HAVE_TUNABLES]:
> Likewise.
> * sysdeps/x86/cpu-features.c (init_cpu_features): Always set
> up hwcap and hwcap_mask.
LGTM with a remark below.
> ---
> elf/dl-hwcaps.h | 15 +++++++--------
> sysdeps/sparc/sparc32/dl-machine.h | 2 +-
> sysdeps/x86/cpu-features.c | 8 +++-----
> 3 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/elf/dl-hwcaps.h b/elf/dl-hwcaps.h
> index 9ce3317..2c4fa3d 100644
> --- a/elf/dl-hwcaps.h
> +++ b/elf/dl-hwcaps.h
> @@ -18,14 +18,13 @@
>
> #include <elf/dl-tunables.h>
>
> -#ifdef SHARED
> -# if HAVE_TUNABLES
> -# define GET_HWCAP_MASK() \
> - TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
> +#if HAVE_TUNABLES
> +# define GET_HWCAP_MASK() TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
> +#else
> +# ifdef SHARED
> +# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
> # else
> -# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
> +/* HWCAP_MASK is ignored in static binaries when built without tunables. */
> +# define GET_HWCAP_MASK() (0)
> # endif
> -#else
> -/* HWCAP_MASK is ignored in static binaries. */
> -# define GET_HWCAP_MASK() (0)
> #endif
> diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
> index f9ae133..95f6732 100644
> --- a/sysdeps/sparc/sparc32/dl-machine.h
> +++ b/sysdeps/sparc/sparc32/dl-machine.h
> @@ -37,7 +37,7 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
> return 1;
> else if (ehdr->e_machine == EM_SPARC32PLUS)
> {
> -#ifdef SHARED
> +#if HAVE_TUNABLES || defined SHARED
> uint64_t hwcap_mask = GET_HWCAP_MASK();
> return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9;
Since GET_HWCAP_MASK is define 0 for !SHARED, I think it we remove the
preprocessor altogether and just add a comment that for static binaries
with tunable enables it hwcap_mask will be a no-op.
> #else
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 4fe58bf..4288001 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -312,17 +312,16 @@ no_cpuid:
> cpu_features->model = model;
> cpu_features->kind = kind;
>
> -#if IS_IN (rtld)
> /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
> GLRO(dl_platform) = NULL;
> GLRO(dl_hwcap) = 0;
> -#if !HAVE_TUNABLES
> +#if !HAVE_TUNABLES && defined SHARED
> /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
> this. */
> GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
> #endif
Just a though: with a SET_HWCAP_MASK macro we can get rid of preprocessor
macros (since we can define it to a no-op for static binaries without
tunables support).
>
> -# ifdef __x86_64__
> +#ifdef __x86_64__
> if (cpu_features->kind == arch_kind_intel)
> {
> if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
> @@ -352,7 +351,7 @@ no_cpuid:
> && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
> GLRO(dl_platform) = "haswell";
> }
> -# else
> +#else
> if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
> GLRO(dl_hwcap) |= HWCAP_X86_SSE2;
>
> @@ -360,6 +359,5 @@ no_cpuid:
> GLRO(dl_platform) = "i686";
> else if (CPU_FEATURES_ARCH_P (cpu_features, I586))
> GLRO(dl_platform) = "i586";
> -# endif
> #endif
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-01 20:12 ` [PATCH 6/6] aarch64: Add hwcap string routines Siddhesh Poyarekar
@ 2017-06-06 19:32 ` Adhemerval Zanella
2017-06-07 15:18 ` Szabolcs Nagy
1 sibling, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-06 19:32 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: sellcey
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 */
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables
2017-06-06 17:57 ` Adhemerval Zanella
2017-06-06 18:16 ` Adhemerval Zanella
@ 2017-06-07 5:36 ` Siddhesh Poyarekar
2017-06-07 12:07 ` Adhemerval Zanella
1 sibling, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-07 5:36 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha; +Cc: sellcey
On Tuesday 06 June 2017 11:27 PM, Adhemerval Zanella wrote:
> LD_HWCAP_MASK is on UNSECURE_ENVVARS at sysdeps/generic/unsecvars.h, so I think
> we should add a security_level: SXID_IGNORE for hwcap_mask as well.
It is in UNSECURE_ENVVARS, so it should be SXID_ERASE (the default) and
not SXID_IGNORE since the latter will make the tunable available for
children of setuid processes.
Siddhesh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables
2017-06-07 5:36 ` Siddhesh Poyarekar
@ 2017-06-07 12:07 ` Adhemerval Zanella
0 siblings, 0 replies; 25+ messages in thread
From: Adhemerval Zanella @ 2017-06-07 12:07 UTC (permalink / raw)
To: siddhesh, libc-alpha; +Cc: sellcey
On 07/06/2017 02:33, Siddhesh Poyarekar wrote:
> On Tuesday 06 June 2017 11:27 PM, Adhemerval Zanella wrote:
>> LD_HWCAP_MASK is on UNSECURE_ENVVARS at sysdeps/generic/unsecvars.h, so I think
>> we should add a security_level: SXID_IGNORE for hwcap_mask as well.
>
> It is in UNSECURE_ENVVARS, so it should be SXID_ERASE (the default) and
> not SXID_IGNORE since the latter will make the tunable available for
> children of setuid processes.
Right, I wasn't aware SXID_ERASE was the default. Thanks for the clarification.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
2017-06-01 20:12 ` [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK Siddhesh Poyarekar
2017-06-06 18:24 ` Adhemerval Zanella
@ 2017-06-07 15:00 ` Szabolcs Nagy
2017-06-07 16:36 ` Siddhesh Poyarekar
1 sibling, 1 reply; 25+ messages in thread
From: Szabolcs Nagy @ 2017-06-07 15:00 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
On 01/06/17 21:12, Siddhesh Poyarekar wrote:
> Now that LD_HWCAP_MASK (or glibc.tune.hwcap_mask) is read early enough
> to influence cpu feature check in aarch64, use it to influence
> multiarch selection. Setting LD_HWCAP_MASK such that it clears
> HWCAP_CPUID will now disable multiarch for the binary.
>
> HWCAP_CPUID is also now set in HWCAP_IMPORTANT so that it is set by
> default. With this patch, this feature is only usable with
> dyanmically linked binaries because LD_HWCAP_MASK is not read for
> static binaries. A future patch fixes that.
>
> * sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> (init_cpu_features): Use glibc.tune.hwcap_mask.
> * sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h: New file.
> ---
> sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 10 +++---
> sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 44 ++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 4 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
>
OK if the new HWCAP_IMPORTANT does not cause
additional open/stat/.. syscalls (please verify
that the lib search path is not increased by
running something like
strace -E LD_PRELOAD=foobar.so /bin/true
and counting the failed syscalls)
(if the search path is increased then i'd like
to know what is the purpose of that feature)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-01 20:12 ` [PATCH 6/6] aarch64: Add hwcap string routines Siddhesh Poyarekar
2017-06-06 19:32 ` Adhemerval Zanella
@ 2017-06-07 15:18 ` Szabolcs Nagy
2017-06-07 15:27 ` Andreas Schwab
2017-06-07 16:38 ` Siddhesh Poyarekar
1 sibling, 2 replies; 25+ messages in thread
From: Szabolcs Nagy @ 2017-06-07 15:18 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
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.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-07 15:18 ` Szabolcs Nagy
@ 2017-06-07 15:27 ` Andreas Schwab
2017-06-07 16:38 ` Siddhesh Poyarekar
1 sibling, 0 replies; 25+ messages in thread
From: Andreas Schwab @ 2017-06-07 15:27 UTC (permalink / raw)
To: Szabolcs Nagy
Cc: Siddhesh Poyarekar, libc-alpha, nd, adhemerval.zanella, sellcey
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK
2017-06-07 15:00 ` Szabolcs Nagy
@ 2017-06-07 16:36 ` Siddhesh Poyarekar
0 siblings, 0 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-07 16:36 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
On Wednesday 07 June 2017 08:30 PM, Szabolcs Nagy wrote:
> OK if the new HWCAP_IMPORTANT does not cause
> additional open/stat/.. syscalls (please verify
> that the lib search path is not increased by
> running something like
> strace -E LD_PRELOAD=foobar.so /bin/true
> and counting the failed syscalls)
>
> (if the search path is increased then i'd like
> to know what is the purpose of that feature)
I thought I already explained it in a previous email - this adds an
additional search path for the dynamic linker to look for libraries that
may be optimized for that specific hwcap bit. A straightforward way to
disable this is to set LD_HWCAP_MASK to 0x0 to clear hwcap bits.
On server systems where this would be used, this would have been done by
ldconfig and hence the lookup will not involve a full search every time,
just for situations where the library is not mapped into ld.so.cache.
Siddhesh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-07 15:18 ` Szabolcs Nagy
2017-06-07 15:27 ` Andreas Schwab
@ 2017-06-07 16:38 ` Siddhesh Poyarekar
2017-06-09 8:24 ` Szabolcs Nagy
1 sibling, 1 reply; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-07 16:38 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-07 16:38 ` Siddhesh Poyarekar
@ 2017-06-09 8:24 ` Szabolcs Nagy
2017-06-09 8:56 ` Siddhesh Poyarekar
0 siblings, 1 reply; 25+ messages in thread
From: Szabolcs Nagy @ 2017-06-09 8:24 UTC (permalink / raw)
To: siddhesh, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
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.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] aarch64: Add hwcap string routines
2017-06-09 8:24 ` Szabolcs Nagy
@ 2017-06-09 8:56 ` Siddhesh Poyarekar
0 siblings, 0 replies; 25+ messages in thread
From: Siddhesh Poyarekar @ 2017-06-09 8:56 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: nd, adhemerval.zanella, sellcey
[-- Attachment #1: Type: text/plain, Size: 343 bytes --]
On Friday 09 June 2017 01:54 PM, Szabolcs Nagy wrote:
> please fix the undefined behaviour you introduced.
>
> and remove the comments with magic numbers so
> on hwcap update comments do not need changes.
Done, I've pushed the attached patch as per your and Andreas' comments.
Sorry I missed your comment about undefined behavior.
Siddhesh
[-- Attachment #2: 0001-aarch64-Fix-undefined-behavior-in-_dl_procinfo.patch --]
[-- Type: text/x-patch, Size: 1938 bytes --]
From 6c85cc2852367ea2db91ff6a1fc0f6fc0653788d Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date: Fri, 9 Jun 2017 14:18:11 +0530
Subject: [PATCH] aarch64: Fix undefined behavior in _dl_procinfo
1 << 31 is undefined, so replace it with a cleaner check. Also remove
magic numbers in comments.
* sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h: Remove
mention of magic numbers in comments.
(_dl_procinfo): Fix undefined behavior
---
ChangeLog | 6 ++++++
sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h | 6 +++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6c6ff13..006e7ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-09 Siddhesh Poyarekar <siddhesh@sourceware.org>
+
+ * sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h: Remove
+ mention of magic numbers in comments.
+ (_dl_procinfo): Fix undefined behavior
+
2017-06-08 Joseph Myers <joseph@codesourcery.com>
* conform/data/sys/wait.h-data (WIFCONTINUED): Do not expect for
diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
index cdb36d3..04fc6be 100644
--- a/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
+++ b/sysdeps/unix/sysv/linux/aarch64/dl-procinfo.h
@@ -40,7 +40,7 @@ _dl_procinfo (unsigned int type, unsigned long int word)
_dl_printf ("AT_HWCAP: ");
for (i = 0; i < 32; ++i)
- if (word & (1 << i))
+ if ((word >> i) & 1)
_dl_printf (" %s", GLRO(dl_aarch64_cap_flags)[i]);
_dl_printf ("\n");
@@ -56,10 +56,10 @@ _dl_hwcap_string (int idx)
};
-/* 13 HWCAP bits set. */
+/* Number of HWCAP bits set. */
#define _DL_HWCAP_COUNT 13
-/* Low 13 bits are allocated in HWCAP. */
+/* Offset of the last bit allocated in HWCAP. */
#define _DL_HWCAP_LAST 12
/* HWCAP_CPUID should be available by default to influence IFUNC as well as
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-06-09 8:56 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-01 20:12 [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries Siddhesh Poyarekar
2017-06-06 19:06 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 1/6] tunables: Clean up hooks to get and set tunables Siddhesh Poyarekar
2017-06-06 17:37 ` Adhemerval Zanella
2017-06-06 17:52 ` Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 2/6] tunables: Add LD_HWCAP_MASK to tunables Siddhesh Poyarekar
2017-06-06 17:57 ` Adhemerval Zanella
2017-06-06 18:16 ` Adhemerval Zanella
2017-06-07 5:36 ` Siddhesh Poyarekar
2017-06-07 12:07 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 3/6] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask Siddhesh Poyarekar
2017-06-06 18:18 ` Adhemerval Zanella
2017-06-01 20:12 ` [PATCH 4/6] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK Siddhesh Poyarekar
2017-06-06 18:24 ` Adhemerval Zanella
2017-06-07 15:00 ` Szabolcs Nagy
2017-06-07 16:36 ` Siddhesh Poyarekar
2017-06-01 20:12 ` [PATCH 6/6] aarch64: Add hwcap string routines Siddhesh Poyarekar
2017-06-06 19:32 ` Adhemerval Zanella
2017-06-07 15:18 ` Szabolcs Nagy
2017-06-07 15:27 ` Andreas Schwab
2017-06-07 16:38 ` Siddhesh Poyarekar
2017-06-09 8:24 ` Szabolcs Nagy
2017-06-09 8:56 ` Siddhesh Poyarekar
2017-06-05 16:28 ` [Ping] [PATCH v3 0/6] aarch64: Allow overriding HWCAP_CPUID feature check Siddhesh Poyarekar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).