[5/5] aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK

Message ID 1495138038-32212-6-git-send-email-siddhesh@sourceware.org
State New, archived
Headers

Commit Message

Siddhesh Poyarekar May 18, 2017, 8:07 p.m. UTC
  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.

	* sysdeps/unix/sysv/linux/aarch64/cpu-features.c
	(init_cpu_features): Use glibc.tune.hwcap_mask.

---
 sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
  

Comments

Szabolcs Nagy May 19, 2017, 9:48 a.m. UTC | #1
On 18/05/17 21:07, 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.
> 
> 	* sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> 	(init_cpu_features): Use glibc.tune.hwcap_mask.
> 

this cannot go in until somebody figures out
why LD_HWCAP_MASK affects the dynamic linker
search path, which is undesirable when masking
the cpuid bit.
  
Siddhesh Poyarekar May 19, 2017, 5:44 p.m. UTC | #2
On Friday 19 May 2017 03:18 PM, Szabolcs Nagy wrote:
> this cannot go in until somebody figures out
> why LD_HWCAP_MASK affects the dynamic linker
> search path, which is undesirable when masking
> the cpuid bit.

I haven't been able to reproduce the problem on aarch64 and my current
guess is that it is specific to x86 and should have been resolved with
HJ's patch for #21391.  I'll start a separate conversation for this with
my observations so far but unless you're able to reproduce the failure
on aarch64, I don't see a reason for it to block this patchset.

Siddhesh
  
Szabolcs Nagy May 19, 2017, 10:12 p.m. UTC | #3
* Siddhesh Poyarekar <siddhesh@sourceware.org> [2017-05-19 23:14:20 +0530]:
> On Friday 19 May 2017 03:18 PM, Szabolcs Nagy wrote:
> > this cannot go in until somebody figures out
> > why LD_HWCAP_MASK affects the dynamic linker
> > search path, which is undesirable when masking
> > the cpuid bit.
> 
> I haven't been able to reproduce the problem on aarch64 and my current
> guess is that it is specific to x86 and should have been resolved with
> HJ's patch for #21391.  I'll start a separate conversation for this with
> my observations so far but unless you're able to reproduce the failure
> on aarch64, I don't see a reason for it to block this patchset.

no oom on aarch64 does not mean there is no problem

LD_HWCAP_MASK changes the behaviour of _dl_init_path
in elf/ld-load.c which we should understand before
the patch is committed (because it implies the
env var historically had different semantics than
what we assumed).
  
Siddhesh Poyarekar May 20, 2017, 3:32 a.m. UTC | #4
On Saturday 20 May 2017 03:42 AM, Szabolcs Nagy wrote:
> no oom on aarch64 does not mean there is no problem
> 
> LD_HWCAP_MASK changes the behaviour of _dl_init_path
> in elf/ld-load.c which we should understand before
> the patch is committed (because it implies the
> env var historically had different semantics than
> what we assumed).

Ahh, sorry, I just realized that we are talking about different things.
I thought you were objecting to including the patch until the OOM was fixed.

One may have additional search paths for every hwcap that is set by the
kernel, so if hwcap_cpuid is set, the linker (and ldconfig) ought to
search for /usr/lib/hwcap_cpuid for libraries to load as well.

This has an impact only if the target system is configured with
libraries in that hwcap path, e.g. if someone decides to have additional
libraries in /usr/lib/<hwcap feature> and that path gets searched only
if that hwcap feature is enabled.  This allows users to ship, say,
separate set of libraries for avx512 on x86_64 in /usr/lib/avx512_1
which gets searched before /usr/lib.

So masking out specific bits will disable searching of libraries in
those paths as well.  I hope that answers your question.

Siddhesh
  
Szabolcs Nagy May 20, 2017, 12:35 p.m. UTC | #5
* Siddhesh Poyarekar <siddhesh@sourceware.org> [2017-05-20 09:02:20 +0530]:
> On Saturday 20 May 2017 03:42 AM, Szabolcs Nagy wrote:
> > no oom on aarch64 does not mean there is no problem
> > 
> > LD_HWCAP_MASK changes the behaviour of _dl_init_path
> > in elf/ld-load.c which we should understand before
> > the patch is committed (because it implies the
> > env var historically had different semantics than
> > what we assumed).
> 
> Ahh, sorry, I just realized that we are talking about different things.
> I thought you were objecting to including the patch until the OOM was fixed.
> 
> One may have additional search paths for every hwcap that is set by the
> kernel, so if hwcap_cpuid is set, the linker (and ldconfig) ought to
> search for /usr/lib/hwcap_cpuid for libraries to load as well.
> 
> This has an impact only if the target system is configured with
> libraries in that hwcap path, e.g. if someone decides to have additional
> libraries in /usr/lib/<hwcap feature> and that path gets searched only
> if that hwcap feature is enabled.  This allows users to ship, say,
> separate set of libraries for avx512 on x86_64 in /usr/lib/avx512_1
> which gets searched before /usr/lib.
> 
> So masking out specific bits will disable searching of libraries in
> those paths as well.  I hope that answers your question.

ok now i understand it better, my confusion is that
the default seems to be _dl_hwcap_mask == 0, which
would disable the cpuid bit unless one explicitly
set LD_HWCAP_MASK=2048, i thought we wanted the
opposite behaviour (cpuid based dispatch by default,
which can be disabled with explicit setting).
  
Siddhesh Poyarekar May 20, 2017, 1:23 p.m. UTC | #6
On Saturday 20 May 2017 06:05 PM, Szabolcs Nagy wrote:
> ok now i understand it better, my confusion is that
> the default seems to be _dl_hwcap_mask == 0, which
> would disable the cpuid bit unless one explicitly
> set LD_HWCAP_MASK=2048, i thought we wanted the
> opposite behaviour (cpuid based dispatch by default,
> which can be disabled with explicit setting).

Ah nice catch, we need to add HWCAP_CPUID to the important hwcaps.  I'll
add that change to this last patch and repost.

Siddhesh
  

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 7025062..0478fcc 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -18,18 +18,25 @@ 
 
 #include <cpu-features.h>
 #include <sys/auxv.h>
+#include <elf/dl-tunables.h>
 
 static inline void
 init_cpu_features (struct cpu_features *cpu_features)
 {
-  if (GLRO(dl_hwcap) & HWCAP_CPUID)
+#if HAVE_TUNABLES
+  uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t);
+#else
+  uint64_t hwcap_mask = GLRO (dl_hwcap_mask);
+#endif
+
+  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;
 }