[PATCH v2 0/5] aarch64: Allow overriding HWCAP_CPUID feature check

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 0/5] aarch64: Allow overriding HWCAP_CPUID feature check

Siddhesh Poyarekar-9
Hi,

This is take 2 of the remaining patches in the set that allows overriding
aarch64 ifunc using the LD_HWCAP_MASK.  This set gets the LD_HWCAP_MASK into
tunables so that it can be read early enough to influence tunables behaviour.
In the process, the routines to detect CPU features have been delayed in x86
and aarch64 so that they can be influenced by tunables.

Changes from previous version:

 - Add documentation for the new tunable and for new tunables API
 - Fixed errors that Adhemerval noted

Siddhesh Poyarekar (5):
  tunables: Add hooks to get and update tunables
  tunables: Add LD_HWCAP_MASK to tunables
  tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask
  Delay initialization of CPU features struct in static binaries
  aarch64: Allow overriding HWCAP_CPUID feature check using HWCAP_MASK

 README.tunables                                | 27 ++++++++++++++++
 csu/libc-start.c                               |  6 ++++
 elf/dl-cache.c                                 |  9 +++++-
 elf/dl-hwcaps.c                                | 15 +++++++--
 elf/dl-support.c                               |  2 ++
 elf/dl-tunables.c                              | 44 +++++++++++++++++++++-----
 elf/dl-tunables.h                              | 42 +++++++++++++++++-------
 elf/dl-tunables.list                           |  7 ++++
 elf/rtld.c                                     |  4 +++
 manual/tunables.texi                           | 22 +++++++++++++
 scripts/gen-tunables.awk                       |  1 +
 sysdeps/generic/ldsodefs.h                     |  2 ++
 sysdeps/sparc/sparc32/dl-machine.h             |  8 ++++-
 sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 15 ++++++---
 sysdeps/unix/sysv/linux/aarch64/libc-start.c   | 23 +++-----------
 sysdeps/x86/cpu-features.c                     |  4 +++
 sysdeps/x86/libc-start.c                       | 23 +++-----------
 17 files changed, 190 insertions(+), 64 deletions(-)

--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5] tunables: Add hooks to get and update tunables

Siddhesh Poyarekar-9
This change adds two new tunable macros to get and update tunables
respectively called TUNABLE_GET and TUNABLE_UPDATE_VAL.

TUNABLE_GET accepts the top namespace, tunable namespace and tunable
id along with the target type as arguments and returns a value of the
target type.  For example:

    TUNABLE_GET (glibc, malloc, check, int32_t)

will return the tunable value for glibc.malloc.check in an int32_t.

TUNABLE_UPDATE_VAL accepts the top namespace, tunable namespace and
tunable id along with the value to be stored into the tunable
structure followed by the type of the value.  For example:

  TUNABLE_UPDATE_VAL (glibc, malloc, check, 1, int32_t);

will update the glibc.malloc.check tunable.

Given that the tunable structure is now relro, this is only really
possible from within the dynamic linker before it is relocated.  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.  However whenever we actually do
that, we will have to ensure that the mutable tunables are protected
with locks.

        * elf/dl-tunables.h (strval): Replace with a bool initialized.
        (TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
        prevent collision.
        (__tunable_update_val): New function.
        (TUNABLE_GET): New macro.
        (TUNABLE_SET_VAL_FULL_WITH_CALLBACK): New macro.
        (TUNABLE_SET_VAL): Use it.
        (TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
        (TUNABLE_UPDATE_VAL): New macro.
        (tunables_strtoul): Replace strval with initialized.
        (__tunables_init): Likewise.
        (__tunable_set_val): Likewise.
        (do_tunable_update_val): New function.
        (tunables_initialize): Use it.
        (__tunable_update_val): New function.
        * README.tunables: Document the new macros.
---
 README.tunables   | 27 +++++++++++++++++++++++++++
 elf/dl-tunables.c | 43 +++++++++++++++++++++++++++++++++++--------
 elf/dl-tunables.h | 40 ++++++++++++++++++++++++++++------------
 3 files changed, 90 insertions(+), 20 deletions(-)

diff --git a/README.tunables b/README.tunables
index 0e9b0d7..0bea2ca 100644
--- a/README.tunables
+++ b/README.tunables
@@ -75,6 +75,33 @@ The list of allowed attributes are:
    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.
 
+GETTING A TUNABLE VALUE
+-----------------------
+
+One may get the value of a tunable anywhere in glibc code using the TUNABLE_GET
+macro as follows:
+
+  TUNABLE_GET (glibc, malloc, check, int32_t)
+
+The first three arguments of the macro are the top namespace, tunable namespace
+and the tunable name respectively.  The last argument is the type of the
+tunable and must match the type of the tunable.
+
+UPDATING A TUNABLE VALUE
+------------------------
+
+The tunable list is currently stored in a relro section, i.e. it cannot be
+modified after relocation.  As a result, one may update the value of a tunable
+only in the dynamic linker.  The TUNABLE_UPDATE_VALUE macro can be used to do
+this:
+
+    TUNABLE_UPDATE_VAL (glibc, malloc, check, val, int32_t);
+
+where the first thre arguments of the macro are the top namespace, tunable
+namespace and the tunable name respectively.  The fourth argument is the value
+to assign to the tunable and the last argument is the type of the tunable.  The
+type in the fifth argument must match the type of the tunable.
+
 FUTURE WORK
 -----------
 
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 8d72e26..abf10e5 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_update_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;
@@ -432,7 +459,7 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
 
   /* Don't do anything if our tunable was not set during initialization or if
      it failed validation.  */
-  if (cur->strval == NULL)
+  if (!cur->initialized)
     return;
 
   /* Caller does not need the value, just call the callback with our tunable
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index f33adfb..f465370 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,29 +61,45 @@ 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_update_val (tunable_id_t, void *);
+
+/* Get and return a tunable value.  */
+# define TUNABLE_GET(__top, __ns, __id, __type) \
+({      \
+  tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id);      \
+  __type ret;      \
+  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (id, &ret, NULL);      \
+  ret;      \
+})
 
 /* 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) \
-({      \
-  __tunable_set_val      \
-   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
-    NULL);      \
-})
+  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE,       \
+ TUNABLE_NAMESPACE,   \
+ __id), (__val), NULL)
 
 /* Same as TUNABLE_SET_VAL, but also call the callback function __CB.  */
 # define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
+  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE,       \
+ TUNABLE_NAMESPACE,   \
+ __id),      \
+      (__val), DL_TUNABLE_CALLBACK (__cb))
+
+# define TUNABLE_SET_VAL_FULL_WITH_CALLBACK(__id,__val,__cb)\
+  __tunable_set_val (__id, (__val), (__cb))
+
+# define TUNABLE_UPDATE_VAL(__top, __ns, __id, __val, __type) \
 ({      \
-  __tunable_set_val      \
-   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
-    DL_TUNABLE_CALLBACK (__cb));      \
+  __tunable_update_val (TUNABLE_ENUM_NAME (__top, __ns, __id),      \
+ & (__type) {__val});      \
 })
 
 /* Namespace sanity for callback functions.  Use this macro to keep the
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/5] tunables: Add LD_HWCAP_MASK to tunables

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
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     | 22 ++++++++++++++++++++++
 scripts/gen-tunables.awk |  1 +
 3 files changed, 30 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..37aefca 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,23 @@ 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.
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

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
Drop _dl_hwcap_mask when building with tunables.  This completes the
transition of hwcap_mask reading from _dl_hwcap_mask to tunables.

        * elf/dl-cache.c: Include dl-tunables.h.
        (_dl_load_cache_lookup)[HAVE_TUNABLES]: Read
        glibc.tune.hwcap_mask.
        * elf/dl-hwcaps.c: Include dl-tunables.h.
        (_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update
        glibc.tune.hwcap_mask.
        * sysdeps/sparc/sparc32/dl-machine.h: Likewise.
        * elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop
        _dl_hwcap_mask.
        * elf/dl-tunables.c (__tunable_set_val): Make a hidden alias.
        * elf/dl-tunables.h (__tunable_set_val): Likewise.
        * 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                     |  9 ++++++++-
 elf/dl-hwcaps.c                    | 15 +++++++++++++--
 elf/dl-support.c                   |  2 ++
 elf/dl-tunables.c                  |  1 +
 elf/dl-tunables.h                  |  2 ++
 elf/rtld.c                         |  4 ++++
 sysdeps/generic/ldsodefs.h         |  2 ++
 sysdeps/sparc/sparc32/dl-machine.h |  8 +++++++-
 sysdeps/x86/cpu-features.c         |  4 ++++
 9 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 017c78a..b7ae05c 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-tunables.h>
 
 #ifndef _DL_PLATFORMS_COUNT
 # define _DL_PLATFORMS_COUNT 0
@@ -258,8 +259,14 @@ _dl_load_cache_lookup (const char *name)
       if (platform != (uint64_t) -1)
  platform = 1ULL << platform;
 
+#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
+
 #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..2b9f7f1 100644
--- a/elf/dl-hwcaps.c
+++ b/elf/dl-hwcaps.c
@@ -24,6 +24,7 @@
 #include <ldsodefs.h>
 
 #include <dl-procinfo.h>
+#include <dl-tunables.h>
 
 #ifdef _DL_FIRST_PLATFORM
 # define _DL_FIRST_EXTRA (_DL_FIRST_PLATFORM + _DL_PLATFORMS_COUNT)
@@ -37,8 +38,13 @@ internal_function
 _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
       size_t *max_capstrlen)
 {
+#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
   /* 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 +131,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_UPDATE_VAL (glibc, tune, hwcap_mask, hwcap_mask, uint64_t);
+#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-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/dl-tunables.c b/elf/dl-tunables.c
index abf10e5..be7733f 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -497,3 +497,4 @@ cb:
   if (callback)
     callback (&cur->val);
 }
+rtld_hidden_def (__tunable_set_val)
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index f465370..003e98b 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -70,6 +70,8 @@ extern void __tunables_init (char **);
 extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
 extern void __tunable_update_val (tunable_id_t, void *);
 
+rtld_hidden_proto (__tunable_set_val)
+
 /* Get and return a tunable value.  */
 # define TUNABLE_GET(__top, __ns, __id, __type) \
 ({      \
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 cf7272f..f5e8078 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-tunables.h>
 
 /* Return nonzero iff ELF header is compatible with the running host.  */
 static inline int
@@ -39,7 +40,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
       /* XXX The following is wrong!  Dave Miller rejected to implement it
  correctly.  If this causes problems shoot *him*!  */
 #ifdef SHARED
-      return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9;
+# 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
+      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

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5] Delay initialization of CPU features struct in static binaries

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
Allow the CPU features structure set up to be overridden by tunables
by delaying it to until after tunables are initialized.  The
initialization is already delayed in dynamically linked glibc, it is
only in static binaries that the initialization is set early to allow
it to influence IFUNC relocations that happen in libc-start.  It is a
bit too early however and there is a good place between tunables
initialization and IFUNC relocations where this can be done.

Verified that this does not regress the testsuite.

        * csu/libc-start.c [!ARCH_INIT_CPU_FEATURES]: Define
        ARCH_INIT_CPU_FEATURES.
        (LIBC_START_MAIN): Call it.
        * sysdeps/unix/sysv/linux/aarch64/libc-start.c
        (__libc_start_main): Remove.
        (ARCH_INIT_CPU_FEATURES): New macro.
        * sysdeps/x86/libc-start.c (__libc_start_main): Remove.
        (ARCH_INIT_CPU_FEATURES): New macro.
---
 csu/libc-start.c                             |  6 ++++++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c | 23 +++++------------------
 sysdeps/x86/libc-start.c                     | 23 +++++------------------
 3 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 9a56dcb..c2dd159 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -104,6 +104,10 @@ apply_irel (void)
 # define MAIN_AUXVEC_PARAM
 #endif
 
+#ifndef ARCH_INIT_CPU_FEATURES
+# define ARCH_INIT_CPU_FEATURES()
+#endif
+
 STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
  MAIN_AUXVEC_DECL),
     int argc,
@@ -182,6 +186,8 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 
   __tunables_init (__environ);
 
+  ARCH_INIT_CPU_FEATURES ();
+
   /* Perform IREL{,A} relocations.  */
   apply_irel ();
 
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
index a5babd4..089a728 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
@@ -16,26 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifdef SHARED
-# include <csu/libc-start.c>
-# else
-/* The main work is done in the generic function.  */
-# define LIBC_START_DISABLE_INLINE
-# define LIBC_START_MAIN generic_start_main
-# include <csu/libc-start.c>
+#ifndef SHARED
+# include <ldsodefs.h>
 # include <cpu-features.c>
 
 extern struct cpu_features _dl_aarch64_cpu_features;
 
-int
-__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
-   int argc, char **argv,
-   __typeof (main) init,
-   void (*fini) (void),
-   void (*rtld_fini) (void), void *stack_end)
-{
-  init_cpu_features (&_dl_aarch64_cpu_features);
-  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
-     stack_end);
-}
+# define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_aarch64_cpu_features)
+
 #endif
+#include <csu/libc-start.c>
diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
index 9a56adc..e11b490 100644
--- a/sysdeps/x86/libc-start.c
+++ b/sysdeps/x86/libc-start.c
@@ -15,27 +15,14 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifdef SHARED
-# include <csu/libc-start.c>
-# else
-/* The main work is done in the generic function.  */
-# define LIBC_START_DISABLE_INLINE
-# define LIBC_START_MAIN generic_start_main
-# include <csu/libc-start.c>
+#ifndef SHARED
+#include <ldsodefs.h>
 # include <cpu-features.h>
 # include <cpu-features.c>
 
 extern struct cpu_features _dl_x86_cpu_features;
 
-int
-__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
-   int argc, char **argv,
-   __typeof (main) init,
-   void (*fini) (void),
-   void (*rtld_fini) (void), void *stack_end)
-{
-  init_cpu_features (&_dl_x86_cpu_features);
-  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
-     stack_end);
-}
+#define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_x86_cpu_features)
+
 #endif
+# include <csu/libc-start.c>
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

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

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
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(-)

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;
 }
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

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

Szabolcs Nagy-2
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.

Reply | Threaded
Open this post in threaded view
|

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

Siddhesh Poyarekar-9
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
Reply | Threaded
Open this post in threaded view
|

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

Szabolcs Nagy
* Siddhesh Poyarekar <[hidden email]> [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).

Reply | Threaded
Open this post in threaded view
|

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

Siddhesh Poyarekar-9
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
Reply | Threaded
Open this post in threaded view
|

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

Szabolcs Nagy
* Siddhesh Poyarekar <[hidden email]> [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).

Reply | Threaded
Open this post in threaded view
|

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

Siddhesh Poyarekar-9
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/5] tunables: Add LD_HWCAP_MASK to tunables

Adhemerval Zanella-2
In reply to this post by Siddhesh Poyarekar-9


On 18/05/2017 17:07, 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 small fix below.

> ---
>  elf/dl-tunables.list     |  7 +++++++
>  manual/tunables.texi     | 22 ++++++++++++++++++++++
>  scripts/gen-tunables.awk |  1 +
>  3 files changed, 30 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..37aefca 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,23 @@ 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.

Missing @end deftp here.

> 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"
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] tunables: Add hooks to get and update tunables

Adhemerval Zanella-2
In reply to this post by Siddhesh Poyarekar-9


On 18/05/2017 17:07, Siddhesh Poyarekar wrote:

> This change adds two new tunable macros to get and update tunables
> respectively called TUNABLE_GET and TUNABLE_UPDATE_VAL.
>
> TUNABLE_GET accepts the top namespace, tunable namespace and tunable
> id along with the target type as arguments and returns a value of the
> target type.  For example:
>
>     TUNABLE_GET (glibc, malloc, check, int32_t)
>
> will return the tunable value for glibc.malloc.check in an int32_t.
>
> TUNABLE_UPDATE_VAL accepts the top namespace, tunable namespace and
> tunable id along with the value to be stored into the tunable
> structure followed by the type of the value.  For example:
>
>   TUNABLE_UPDATE_VAL (glibc, malloc, check, 1, int32_t);
>
> will update the glibc.malloc.check tunable.
>
> Given that the tunable structure is now relro, this is only really
> possible from within the dynamic linker before it is relocated.  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.  However whenever we actually do
> that, we will have to ensure that the mutable tunables are protected
> with locks.
>
> * elf/dl-tunables.h (strval): Replace with a bool initialized.
> (TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
> prevent collision.
> (__tunable_update_val): New function.
> (TUNABLE_GET): New macro.
> (TUNABLE_SET_VAL_FULL_WITH_CALLBACK): New macro.
> (TUNABLE_SET_VAL): Use it.
> (TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
> (TUNABLE_UPDATE_VAL): New macro.
> (tunables_strtoul): Replace strval with initialized.
> (__tunables_init): Likewise.
> (__tunable_set_val): Likewise.
> (do_tunable_update_val): New function.
> (tunables_initialize): Use it.
> (__tunable_update_val): New function.
> * README.tunables: Document the new macros.

LGTM (I added just some document extension points and it is not a patch
blocker).

> ---
>  README.tunables   | 27 +++++++++++++++++++++++++++
>  elf/dl-tunables.c | 43 +++++++++++++++++++++++++++++++++++--------
>  elf/dl-tunables.h | 40 ++++++++++++++++++++++++++++------------
>  3 files changed, 90 insertions(+), 20 deletions(-)
>
> diff --git a/README.tunables b/README.tunables
> index 0e9b0d7..0bea2ca 100644
> --- a/README.tunables
> +++ b/README.tunables
> @@ -75,6 +75,33 @@ The list of allowed attributes are:
>     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.
>  
> +GETTING A TUNABLE VALUE
> +-----------------------
> +
> +One may get the value of a tunable anywhere in glibc code using the TUNABLE_GET
> +macro as follows:
> +
> +  TUNABLE_GET (glibc, malloc, check, int32_t)
> +
> +The first three arguments of the macro are the top namespace, tunable namespace
> +and the tunable name respectively.  The last argument is the type of the
> +tunable and must match the type of the tunable.
> +
> +UPDATING A TUNABLE VALUE
> +------------------------
> +
> +The tunable list is currently stored in a relro section, i.e. it cannot be
> +modified after relocation.  As a result, one may update the value of a tunable
> +only in the dynamic linker.  The TUNABLE_UPDATE_VALUE macro can be used to do
> +this:
> +
> +    TUNABLE_UPDATE_VAL (glibc, malloc, check, val, int32_t);
> +
> +where the first thre arguments of the macro are the top namespace, tunable
> +namespace and the tunable name respectively.  The fourth argument is the value
> +to assign to the tunable and the last argument is the type of the tunable.  The
> +type in the fifth argument must match the type of the tunable.
> +

Wouldn't be worth do point where in code it updates now?  The idea would just to
add some reference point for possible future work.

>  FUTURE WORK
>  -----------
>  
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 8d72e26..abf10e5 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_update_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;
> @@ -432,7 +459,7 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
>  
>    /* Don't do anything if our tunable was not set during initialization or if
>       it failed validation.  */
> -  if (cur->strval == NULL)
> +  if (!cur->initialized)
>      return;
>  
>    /* Caller does not need the value, just call the callback with our tunable
> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index f33adfb..f465370 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,29 +61,45 @@ 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_update_val (tunable_id_t, void *);
> +
> +/* Get and return a tunable value.  */
> +# define TUNABLE_GET(__top, __ns, __id, __type) \
> +({      \
> +  tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id);      \
> +  __type ret;      \
> +  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (id, &ret, NULL);      \
> +  ret;      \
> +})
>  
>  /* 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) \
> -({      \
> -  __tunable_set_val      \
> -   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
> -    NULL);      \
> -})
> +  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE,       \
> + TUNABLE_NAMESPACE,   \
> + __id), (__val), NULL)
>  
>  /* Same as TUNABLE_SET_VAL, but also call the callback function __CB.  */
>  # define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
> +  TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE,       \
> + TUNABLE_NAMESPACE,   \
> + __id),      \
> +      (__val), DL_TUNABLE_CALLBACK (__cb))
> +
> +# define TUNABLE_SET_VAL_FULL_WITH_CALLBACK(__id,__val,__cb)\
> +  __tunable_set_val (__id, (__val), (__cb))
> +
> +# define TUNABLE_UPDATE_VAL(__top, __ns, __id, __val, __type) \
>  ({      \
> -  __tunable_set_val      \
> -   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
> -    DL_TUNABLE_CALLBACK (__cb));      \
> +  __tunable_update_val (TUNABLE_ENUM_NAME (__top, __ns, __id),      \
> + & (__type) {__val});      \
>  })
>  
>  /* Namespace sanity for callback functions.  Use this macro to keep the
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/5] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask

Adhemerval Zanella-2
In reply to this post by Siddhesh Poyarekar-9


On 18/05/2017 17:07, 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-cache.c: Include dl-tunables.h.
> (_dl_load_cache_lookup)[HAVE_TUNABLES]: Read
> glibc.tune.hwcap_mask.
> * elf/dl-hwcaps.c: Include dl-tunables.h.
> (_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update
> glibc.tune.hwcap_mask.
> * sysdeps/sparc/sparc32/dl-machine.h: Likewise.
> * elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop
> _dl_hwcap_mask.
> * elf/dl-tunables.c (__tunable_set_val): Make a hidden alias.
> * elf/dl-tunables.h (__tunable_set_val): Likewise.
> * 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.

> ---
>  elf/dl-cache.c                     |  9 ++++++++-
>  elf/dl-hwcaps.c                    | 15 +++++++++++++--
>  elf/dl-support.c                   |  2 ++
>  elf/dl-tunables.c                  |  1 +
>  elf/dl-tunables.h                  |  2 ++
>  elf/rtld.c                         |  4 ++++
>  sysdeps/generic/ldsodefs.h         |  2 ++
>  sysdeps/sparc/sparc32/dl-machine.h |  8 +++++++-
>  sysdeps/x86/cpu-features.c         |  4 ++++
>  9 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 017c78a..b7ae05c 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-tunables.h>
>  
>  #ifndef _DL_PLATFORMS_COUNT
>  # define _DL_PLATFORMS_COUNT 0
> @@ -258,8 +259,14 @@ _dl_load_cache_lookup (const char *name)
>        if (platform != (uint64_t) -1)
>   platform = 1ULL << platform;
>  
> +#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
> +
>  #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..2b9f7f1 100644
> --- a/elf/dl-hwcaps.c
> +++ b/elf/dl-hwcaps.c
> @@ -24,6 +24,7 @@
>  #include <ldsodefs.h>
>  
>  #include <dl-procinfo.h>
> +#include <dl-tunables.h>
>  
>  #ifdef _DL_FIRST_PLATFORM
>  # define _DL_FIRST_EXTRA (_DL_FIRST_PLATFORM + _DL_PLATFORMS_COUNT)
> @@ -37,8 +38,13 @@ internal_function
>  _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
>        size_t *max_capstrlen)
>  {
> +#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
>    /* 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 +131,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_UPDATE_VAL (glibc, tune, hwcap_mask, hwcap_mask, uint64_t);
> +#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-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/dl-tunables.c b/elf/dl-tunables.c
> index abf10e5..be7733f 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -497,3 +497,4 @@ cb:
>    if (callback)
>      callback (&cur->val);
>  }
> +rtld_hidden_def (__tunable_set_val)
> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index f465370..003e98b 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -70,6 +70,8 @@ extern void __tunables_init (char **);
>  extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
>  extern void __tunable_update_val (tunable_id_t, void *);
>  
> +rtld_hidden_proto (__tunable_set_val)
> +
>  /* Get and return a tunable value.  */
>  # define TUNABLE_GET(__top, __ns, __id, __type) \
>  ({      \
> 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 cf7272f..f5e8078 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-tunables.h>
>  
>  /* Return nonzero iff ELF header is compatible with the running host.  */
>  static inline int
> @@ -39,7 +40,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
>        /* XXX The following is wrong!  Dave Miller rejected to implement it
>   correctly.  If this causes problems shoot *him*!  */
>  #ifdef SHARED
> -      return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9;
> +# 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
> +      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)
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Delay initialization of CPU features struct in static binaries

H.J. Lu-30
In reply to this post by Siddhesh Poyarekar-9
On Thu, May 18, 2017 at 1:07 PM, Siddhesh Poyarekar
<[hidden email]> wrote:

> Allow the CPU features structure set up to be overridden by tunables
> by delaying it to until after tunables are initialized.  The
> initialization is already delayed in dynamically linked glibc, it is
> only in static binaries that the initialization is set early to allow
> it to influence IFUNC relocations that happen in libc-start.  It is a
> bit too early however and there is a good place between tunables
> initialization and IFUNC relocations where this can be done.
>
> Verified that this does not regress the testsuite.
>
>         * csu/libc-start.c [!ARCH_INIT_CPU_FEATURES]: Define
>         ARCH_INIT_CPU_FEATURES.
>         (LIBC_START_MAIN): Call it.
>         * sysdeps/unix/sysv/linux/aarch64/libc-start.c
>         (__libc_start_main): Remove.
>         (ARCH_INIT_CPU_FEATURES): New macro.
>         * sysdeps/x86/libc-start.c (__libc_start_main): Remove.
>         (ARCH_INIT_CPU_FEATURES): New macro.
> ---
>

Looks good to me.

Thanks.

--
H.J.