[v5] PowerPC: Influence cpu/arch hwcap features via GLIBC_TUNABLES.
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Testing failed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
Commit Message
From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
This patch enables the option to influence hwcaps used by PowerPC.
The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
and zzz, where the feature name is case-sensitive and has to match the ones
mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
Note that the tunable only handles the features which are really used
in the IFUNC selection. All others are ignored as the values are only
used inside glibc.
---
manual/tunables.texi | 5 +-
sysdeps/powerpc/cpu-features.c | 97 ++++++++++++++++-
sysdeps/powerpc/cpu-features.h | 102 ++++++++++++++++++
sysdeps/powerpc/dl-tunables.list | 3 +
sysdeps/powerpc/hwcapinfo.c | 4 +
.../power4/multiarch/ifunc-impl-list.c | 4 +-
.../powerpc32/power4/multiarch/init-arch.h | 10 +-
sysdeps/powerpc/powerpc64/dl-machine.h | 2 -
.../powerpc64/multiarch/ifunc-impl-list.c | 7 +-
9 files changed, 222 insertions(+), 12 deletions(-)
Comments
On 7/10/23 1:21 PM, bmahi496@linux.ibm.com wrote:
> Note that the tunable only handles the features which are really used
> in the IFUNC selection. All others are ignored as the values are only
> used inside glibc.
I don't understand this comment. Your code seems to handle all hwcap
features, not just the features used within glibc. That's fine with me,
because I'd like these changes to eventually be exported to the TCB and
then we should handle all known features. Maybe the comment just needs
updating?
> + /* Copy the features from dl_powerpc_cpu_features, which contains the
> + features provided by AT_HWCAP and AT_HWCAP2. */
> + struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
^
Space needed before the '('.
> + tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
Ditto for both '('s and before and after the '/'.
> + unsigned short offset=0;
Please make this either size_t or unsigned long. This isn't a struct
field or array element type, so we don't need to use the smallest
type available to save space. Using unsigned short here just causes
some unnecessary sign extensions in the loop. If you use size_t or
unsigned long, we won't get them.
> + /* Enable the features and also checking that no unsupported
> + features were enabled by user. */
s/checking/check/
> + offset = offset + STRLEN_DEFAULT (hwcap_name) + 1;
> + }
> + token += token_len;
NIT: I think saying "offset += STRLEN_DEFAULT (hwcap_name) + 1;"
better matches your other usage like for token above and
other examples. Ignore if you wish.
> @@ -63,6 +64,9 @@ __tcb_parse_hwcap_and_convert_at_platform (void)
> else if (h1 & PPC_FEATURE_POWER5)
> h1 |= PPC_FEATURE_POWER4;
>
> + uint64_t array_hwcaps[] = { h1, h2 };
> + init_cpu_features (&GLRO(dl_powerpc_cpu_features), array_hwcaps);
^
Space before '('.
> @@ -28,7 +29,8 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> {
> size_t i = max;
>
> - unsigned long int hwcap = GLRO(dl_hwcap);
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
^
Ditto.
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features); \
^
Ditto.
> @@ -27,9 +28,9 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> size_t max)
> {
> size_t i = max;
> -
> - unsigned long int hwcap = GLRO(dl_hwcap);
> - unsigned long int hwcap2 = GLRO(dl_hwcap2);
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
^
Ditto.
Peter
On 10/07/23 18:28, Peter Bergner wrote:
> On 7/10/23 1:21 PM, bmahi496@linux.ibm.com wrote:
>> Note that the tunable only handles the features which are really used
>> in the IFUNC selection. All others are ignored as the values are only
>> used inside glibc.
>
> I don't understand this comment. Your code seems to handle all hwcap
> features, not just the features used within glibc. That's fine with me,
> because I'd like these changes to eventually be exported to the TCB and
> then we should handle all known features. Maybe the comment just needs
> updating?
>
>
>
>
>> + /* Copy the features from dl_powerpc_cpu_features, which contains the
>> + features provided by AT_HWCAP and AT_HWCAP2. */
>> + struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
> ^
> Space needed before the '('.
For variable-like access, such as with macro for GL or GLRO, there is no
need for the space.
>
>
>
>> + tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
>
> Ditto for both '('s and before and after the '/'.
User array_length macro instead.
On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>
> This patch enables the option to influence hwcaps used by PowerPC.
> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
> and zzz, where the feature name is case-sensitive and has to match the ones
> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>
> Note that the tunable only handles the features which are really used
> in the IFUNC selection. All others are ignored as the values are only
> used inside glibc.
It is still missing a regression test to check if tunable work as intended.
> ---
> manual/tunables.texi | 5 +-
> sysdeps/powerpc/cpu-features.c | 97 ++++++++++++++++-
> sysdeps/powerpc/cpu-features.h | 102 ++++++++++++++++++
> sysdeps/powerpc/dl-tunables.list | 3 +
> sysdeps/powerpc/hwcapinfo.c | 4 +
> .../power4/multiarch/ifunc-impl-list.c | 4 +-
> .../powerpc32/power4/multiarch/init-arch.h | 10 +-
> sysdeps/powerpc/powerpc64/dl-machine.h | 2 -
> .../powerpc64/multiarch/ifunc-impl-list.c | 7 +-
> 9 files changed, 222 insertions(+), 12 deletions(-)
>
> diff --git a/manual/tunables.texi b/manual/tunables.texi
> index 4ca0e42a11..776fd93fd9 100644
> --- a/manual/tunables.texi
> +++ b/manual/tunables.texi
> @@ -513,7 +513,10 @@ On s390x, the supported HWCAP and STFLE features can be found in
> @code{sysdeps/s390/cpu-features.c}. In addition the user can also set
> a CPU arch-level like @code{z13} instead of single HWCAP and STFLE features.
>
> -This tunable is specific to i386, x86-64 and s390x.
> +On powerpc, the supported HWCAP and HWCAP2 features can be found in
> +@code{sysdeps/powerpc/dl-procinfo.c}.
> +
> +This tunable is specific to i386, x86-64, s390x and powerpc.
> @end deftp
>
> @deftp Tunable glibc.cpu.cached_memopt
> diff --git a/sysdeps/powerpc/cpu-features.c b/sysdeps/powerpc/cpu-features.c
> index 0ef3cf89d2..bf1c5353da 100644
> --- a/sysdeps/powerpc/cpu-features.c
> +++ b/sysdeps/powerpc/cpu-features.c
> @@ -19,14 +19,109 @@
> #include <stdint.h>
> #include <cpu-features.h>
> #include <elf/dl-tunables.h>
> +#include <unistd.h>
> +#include <string.h>
> +#define MEMCMP_DEFAULT memcmp
> +#define STRLEN_DEFAULT strlen
> +
> +static void
> +TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> +{
> + /* The current IFUNC selection is always using the most recent
> + features which are available via AT_HWCAP or AT_HWCAP2. But in
> + some scenarios it is useful to adjust this selection.
> +
> + The environment variable:
> +
> + GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,....
> +
> + Can be used to enable HWCAP/HWCAP2 feature yyy, disable HWCAP/HWCAP2
> + feature xxx, where the feature name is case-sensitive and has to match
> + the ones mentioned in the file{sysdeps/powerpc/dl-procinfo.c}. */
> +
> + /* Copy the features from dl_powerpc_cpu_features, which contains the
> + features provided by AT_HWCAP and AT_HWCAP2. */
> + struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
> + unsigned long int tcbv_hwcap = cpu_features->hwcap;
> + unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
> + unsigned int tun_count;
> + const char *token = valp->strval;
> + tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
> + do
> + {
> + const char *token_end, *feature;
> + bool disable;
> + unsigned short offset=0;
> + size_t token_len, i, feature_len;
> + /* Find token separator or end of string. */
> + for (token_end = token; *token_end != ','; token_end++)
> + if (*token_end == '\0')
> + break;
> +
> + /* Determine feature. */
> + token_len = token_end - token;
> + if (*token == '-')
> + {
> + disable = true;
> + feature = token + 1;
> + feature_len = token_len - 1;
> + }
> + else
> + {
> + disable = false;
> + feature = token;
> + feature_len = token_len;
> + }
> + for (i = 0; i < tun_count; ++i)
> + {
> + const char *hwcap_name = hwcap_names + offset;
> + /* Check the tunable name on the supported list. */
> + if (STRLEN_DEFAULT (hwcap_name) == feature_len
> + && MEMCMP_DEFAULT (feature, hwcap_name, feature_len)
> + == 0)
> + {
> + /* Update the hwcap and hwcap2 bits. */
> + if (disable)
> + {
> + /* Id is 1 for hwcap2 tunable. */
> + if (hwcap_tunables[i].id)
> + cpu_features->hwcap2 &= ~(hwcap_tunables[i].mask);
> + else
> + cpu_features->hwcap &= ~(hwcap_tunables[i].mask);
> + }
> + else
> + {
> + /* Enable the features and also checking that no unsupported
> + features were enabled by user. */
> + if (hwcap_tunables[i].id)
> + cpu_features->hwcap2 |= (tcbv_hwcap2 & hwcap_tunables[i].mask);
> + else
> + cpu_features->hwcap |= (tcbv_hwcap & hwcap_tunables[i].mask);
> + }
> + break;
> + }
> + offset = offset + STRLEN_DEFAULT (hwcap_name) + 1;
> + }
> + token += token_len;
> + /* ... and skip token separator for next round. */
> + if (*token == ',') token++;
> + }
> + while (*token != '\0');
> +}
>
> static inline void
> -init_cpu_features (struct cpu_features *cpu_features)
> +init_cpu_features (struct cpu_features *cpu_features, uint64_t hwcaps[])
> {
> + /* Fill the cpu_features with the supported hwcaps
> + which are set by __tcb_parse_hwcap_and_convert_at_platform. */
> + cpu_features->hwcap = hwcaps[0];
> + cpu_features->hwcap2 = hwcaps[1];
> /* Default is to use aligned memory access on optimized function unless
> tunables is enable, since for this case user can explicit disable
> unaligned optimizations. */
> int32_t cached_memfunc = TUNABLE_GET (glibc, cpu, cached_memopt, int32_t,
> NULL);
> cpu_features->use_cached_memopt = (cached_memfunc > 0);
> + TUNABLE_GET (glibc, cpu, hwcaps, tunable_val_t *,
> + TUNABLE_CALLBACK (set_hwcaps));
> }
> diff --git a/sysdeps/powerpc/cpu-features.h b/sysdeps/powerpc/cpu-features.h
> index d316dc3d64..e5fce88e5e 100644
> --- a/sysdeps/powerpc/cpu-features.h
> +++ b/sysdeps/powerpc/cpu-features.h
> @@ -19,10 +19,112 @@
> # define __CPU_FEATURES_POWERPC_H
>
> #include <stdbool.h>
> +#include <sys/auxv.h>
>
> struct cpu_features
> {
> bool use_cached_memopt;
> + unsigned long int hwcap;
> + unsigned long int hwcap2;
> +};
> +
> +static const char hwcap_names[] = {
> + "4xxmac\0"
> + "altivec\0"
> + "arch_2_05\0"
> + "arch_2_06\0"
> + "archpmu\0"
> + "booke\0"
> + "cellbe\0"
> + "dfp\0"
> + "efpdouble\0"
> + "efpsingle\0"
> + "fpu\0"
> + "ic_snoop\0"
> + "mmu\0"
> + "notb\0"
> + "pa6t\0"
> + "power4\0"
> + "power5\0"
> + "power5+\0"
> + "power6x\0"
> + "ppc32\0"
> + "ppc601\0"
> + "ppc64\0"
> + "ppcle\0"
> + "smt\0"
> + "spe\0"
> + "true_le\0"
> + "ucache\0"
> + "vsx\0"
> + "arch_2_07\0"
> + "dscr\0"
> + "ebb\0"
> + "htm\0"
> + "htm-nosc\0"
> + "htm-no-suspend\0"
> + "isel\0"
> + "tar\0"
> + "vcrypto\0"
> + "arch_3_00\0"
> + "ieee128\0"
> + "darn\0"
> + "scv\0"
> + "arch_3_1\0"
> + "mma\0"
> +};
> +
> +static const struct
> +{
> + unsigned int mask;
> + bool id;
> +} hwcap_tunables[] = {
> + /* AT_HWCAP tunable masks. */
> + { PPC_FEATURE_HAS_4xxMAC, 0 },
> + { PPC_FEATURE_HAS_ALTIVEC, 0 },
> + { PPC_FEATURE_ARCH_2_05, 0 },
> + { PPC_FEATURE_ARCH_2_06, 0 },
> + { PPC_FEATURE_PSERIES_PERFMON_COMPAT, 0 },
> + { PPC_FEATURE_BOOKE, 0 },
> + { PPC_FEATURE_CELL_BE, 0 },
> + { PPC_FEATURE_HAS_DFP, 0 },
> + { PPC_FEATURE_HAS_EFP_DOUBLE, 0 },
> + { PPC_FEATURE_HAS_EFP_SINGLE, 0 },
> + { PPC_FEATURE_HAS_FPU, 0 },
> + { PPC_FEATURE_ICACHE_SNOOP, 0 },
> + { PPC_FEATURE_HAS_MMU, 0 },
> + { PPC_FEATURE_NO_TB, 0 },
> + { PPC_FEATURE_PA6T, 0 },
> + { PPC_FEATURE_POWER4, 0 },
> + { PPC_FEATURE_POWER5, 0 },
> + { PPC_FEATURE_POWER5_PLUS, 0 },
> + { PPC_FEATURE_POWER6_EXT, 0 },
> + { PPC_FEATURE_32, 0 },
> + { PPC_FEATURE_601_INSTR, 0 },
> + { PPC_FEATURE_64, 0 },
> + { PPC_FEATURE_PPC_LE, 0 },
> + { PPC_FEATURE_SMT, 0 },
> + { PPC_FEATURE_HAS_SPE, 0 },
> + { PPC_FEATURE_TRUE_LE, 0 },
> + { PPC_FEATURE_UNIFIED_CACHE, 0 },
> + { PPC_FEATURE_HAS_VSX, 0 },
> +
> + /* AT_HWCAP2 tunable masks. */
> + { PPC_FEATURE2_ARCH_2_07, 1 },
> + { PPC_FEATURE2_HAS_DSCR, 1 },
> + { PPC_FEATURE2_HAS_EBB, 1 },
> + { PPC_FEATURE2_HAS_HTM, 1 },
> + { PPC_FEATURE2_HTM_NOSC, 1 },
> + { PPC_FEATURE2_HTM_NO_SUSPEND, 1 },
> + { PPC_FEATURE2_HAS_ISEL, 1 },
> + { PPC_FEATURE2_HAS_TAR, 1 },
> + { PPC_FEATURE2_HAS_VEC_CRYPTO, 1 },
> + { PPC_FEATURE2_ARCH_3_00, 1 },
> + { PPC_FEATURE2_HAS_IEEE128, 1 },
> + { PPC_FEATURE2_DARN, 1 },
> + { PPC_FEATURE2_SCV, 1 },
> + { PPC_FEATURE2_ARCH_3_1, 1 },
> + { PPC_FEATURE2_MMA, 1 },
> };
>
> #endif /* __CPU_FEATURES_H */
> diff --git a/sysdeps/powerpc/dl-tunables.list b/sysdeps/powerpc/dl-tunables.list
> index 87d6235c75..807b7f8013 100644
> --- a/sysdeps/powerpc/dl-tunables.list
> +++ b/sysdeps/powerpc/dl-tunables.list
> @@ -24,5 +24,8 @@ glibc {
> maxval: 1
> default: 0
> }
> + hwcaps {
> + type: STRING
> + }
> }
> }
> diff --git a/sysdeps/powerpc/hwcapinfo.c b/sysdeps/powerpc/hwcapinfo.c
> index e26e64d99e..f2c473c556 100644
> --- a/sysdeps/powerpc/hwcapinfo.c
> +++ b/sysdeps/powerpc/hwcapinfo.c
> @@ -19,6 +19,7 @@
> #include <unistd.h>
> #include <shlib-compat.h>
> #include <dl-procinfo.h>
> +#include <cpu-features.c>
>
> tcbhead_t __tcb __attribute__ ((visibility ("hidden")));
>
> @@ -63,6 +64,9 @@ __tcb_parse_hwcap_and_convert_at_platform (void)
> else if (h1 & PPC_FEATURE_POWER5)
> h1 |= PPC_FEATURE_POWER4;
>
> + uint64_t array_hwcaps[] = { h1, h2 };
> + init_cpu_features (&GLRO(dl_powerpc_cpu_features), array_hwcaps);
> +
> /* Consolidate both HWCAP and HWCAP2 into a single doubleword so that
> we can read both in a single load later. */
> __tcb.hwcap = (h1 << 32) | (h2 & 0xffffffff);
> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
> index b4f80539e7..986c37d71e 100644
> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
> @@ -21,6 +21,7 @@
> #include <wchar.h>
> #include <ldsodefs.h>
> #include <ifunc-impl-list.h>
> +#include <cpu-features.h>
>
> size_t
> __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> @@ -28,7 +29,8 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> {
> size_t i = max;
>
> - unsigned long int hwcap = GLRO(dl_hwcap);
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
> + unsigned long int hwcap = features->hwcap;
> /* hwcap contains only the latest supported ISA, the code checks which is
> and fills the previous supported ones. */
> if (hwcap & PPC_FEATURE_ARCH_2_06)
> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h b/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
> index 3dd00e02ee..a0bbd12012 100644
> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
> @@ -16,6 +16,7 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <ldsodefs.h>
> +#include <cpu-features.h>
>
> /* The code checks if _rtld_global_ro was realocated before trying to access
> the dl_hwcap field. The assembly is to make the compiler not optimize the
> @@ -32,11 +33,12 @@
> # define __GLRO(value) GLRO(value)
> #endif
>
> -/* dl_hwcap contains only the latest supported ISA, the macro checks which is
> - and fills the previous ones. */
> +/* Get the hardware information post the tunables set , the macro checks
> + it and fills the previous ones. */
> #define INIT_ARCH() \
> - unsigned long int hwcap = __GLRO(dl_hwcap); \
> - unsigned long int __attribute__((unused)) hwcap2 = __GLRO(dl_hwcap2); \
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features); \
> + unsigned long int hwcap = features->hwcap; \
> + unsigned long int __attribute__((unused)) hwcap2 = features->hwcap2; \
> bool __attribute__((unused)) use_cached_memopt = \
> __GLRO(dl_powerpc_cpu_features.use_cached_memopt); \
> if (hwcap & PPC_FEATURE_ARCH_2_06) \
> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> index 9b8943bc91..449208e86f 100644
> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> @@ -27,7 +27,6 @@
> #include <dl-tls.h>
> #include <sysdep.h>
> #include <hwcapinfo.h>
> -#include <cpu-features.c>
> #include <dl-static-tls.h>
> #include <dl-funcdesc.h>
> #include <dl-machine-rel.h>
> @@ -297,7 +296,6 @@ static inline void __attribute__ ((unused))
> dl_platform_init (void)
> {
> __tcb_parse_hwcap_and_convert_at_platform ();
> - init_cpu_features (&GLRO(dl_powerpc_cpu_features));
> }
> #endif
>
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> index ebe9434052..fc26dd0e17 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> @@ -17,6 +17,7 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <assert.h>
> +#include <cpu-features.h>
> #include <string.h>
> #include <wchar.h>
> #include <ldsodefs.h>
> @@ -27,9 +28,9 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> size_t max)
> {
> size_t i = max;
> -
> - unsigned long int hwcap = GLRO(dl_hwcap);
> - unsigned long int hwcap2 = GLRO(dl_hwcap2);
> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
> + unsigned long int hwcap = features->hwcap;
> + unsigned long int hwcap2 = features->hwcap2;
> #ifdef SHARED
> int cacheline_size = GLRO(dl_cache_line_size);
> #endif
On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>
> On 10/07/23 15:21,bmahi496@linux.ibm.com wrote:
>> From: Mahesh Bodapati<mahesh.bodapati@ibm.com>
>>
>> This patch enables the option to influence hwcaps used by PowerPC.
>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>> and zzz, where the feature name is case-sensitive and has to match the ones
>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>
>> Note that the tunable only handles the features which are really used
>> in the IFUNC selection. All others are ignored as the values are only
>> used inside glibc.
> It is still missing a regression test to check if tunable work as intended.
I ran glibc tests with and without setting hwcap tunables and no
regressions were found.
Summary of test results:
4458 PASS
9 UNSUPPORTED
16 XFAIL
2 XPASS
The runtime behavior is as expected when we set hwcap tunables.
We tested on power10,power9 BE and powerpc32 machines and the results
were good.
Could you go through the attached sheet ,we have listed runtime behavior
for some of the tunable options.
We are not sure on how to add regressions tests for tunable feature.
I didn't see hwcap tunable specific tests added for other targets as well.
>
>> ---
>> manual/tunables.texi | 5 +-
>> sysdeps/powerpc/cpu-features.c | 97 ++++++++++++++++-
>> sysdeps/powerpc/cpu-features.h | 102 ++++++++++++++++++
>> sysdeps/powerpc/dl-tunables.list | 3 +
>> sysdeps/powerpc/hwcapinfo.c | 4 +
>> .../power4/multiarch/ifunc-impl-list.c | 4 +-
>> .../powerpc32/power4/multiarch/init-arch.h | 10 +-
>> sysdeps/powerpc/powerpc64/dl-machine.h | 2 -
>> .../powerpc64/multiarch/ifunc-impl-list.c | 7 +-
>> 9 files changed, 222 insertions(+), 12 deletions(-)
>>
>> diff --git a/manual/tunables.texi b/manual/tunables.texi
>> index 4ca0e42a11..776fd93fd9 100644
>> --- a/manual/tunables.texi
>> +++ b/manual/tunables.texi
>> @@ -513,7 +513,10 @@ On s390x, the supported HWCAP and STFLE features can be found in
>> @code{sysdeps/s390/cpu-features.c}. In addition the user can also set
>> a CPU arch-level like @code{z13} instead of single HWCAP and STFLE features.
>>
>> -This tunable is specific to i386, x86-64 and s390x.
>> +On powerpc, the supported HWCAP and HWCAP2 features can be found in
>> +@code{sysdeps/powerpc/dl-procinfo.c}.
>> +
>> +This tunable is specific to i386, x86-64, s390x and powerpc.
>> @end deftp
>>
>> @deftp Tunable glibc.cpu.cached_memopt
>> diff --git a/sysdeps/powerpc/cpu-features.c b/sysdeps/powerpc/cpu-features.c
>> index 0ef3cf89d2..bf1c5353da 100644
>> --- a/sysdeps/powerpc/cpu-features.c
>> +++ b/sysdeps/powerpc/cpu-features.c
>> @@ -19,14 +19,109 @@
>> #include <stdint.h>
>> #include <cpu-features.h>
>> #include <elf/dl-tunables.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +#define MEMCMP_DEFAULT memcmp
>> +#define STRLEN_DEFAULT strlen
>> +
>> +static void
>> +TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
>> +{
>> + /* The current IFUNC selection is always using the most recent
>> + features which are available via AT_HWCAP or AT_HWCAP2. But in
>> + some scenarios it is useful to adjust this selection.
>> +
>> + The environment variable:
>> +
>> + GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,....
>> +
>> + Can be used to enable HWCAP/HWCAP2 feature yyy, disable HWCAP/HWCAP2
>> + feature xxx, where the feature name is case-sensitive and has to match
>> + the ones mentioned in the file{sysdeps/powerpc/dl-procinfo.c}. */
>> +
>> + /* Copy the features from dl_powerpc_cpu_features, which contains the
>> + features provided by AT_HWCAP and AT_HWCAP2. */
>> + struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
>> + unsigned long int tcbv_hwcap = cpu_features->hwcap;
>> + unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
>> + unsigned int tun_count;
>> + const char *token = valp->strval;
>> + tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
>> + do
>> + {
>> + const char *token_end, *feature;
>> + bool disable;
>> + unsigned short offset=0;
>> + size_t token_len, i, feature_len;
>> + /* Find token separator or end of string. */
>> + for (token_end = token; *token_end != ','; token_end++)
>> + if (*token_end == '\0')
>> + break;
>> +
>> + /* Determine feature. */
>> + token_len = token_end - token;
>> + if (*token == '-')
>> + {
>> + disable = true;
>> + feature = token + 1;
>> + feature_len = token_len - 1;
>> + }
>> + else
>> + {
>> + disable = false;
>> + feature = token;
>> + feature_len = token_len;
>> + }
>> + for (i = 0; i < tun_count; ++i)
>> + {
>> + const char *hwcap_name = hwcap_names + offset;
>> + /* Check the tunable name on the supported list. */
>> + if (STRLEN_DEFAULT (hwcap_name) == feature_len
>> + && MEMCMP_DEFAULT (feature, hwcap_name, feature_len)
>> + == 0)
>> + {
>> + /* Update the hwcap and hwcap2 bits. */
>> + if (disable)
>> + {
>> + /* Id is 1 for hwcap2 tunable. */
>> + if (hwcap_tunables[i].id)
>> + cpu_features->hwcap2 &= ~(hwcap_tunables[i].mask);
>> + else
>> + cpu_features->hwcap &= ~(hwcap_tunables[i].mask);
>> + }
>> + else
>> + {
>> + /* Enable the features and also checking that no unsupported
>> + features were enabled by user. */
>> + if (hwcap_tunables[i].id)
>> + cpu_features->hwcap2 |= (tcbv_hwcap2 & hwcap_tunables[i].mask);
>> + else
>> + cpu_features->hwcap |= (tcbv_hwcap & hwcap_tunables[i].mask);
>> + }
>> + break;
>> + }
>> + offset = offset + STRLEN_DEFAULT (hwcap_name) + 1;
>> + }
>> + token += token_len;
>> + /* ... and skip token separator for next round. */
>> + if (*token == ',') token++;
>> + }
>> + while (*token != '\0');
>> +}
>>
>> static inline void
>> -init_cpu_features (struct cpu_features *cpu_features)
>> +init_cpu_features (struct cpu_features *cpu_features, uint64_t hwcaps[])
>> {
>> + /* Fill the cpu_features with the supported hwcaps
>> + which are set by __tcb_parse_hwcap_and_convert_at_platform. */
>> + cpu_features->hwcap = hwcaps[0];
>> + cpu_features->hwcap2 = hwcaps[1];
>> /* Default is to use aligned memory access on optimized function unless
>> tunables is enable, since for this case user can explicit disable
>> unaligned optimizations. */
>> int32_t cached_memfunc = TUNABLE_GET (glibc, cpu, cached_memopt, int32_t,
>> NULL);
>> cpu_features->use_cached_memopt = (cached_memfunc > 0);
>> + TUNABLE_GET (glibc, cpu, hwcaps, tunable_val_t *,
>> + TUNABLE_CALLBACK (set_hwcaps));
>> }
>> diff --git a/sysdeps/powerpc/cpu-features.h b/sysdeps/powerpc/cpu-features.h
>> index d316dc3d64..e5fce88e5e 100644
>> --- a/sysdeps/powerpc/cpu-features.h
>> +++ b/sysdeps/powerpc/cpu-features.h
>> @@ -19,10 +19,112 @@
>> # define __CPU_FEATURES_POWERPC_H
>>
>> #include <stdbool.h>
>> +#include <sys/auxv.h>
>>
>> struct cpu_features
>> {
>> bool use_cached_memopt;
>> + unsigned long int hwcap;
>> + unsigned long int hwcap2;
>> +};
>> +
>> +static const char hwcap_names[] = {
>> + "4xxmac\0"
>> + "altivec\0"
>> + "arch_2_05\0"
>> + "arch_2_06\0"
>> + "archpmu\0"
>> + "booke\0"
>> + "cellbe\0"
>> + "dfp\0"
>> + "efpdouble\0"
>> + "efpsingle\0"
>> + "fpu\0"
>> + "ic_snoop\0"
>> + "mmu\0"
>> + "notb\0"
>> + "pa6t\0"
>> + "power4\0"
>> + "power5\0"
>> + "power5+\0"
>> + "power6x\0"
>> + "ppc32\0"
>> + "ppc601\0"
>> + "ppc64\0"
>> + "ppcle\0"
>> + "smt\0"
>> + "spe\0"
>> + "true_le\0"
>> + "ucache\0"
>> + "vsx\0"
>> + "arch_2_07\0"
>> + "dscr\0"
>> + "ebb\0"
>> + "htm\0"
>> + "htm-nosc\0"
>> + "htm-no-suspend\0"
>> + "isel\0"
>> + "tar\0"
>> + "vcrypto\0"
>> + "arch_3_00\0"
>> + "ieee128\0"
>> + "darn\0"
>> + "scv\0"
>> + "arch_3_1\0"
>> + "mma\0"
>> +};
>> +
>> +static const struct
>> +{
>> + unsigned int mask;
>> + bool id;
>> +} hwcap_tunables[] = {
>> + /* AT_HWCAP tunable masks. */
>> + { PPC_FEATURE_HAS_4xxMAC, 0 },
>> + { PPC_FEATURE_HAS_ALTIVEC, 0 },
>> + { PPC_FEATURE_ARCH_2_05, 0 },
>> + { PPC_FEATURE_ARCH_2_06, 0 },
>> + { PPC_FEATURE_PSERIES_PERFMON_COMPAT, 0 },
>> + { PPC_FEATURE_BOOKE, 0 },
>> + { PPC_FEATURE_CELL_BE, 0 },
>> + { PPC_FEATURE_HAS_DFP, 0 },
>> + { PPC_FEATURE_HAS_EFP_DOUBLE, 0 },
>> + { PPC_FEATURE_HAS_EFP_SINGLE, 0 },
>> + { PPC_FEATURE_HAS_FPU, 0 },
>> + { PPC_FEATURE_ICACHE_SNOOP, 0 },
>> + { PPC_FEATURE_HAS_MMU, 0 },
>> + { PPC_FEATURE_NO_TB, 0 },
>> + { PPC_FEATURE_PA6T, 0 },
>> + { PPC_FEATURE_POWER4, 0 },
>> + { PPC_FEATURE_POWER5, 0 },
>> + { PPC_FEATURE_POWER5_PLUS, 0 },
>> + { PPC_FEATURE_POWER6_EXT, 0 },
>> + { PPC_FEATURE_32, 0 },
>> + { PPC_FEATURE_601_INSTR, 0 },
>> + { PPC_FEATURE_64, 0 },
>> + { PPC_FEATURE_PPC_LE, 0 },
>> + { PPC_FEATURE_SMT, 0 },
>> + { PPC_FEATURE_HAS_SPE, 0 },
>> + { PPC_FEATURE_TRUE_LE, 0 },
>> + { PPC_FEATURE_UNIFIED_CACHE, 0 },
>> + { PPC_FEATURE_HAS_VSX, 0 },
>> +
>> + /* AT_HWCAP2 tunable masks. */
>> + { PPC_FEATURE2_ARCH_2_07, 1 },
>> + { PPC_FEATURE2_HAS_DSCR, 1 },
>> + { PPC_FEATURE2_HAS_EBB, 1 },
>> + { PPC_FEATURE2_HAS_HTM, 1 },
>> + { PPC_FEATURE2_HTM_NOSC, 1 },
>> + { PPC_FEATURE2_HTM_NO_SUSPEND, 1 },
>> + { PPC_FEATURE2_HAS_ISEL, 1 },
>> + { PPC_FEATURE2_HAS_TAR, 1 },
>> + { PPC_FEATURE2_HAS_VEC_CRYPTO, 1 },
>> + { PPC_FEATURE2_ARCH_3_00, 1 },
>> + { PPC_FEATURE2_HAS_IEEE128, 1 },
>> + { PPC_FEATURE2_DARN, 1 },
>> + { PPC_FEATURE2_SCV, 1 },
>> + { PPC_FEATURE2_ARCH_3_1, 1 },
>> + { PPC_FEATURE2_MMA, 1 },
>> };
>>
>> #endif /* __CPU_FEATURES_H */
>> diff --git a/sysdeps/powerpc/dl-tunables.list b/sysdeps/powerpc/dl-tunables.list
>> index 87d6235c75..807b7f8013 100644
>> --- a/sysdeps/powerpc/dl-tunables.list
>> +++ b/sysdeps/powerpc/dl-tunables.list
>> @@ -24,5 +24,8 @@ glibc {
>> maxval: 1
>> default: 0
>> }
>> + hwcaps {
>> + type: STRING
>> + }
>> }
>> }
>> diff --git a/sysdeps/powerpc/hwcapinfo.c b/sysdeps/powerpc/hwcapinfo.c
>> index e26e64d99e..f2c473c556 100644
>> --- a/sysdeps/powerpc/hwcapinfo.c
>> +++ b/sysdeps/powerpc/hwcapinfo.c
>> @@ -19,6 +19,7 @@
>> #include <unistd.h>
>> #include <shlib-compat.h>
>> #include <dl-procinfo.h>
>> +#include <cpu-features.c>
>>
>> tcbhead_t __tcb __attribute__ ((visibility ("hidden")));
>>
>> @@ -63,6 +64,9 @@ __tcb_parse_hwcap_and_convert_at_platform (void)
>> else if (h1 & PPC_FEATURE_POWER5)
>> h1 |= PPC_FEATURE_POWER4;
>>
>> + uint64_t array_hwcaps[] = { h1, h2 };
>> + init_cpu_features (&GLRO(dl_powerpc_cpu_features), array_hwcaps);
>> +
>> /* Consolidate both HWCAP and HWCAP2 into a single doubleword so that
>> we can read both in a single load later. */
>> __tcb.hwcap = (h1 << 32) | (h2 & 0xffffffff);
>> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
>> index b4f80539e7..986c37d71e 100644
>> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
>> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c
>> @@ -21,6 +21,7 @@
>> #include <wchar.h>
>> #include <ldsodefs.h>
>> #include <ifunc-impl-list.h>
>> +#include <cpu-features.h>
>>
>> size_t
>> __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> @@ -28,7 +29,8 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> {
>> size_t i = max;
>>
>> - unsigned long int hwcap = GLRO(dl_hwcap);
>> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
>> + unsigned long int hwcap = features->hwcap;
>> /* hwcap contains only the latest supported ISA, the code checks which is
>> and fills the previous supported ones. */
>> if (hwcap & PPC_FEATURE_ARCH_2_06)
>> diff --git a/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h b/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
>> index 3dd00e02ee..a0bbd12012 100644
>> --- a/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
>> +++ b/sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
>> @@ -16,6 +16,7 @@
>> <https://www.gnu.org/licenses/>. */
>>
>> #include <ldsodefs.h>
>> +#include <cpu-features.h>
>>
>> /* The code checks if _rtld_global_ro was realocated before trying to access
>> the dl_hwcap field. The assembly is to make the compiler not optimize the
>> @@ -32,11 +33,12 @@
>> # define __GLRO(value) GLRO(value)
>> #endif
>>
>> -/* dl_hwcap contains only the latest supported ISA, the macro checks which is
>> - and fills the previous ones. */
>> +/* Get the hardware information post the tunables set , the macro checks
>> + it and fills the previous ones. */
>> #define INIT_ARCH() \
>> - unsigned long int hwcap = __GLRO(dl_hwcap); \
>> - unsigned long int __attribute__((unused)) hwcap2 = __GLRO(dl_hwcap2); \
>> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features); \
>> + unsigned long int hwcap = features->hwcap; \
>> + unsigned long int __attribute__((unused)) hwcap2 = features->hwcap2; \
>> bool __attribute__((unused)) use_cached_memopt = \
>> __GLRO(dl_powerpc_cpu_features.use_cached_memopt); \
>> if (hwcap & PPC_FEATURE_ARCH_2_06) \
>> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
>> index 9b8943bc91..449208e86f 100644
>> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
>> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
>> @@ -27,7 +27,6 @@
>> #include <dl-tls.h>
>> #include <sysdep.h>
>> #include <hwcapinfo.h>
>> -#include <cpu-features.c>
>> #include <dl-static-tls.h>
>> #include <dl-funcdesc.h>
>> #include <dl-machine-rel.h>
>> @@ -297,7 +296,6 @@ static inline void __attribute__ ((unused))
>> dl_platform_init (void)
>> {
>> __tcb_parse_hwcap_and_convert_at_platform ();
>> - init_cpu_features (&GLRO(dl_powerpc_cpu_features));
>> }
>> #endif
>>
>> diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
>> index ebe9434052..fc26dd0e17 100644
>> --- a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
>> +++ b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
>> @@ -17,6 +17,7 @@
>> <https://www.gnu.org/licenses/>. */
>>
>> #include <assert.h>
>> +#include <cpu-features.h>
>> #include <string.h>
>> #include <wchar.h>
>> #include <ldsodefs.h>
>> @@ -27,9 +28,9 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> size_t max)
>> {
>> size_t i = max;
>> -
>> - unsigned long int hwcap = GLRO(dl_hwcap);
>> - unsigned long int hwcap2 = GLRO(dl_hwcap2);
>> + const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
>> + unsigned long int hwcap = features->hwcap;
>> + unsigned long int hwcap2 = features->hwcap2;
>> #ifdef SHARED
>> int cacheline_size = GLRO(dl_cache_line_size);
>> #endif
On 11/07/23 5:54 pm, Adhemerval Zanella Netto wrote:
>
> On 10/07/23 18:28, Peter Bergner wrote:
>> On 7/10/23 1:21 PM, bmahi496@linux.ibm.com wrote:
>>> Note that the tunable only handles the features which are really used
>>> in the IFUNC selection. All others are ignored as the values are only
>>> used inside glibc.
>> I don't understand this comment. Your code seems to handle all hwcap
>> features, not just the features used within glibc. That's fine with me,
>> because I'd like these changes to eventually be exported to the TCB and
>> then we should handle all known features. Maybe the comment just needs
>> updating?
>>
>>
>>
>>
>>> + /* Copy the features from dl_powerpc_cpu_features, which contains the
>>> + features provided by AT_HWCAP and AT_HWCAP2. */
>>> + struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
>> ^
>> Space needed before the '('.
> For variable-like access, such as with macro for GL or GLRO, there is no
> need for the space.
okay.
>
>>
>>
>>> + tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
>> Ditto for both '('s and before and after the '/'.
> User array_length macro instead.
Thanks. I will use array_length macro.
On 11/07/23 13:03, MAHESH BODAPATI wrote:
>
> On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>> On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
>>> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>>>
>>> This patch enables the option to influence hwcaps used by PowerPC.
>>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>>> and zzz, where the feature name is case-sensitive and has to match the ones
>>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>>
>>> Note that the tunable only handles the features which are really used
>>> in the IFUNC selection. All others are ignored as the values are only
>>> used inside glibc.
>> It is still missing a regression test to check if tunable work as intended.
>
>
> I ran glibc tests with and without setting hwcap tunables and no regressions were found.
> Summary of test results:
> 4458 PASS
> 9 UNSUPPORTED
> 16 XFAIL
> 2 XPASS
>
> The runtime behavior is as expected when we set hwcap tunables.
> We tested on power10,power9 BE and powerpc32 machines and the results were good.
> Could you go through the attached sheet ,we have listed runtime behavior for some of the tunable options.
> We are not sure on how to add regressions tests for tunable feature.
> I didn't see hwcap tunable specific tests added for other targets as well.
There are only tests for x86, so running the tests won't really exercise this
code path for powerpc:
$ git grep glibc.cpu.hwcaps
manual/tunables.texi:glibc.cpu.hwcaps:
manual/tunables.texi:@deftp Tunable glibc.cpu.hwcaps
manual/tunables.texi:The @code{glibc.cpu.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to
sysdeps/s390/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,zzz,....
sysdeps/x86/Makefile:tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
sysdeps/x86/Makefile:tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/cpu-tunables.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz,....
sysdeps/x86/dl-cet.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/include/cpu-features.h: GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC
sysdeps/x86/tst-cet-legacy-9.c: -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK. */
sysdeps/x86_64/Makefile: GLIBC_TUNABLES=glibc.cpu.hwcaps=-AVX512F,-AVX2
Since testing depending of the auxv returned values from kernel and there is no
easy way to override it, one possibility is to get the current hwcap/hwcap2,
and spawn a new process with GLIBC_TUNABLES masking some bits, and check
the result hwcap/hwcap2 to see if it matches the masked value. You may
add multiple tests and mark them UNSUPPORTED if the hwcap does not contain
the expected bits (for instance ISA 2.07 or ISA 3.0).
On 11/07/23 13:03, MAHESH BODAPATI wrote:
>
> On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>> On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
>>> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>>>
>>> This patch enables the option to influence hwcaps used by PowerPC.
>>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>>> and zzz, where the feature name is case-sensitive and has to match the ones
>>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>>
>>> Note that the tunable only handles the features which are really used
>>> in the IFUNC selection. All others are ignored as the values are only
>>> used inside glibc.
>> It is still missing a regression test to check if tunable work as intended.
>
>
> I ran glibc tests with and without setting hwcap tunables and no regressions were found.
> Summary of test results:
> 4458 PASS
> 9 UNSUPPORTED
> 16 XFAIL
> 2 XPASS
>
> The runtime behavior is as expected when we set hwcap tunables.
> We tested on power10,power9 BE and powerpc32 machines and the results were good.
> Could you go through the attached sheet ,we have listed runtime behavior for some of the tunable options.
> We are not sure on how to add regressions tests for tunable feature.
> I didn't see hwcap tunable specific tests added for other targets as well.
There are only tests for x86, so running the tests won't really exercise this
code path for powerpc (and that's why you haven't seem a OOB access in a previous
version):
$ git grep glibc.cpu.hwcaps
manual/tunables.texi:glibc.cpu.hwcaps:
manual/tunables.texi:@deftp Tunable glibc.cpu.hwcaps
manual/tunables.texi:The @code{glibc.cpu.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to
sysdeps/s390/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,zzz,....
sysdeps/x86/Makefile:tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
sysdeps/x86/Makefile:tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/Makefile:tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/cpu-tunables.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz,....
sysdeps/x86/dl-cet.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
sysdeps/x86/include/cpu-features.h: GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC
sysdeps/x86/tst-cet-legacy-9.c: -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK. */
sysdeps/x86_64/Makefile: GLIBC_TUNABLES=glibc.cpu.hwcaps=-AVX512F,-AVX2
Since testing depending of the auxv returned values from kernel and there is no
easy way to override it, one possibility is to get the current hwcap/hwcap2,
and spawn a new process with GLIBC_TUNABLES masking some bits, and check
the result hwcap/hwcap2 to see if it matches the masked value. You may
add multiple tests and mark them UNSUPPORTED if the hwcap does not contain
the expected bits (for instance ISA 2.07 or ISA 3.0).
On 11/07/23 9:53 pm, Adhemerval Zanella Netto wrote:
>
> On 11/07/23 13:03, MAHESH BODAPATI wrote:
>> On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>>> On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
>>>> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>>>>
>>>> This patch enables the option to influence hwcaps used by PowerPC.
>>>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>>>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>>>> and zzz, where the feature name is case-sensitive and has to match the ones
>>>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>>>
>>>> Note that the tunable only handles the features which are really used
>>>> in the IFUNC selection. All others are ignored as the values are only
>>>> used inside glibc.
>>> It is still missing a regression test to check if tunable work as intended.
>>
>> I ran glibc tests with and without setting hwcap tunables and no regressions were found.
>> Summary of test results:
>> 4458 PASS
>> 9 UNSUPPORTED
>> 16 XFAIL
>> 2 XPASS
>>
>> The runtime behavior is as expected when we set hwcap tunables.
>> We tested on power10,power9 BE and powerpc32 machines and the results were good.
>> Could you go through the attached sheet ,we have listed runtime behavior for some of the tunable options.
>> We are not sure on how to add regressions tests for tunable feature.
>> I didn't see hwcap tunable specific tests added for other targets as well.
> There are only tests for x86, so running the tests won't really exercise this
> code path for powerpc (and that's why you haven't seem a OOB access in a previous
> version):
>
> $ git grep glibc.cpu.hwcaps
> manual/tunables.texi:glibc.cpu.hwcaps:
> manual/tunables.texi:@deftp Tunable glibc.cpu.hwcaps
> manual/tunables.texi:The @code{glibc.cpu.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to
> sysdeps/s390/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,zzz,....
> sysdeps/x86/Makefile:tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> sysdeps/x86/Makefile:tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/Makefile:tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/Makefile:tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/Makefile:tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/cpu-tunables.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz,....
> sysdeps/x86/dl-cet.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> sysdeps/x86/include/cpu-features.h: GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC
> sysdeps/x86/tst-cet-legacy-9.c: -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK. */
> sysdeps/x86_64/Makefile: GLIBC_TUNABLES=glibc.cpu.hwcaps=-AVX512F,-AVX2
>
> Since testing depending of the auxv returned values from kernel and there is no
> easy way to override it, one possibility is to get the current hwcap/hwcap2,
> and spawn a new process with GLIBC_TUNABLES masking some bits, and check
> the result hwcap/hwcap2 to see if it matches the masked value. You may
> add multiple tests and mark them UNSUPPORTED if the hwcap does not contain
> the expected bits (for instance ISA 2.07 or ISA 3.0).
We didn't update the AT_HWCAP1/2 auxv. we have updated the values of
cpu_features structure based on the
tunable set's. the hwcap1/2 values of struct cpu_features has been used
for ifunc selection.
so we didn't change the values of auxv as similar to S390.
On 12/07/23 08:04, MAHESH BODAPATI wrote:
>
> On 11/07/23 9:53 pm, Adhemerval Zanella Netto wrote:
>>
>> On 11/07/23 13:03, MAHESH BODAPATI wrote:
>>> On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>>>> On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
>>>>> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>>>>>
>>>>> This patch enables the option to influence hwcaps used by PowerPC.
>>>>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>>>>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>>>>> and zzz, where the feature name is case-sensitive and has to match the ones
>>>>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>>>>
>>>>> Note that the tunable only handles the features which are really used
>>>>> in the IFUNC selection. All others are ignored as the values are only
>>>>> used inside glibc.
>>>> It is still missing a regression test to check if tunable work as intended.
>>>
>>> I ran glibc tests with and without setting hwcap tunables and no regressions were found.
>>> Summary of test results:
>>> 4458 PASS
>>> 9 UNSUPPORTED
>>> 16 XFAIL
>>> 2 XPASS
>>>
>>> The runtime behavior is as expected when we set hwcap tunables.
>>> We tested on power10,power9 BE and powerpc32 machines and the results were good.
>>> Could you go through the attached sheet ,we have listed runtime behavior for some of the tunable options.
>>> We are not sure on how to add regressions tests for tunable feature.
>>> I didn't see hwcap tunable specific tests added for other targets as well.
>> There are only tests for x86, so running the tests won't really exercise this
>> code path for powerpc (and that's why you haven't seem a OOB access in a previous
>> version):
>>
>> $ git grep glibc.cpu.hwcaps
>> manual/tunables.texi:glibc.cpu.hwcaps:
>> manual/tunables.texi:@deftp Tunable glibc.cpu.hwcaps
>> manual/tunables.texi:The @code{glibc.cpu.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to
>> sysdeps/s390/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,zzz,....
>> sysdeps/x86/Makefile:tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
>> sysdeps/x86/Makefile:tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/Makefile:tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/Makefile:tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/Makefile:tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/cpu-tunables.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz,....
>> sysdeps/x86/dl-cet.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>> sysdeps/x86/include/cpu-features.h: GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC
>> sysdeps/x86/tst-cet-legacy-9.c: -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK. */
>> sysdeps/x86_64/Makefile: GLIBC_TUNABLES=glibc.cpu.hwcaps=-AVX512F,-AVX2
>>
>> Since testing depending of the auxv returned values from kernel and there is no
>> easy way to override it, one possibility is to get the current hwcap/hwcap2,
>> and spawn a new process with GLIBC_TUNABLES masking some bits, and check
>> the result hwcap/hwcap2 to see if it matches the masked value. You may
>> add multiple tests and mark them UNSUPPORTED if the hwcap does not contain
>> the expected bits (for instance ISA 2.07 or ISA 3.0).
>
>
> We didn't update the AT_HWCAP1/2 auxv. we have updated the values of cpu_features structure based on the
> tunable set's. the hwcap1/2 values of struct cpu_features has been used for ifunc selection.
> so we didn't change the values of auxv as similar to S390.
You can still check the hwcap selection with __libc_ifunc_impl_list, which retrieve
the ifunc selection based on the cpu_features. The test below is for memcpy, but
you can easily extend to other functions. The __libc_ifunc_impl_list sets more
ISA bits depending the current ISA to enable testing all the ifunc if the processor
supports, so the test requires to disable more bits.
diff --git a/sysdeps/unix/sysv/linux/powerpc/Makefile b/sysdeps/unix/sysv/linux/powerpc/Makefile
index 93783cae00..24827efe79 100644
--- a/sysdeps/unix/sysv/linux/powerpc/Makefile
+++ b/sysdeps/unix/sysv/linux/powerpc/Makefile
@@ -23,9 +23,14 @@ ifeq ($(subdir),misc)
sysdep_headers += bits/ppc.h
sysdep_routines += get_timebase_freq
tests-static += test-gettimebasefreq-static
-tests += $(tests-static)
-tests += test-gettimebasefreq
-tests += test-powerpc-linux-sysconf
+tests += \
+ $(tests-static) \
+ test-gettimebasefreq \
+ test-powerpc-linux-sysconf \
+ tst-hwcap-tunables \
+ # tests
+
+tst-hwcap-tunables-ARGS = -- $(host-test-program-cmd)
endif
ifeq ($(subdir),csu)
diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
new file mode 100644
index 0000000000..6c14d717ea
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
@@ -0,0 +1,116 @@
+/* Tests for powerpc GLIBC_TUNABLES=glibc.cpu.hwcaps filter.
+ Copyright (C) 2023 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 <array_length.h>
+#include <getopt.h>
+#include <ifunc-impl-list.h>
+#include <spawn.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <sys/auxv.h>
+#include <sys/wait.h>
+
+/* Nonzero if the program gets called via `exec'. */
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+static int restart;
+
+/* Hold the four initial argument used to respawn the process, plus the extra
+ '--direct', '--restart', and the function to check */
+static char *spargs[8];
+static int fc;
+
+/* Called on process re-execution. */
+_Noreturn static void
+handle_restart (int argc, char *argv[])
+{
+ TEST_VERIFY_EXIT (argc == 1);
+ const char *funcname = argv[0];
+
+ struct libc_ifunc_impl impls[32];
+ int cnt = __libc_ifunc_impl_list ("memcpy", impls, array_length (impls));
+ TEST_VERIFY_EXIT (cnt >= 1);
+ for (int i = 0; i < cnt; i++) {
+ if (strcmp (impls[i].name, funcname) == 0)
+ {
+ TEST_COMPARE (impls[i].usable, false);
+ break;
+ }
+ }
+
+ _exit (EXIT_SUCCESS);
+}
+
+static void
+run_test (const char *filter, const char *funcname)
+{
+ printf ("info: checking filter %s (expect %s ifunc selection to be removed)\n",
+ filter, funcname);
+ char *tunable = xasprintf ("GLIBC_TUNABLES=glibc.cpu.hwcaps=%s", filter);
+ char *const newenvs[] = { (char*) tunable, NULL };
+ spargs[fc] = (char *) funcname;
+
+ pid_t pid;
+ TEST_COMPARE (posix_spawn (&pid, spargs[0], NULL, NULL, spargs, newenvs), 0);
+ int status;
+ TEST_COMPARE (xwaitpid (pid, &status, 0), pid);
+ TEST_VERIFY (WIFEXITED (status));
+ TEST_VERIFY (!WIFSIGNALED (status));
+ TEST_COMPARE (WEXITSTATUS (status), 0);
+
+ free (tunable);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+ if (restart)
+ handle_restart (argc - 1, &argv[1]);
+
+ TEST_VERIFY_EXIT (argc == 2 || argc == 5);
+
+ int i;
+ for (i = 0; i < argc - 1; i++)
+ spargs[i] = argv[i + 1];
+ spargs[i++] = (char *) "--direct";
+ spargs[i++] = (char *) "--restart";
+ fc = i++;
+ spargs[i] = NULL;
+
+ unsigned long int hwcap = getauxval (AT_HWCAP);
+ unsigned long int hwcap2 = getauxval (AT_HWCAP2);
+
+ if (hwcap2 & PPC_FEATURE2_ARCH_3_1)
+ run_test ("-arch_3_1", "__memcpy_power10");
+ if (hwcap2 & PPC_FEATURE2_ARCH_2_07)
+ run_test ("-arch_2_07", "__memcpy_power8_cached");
+ if (hwcap & PPC_FEATURE_ARCH_2_06)
+ run_test ("-arch_2_06", "__memcpy_power7");
+ if (hwcap & PPC_FEATURE_ARCH_2_05)
+ run_test ("-arch_2_06,-arch_2_05","__memcpy_power6");
+ run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4", "__memcpy_power4");
+
+ return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
On 12/07/23 7:37 pm, Adhemerval Zanella Netto wrote:
>
> On 12/07/23 08:04, MAHESH BODAPATI wrote:
>> On 11/07/23 9:53 pm, Adhemerval Zanella Netto wrote:
>>> On 11/07/23 13:03, MAHESH BODAPATI wrote:
>>>> On 11/07/23 7:15 pm, Adhemerval Zanella Netto wrote:
>>>>> On 10/07/23 15:21, bmahi496@linux.ibm.com wrote:
>>>>>> From: Mahesh Bodapati <mahesh.bodapati@ibm.com>
>>>>>>
>>>>>> This patch enables the option to influence hwcaps used by PowerPC.
>>>>>> The environment variable, GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz....,
>>>>>> can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature xxx
>>>>>> and zzz, where the feature name is case-sensitive and has to match the ones
>>>>>> mentioned in the file{sysdeps/powerpc/dl-procinfo.c}.
>>>>>>
>>>>>> Note that the tunable only handles the features which are really used
>>>>>> in the IFUNC selection. All others are ignored as the values are only
>>>>>> used inside glibc.
>>>>> It is still missing a regression test to check if tunable work as intended.
>>>> I ran glibc tests with and without setting hwcap tunables and no regressions were found.
>>>> Summary of test results:
>>>> 4458 PASS
>>>> 9 UNSUPPORTED
>>>> 16 XFAIL
>>>> 2 XPASS
>>>>
>>>> The runtime behavior is as expected when we set hwcap tunables.
>>>> We tested on power10,power9 BE and powerpc32 machines and the results were good.
>>>> Could you go through the attached sheet ,we have listed runtime behavior for some of the tunable options.
>>>> We are not sure on how to add regressions tests for tunable feature.
>>>> I didn't see hwcap tunable specific tests added for other targets as well.
>>> There are only tests for x86, so running the tests won't really exercise this
>>> code path for powerpc (and that's why you haven't seem a OOB access in a previous
>>> version):
>>>
>>> $ git grep glibc.cpu.hwcaps
>>> manual/tunables.texi:glibc.cpu.hwcaps:
>>> manual/tunables.texi:@deftp Tunable glibc.cpu.hwcaps
>>> manual/tunables.texi:The @code{glibc.cpu.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to
>>> sysdeps/s390/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,zzz,....
>>> sysdeps/x86/Makefile:tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
>>> sysdeps/x86/Makefile:tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/Makefile:tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/Makefile:tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/Makefile:tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/cpu-features.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/cpu-tunables.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,-zzz,....
>>> sysdeps/x86/dl-cet.c: GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>>> sysdeps/x86/include/cpu-features.h: GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC
>>> sysdeps/x86/tst-cet-legacy-9.c: -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK. */
>>> sysdeps/x86_64/Makefile: GLIBC_TUNABLES=glibc.cpu.hwcaps=-AVX512F,-AVX2
>>>
>>> Since testing depending of the auxv returned values from kernel and there is no
>>> easy way to override it, one possibility is to get the current hwcap/hwcap2,
>>> and spawn a new process with GLIBC_TUNABLES masking some bits, and check
>>> the result hwcap/hwcap2 to see if it matches the masked value. You may
>>> add multiple tests and mark them UNSUPPORTED if the hwcap does not contain
>>> the expected bits (for instance ISA 2.07 or ISA 3.0).
>>
>> We didn't update the AT_HWCAP1/2 auxv. we have updated the values of cpu_features structure based on the
>> tunable set's. the hwcap1/2 values of struct cpu_features has been used for ifunc selection.
>> so we didn't change the values of auxv as similar to S390.
> You can still check the hwcap selection with __libc_ifunc_impl_list, which retrieve
> the ifunc selection based on the cpu_features. The test below is for memcpy, but
> you can easily extend to other functions. The __libc_ifunc_impl_list sets more
> ISA bits depending the current ISA to enable testing all the ifunc if the processor
> supports, so the test requires to disable more bits.
Thanks Adhemerval.
I enabled testing for powerpc32 in the same memcpy test and also did the
corrections from PATCH_v6 review comments. I will share the updated patch.
>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/Makefile b/sysdeps/unix/sysv/linux/powerpc/Makefile
> index 93783cae00..24827efe79 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/Makefile
> +++ b/sysdeps/unix/sysv/linux/powerpc/Makefile
> @@ -23,9 +23,14 @@ ifeq ($(subdir),misc)
> sysdep_headers += bits/ppc.h
> sysdep_routines += get_timebase_freq
> tests-static += test-gettimebasefreq-static
> -tests += $(tests-static)
> -tests += test-gettimebasefreq
> -tests += test-powerpc-linux-sysconf
> +tests += \
> + $(tests-static) \
> + test-gettimebasefreq \
> + test-powerpc-linux-sysconf \
> + tst-hwcap-tunables \
> + # tests
> +
> +tst-hwcap-tunables-ARGS = -- $(host-test-program-cmd)
> endif
>
> ifeq ($(subdir),csu)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> new file mode 100644
> index 0000000000..6c14d717ea
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> @@ -0,0 +1,116 @@
> +/* Tests for powerpc GLIBC_TUNABLES=glibc.cpu.hwcaps filter.
> + Copyright (C) 2023 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 <array_length.h>
> +#include <getopt.h>
> +#include <ifunc-impl-list.h>
> +#include <spawn.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <sys/auxv.h>
> +#include <sys/wait.h>
> +
> +/* Nonzero if the program gets called via `exec'. */
> +#define CMDLINE_OPTIONS \
> + { "restart", no_argument, &restart, 1 },
> +static int restart;
> +
> +/* Hold the four initial argument used to respawn the process, plus the extra
> + '--direct', '--restart', and the function to check */
> +static char *spargs[8];
> +static int fc;
> +
> +/* Called on process re-execution. */
> +_Noreturn static void
> +handle_restart (int argc, char *argv[])
> +{
> + TEST_VERIFY_EXIT (argc == 1);
> + const char *funcname = argv[0];
> +
> + struct libc_ifunc_impl impls[32];
> + int cnt = __libc_ifunc_impl_list ("memcpy", impls, array_length (impls));
> + TEST_VERIFY_EXIT (cnt >= 1);
> + for (int i = 0; i < cnt; i++) {
> + if (strcmp (impls[i].name, funcname) == 0)
> + {
> + TEST_COMPARE (impls[i].usable, false);
> + break;
> + }
> + }
> +
> + _exit (EXIT_SUCCESS);
> +}
> +
> +static void
> +run_test (const char *filter, const char *funcname)
> +{
> + printf ("info: checking filter %s (expect %s ifunc selection to be removed)\n",
> + filter, funcname);
> + char *tunable = xasprintf ("GLIBC_TUNABLES=glibc.cpu.hwcaps=%s", filter);
> + char *const newenvs[] = { (char*) tunable, NULL };
> + spargs[fc] = (char *) funcname;
> +
> + pid_t pid;
> + TEST_COMPARE (posix_spawn (&pid, spargs[0], NULL, NULL, spargs, newenvs), 0);
> + int status;
> + TEST_COMPARE (xwaitpid (pid, &status, 0), pid);
> + TEST_VERIFY (WIFEXITED (status));
> + TEST_VERIFY (!WIFSIGNALED (status));
> + TEST_COMPARE (WEXITSTATUS (status), 0);
> +
> + free (tunable);
> +}
> +
> +static int
> +do_test (int argc, char *argv[])
> +{
> + if (restart)
> + handle_restart (argc - 1, &argv[1]);
> +
> + TEST_VERIFY_EXIT (argc == 2 || argc == 5);
> +
> + int i;
> + for (i = 0; i < argc - 1; i++)
> + spargs[i] = argv[i + 1];
> + spargs[i++] = (char *) "--direct";
> + spargs[i++] = (char *) "--restart";
> + fc = i++;
> + spargs[i] = NULL;
> +
> + unsigned long int hwcap = getauxval (AT_HWCAP);
> + unsigned long int hwcap2 = getauxval (AT_HWCAP2);
> +
> + if (hwcap2 & PPC_FEATURE2_ARCH_3_1)
> + run_test ("-arch_3_1", "__memcpy_power10");
> + if (hwcap2 & PPC_FEATURE2_ARCH_2_07)
> + run_test ("-arch_2_07", "__memcpy_power8_cached");
> + if (hwcap & PPC_FEATURE_ARCH_2_06)
> + run_test ("-arch_2_06", "__memcpy_power7");
> + if (hwcap & PPC_FEATURE_ARCH_2_05)
> + run_test ("-arch_2_06,-arch_2_05","__memcpy_power6");
> + run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4", "__memcpy_power4");
> +
> + return 0;
> +}
> +
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
@@ -513,7 +513,10 @@ On s390x, the supported HWCAP and STFLE features can be found in
@code{sysdeps/s390/cpu-features.c}. In addition the user can also set
a CPU arch-level like @code{z13} instead of single HWCAP and STFLE features.
-This tunable is specific to i386, x86-64 and s390x.
+On powerpc, the supported HWCAP and HWCAP2 features can be found in
+@code{sysdeps/powerpc/dl-procinfo.c}.
+
+This tunable is specific to i386, x86-64, s390x and powerpc.
@end deftp
@deftp Tunable glibc.cpu.cached_memopt
@@ -19,14 +19,109 @@
#include <stdint.h>
#include <cpu-features.h>
#include <elf/dl-tunables.h>
+#include <unistd.h>
+#include <string.h>
+#define MEMCMP_DEFAULT memcmp
+#define STRLEN_DEFAULT strlen
+
+static void
+TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
+{
+ /* The current IFUNC selection is always using the most recent
+ features which are available via AT_HWCAP or AT_HWCAP2. But in
+ some scenarios it is useful to adjust this selection.
+
+ The environment variable:
+
+ GLIBC_TUNABLES=glibc.cpu.hwcaps=-xxx,yyy,....
+
+ Can be used to enable HWCAP/HWCAP2 feature yyy, disable HWCAP/HWCAP2
+ feature xxx, where the feature name is case-sensitive and has to match
+ the ones mentioned in the file{sysdeps/powerpc/dl-procinfo.c}. */
+
+ /* Copy the features from dl_powerpc_cpu_features, which contains the
+ features provided by AT_HWCAP and AT_HWCAP2. */
+ struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
+ unsigned long int tcbv_hwcap = cpu_features->hwcap;
+ unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
+ unsigned int tun_count;
+ const char *token = valp->strval;
+ tun_count = sizeof(hwcap_tunables)/sizeof(hwcap_tunables[0]);
+ do
+ {
+ const char *token_end, *feature;
+ bool disable;
+ unsigned short offset=0;
+ size_t token_len, i, feature_len;
+ /* Find token separator or end of string. */
+ for (token_end = token; *token_end != ','; token_end++)
+ if (*token_end == '\0')
+ break;
+
+ /* Determine feature. */
+ token_len = token_end - token;
+ if (*token == '-')
+ {
+ disable = true;
+ feature = token + 1;
+ feature_len = token_len - 1;
+ }
+ else
+ {
+ disable = false;
+ feature = token;
+ feature_len = token_len;
+ }
+ for (i = 0; i < tun_count; ++i)
+ {
+ const char *hwcap_name = hwcap_names + offset;
+ /* Check the tunable name on the supported list. */
+ if (STRLEN_DEFAULT (hwcap_name) == feature_len
+ && MEMCMP_DEFAULT (feature, hwcap_name, feature_len)
+ == 0)
+ {
+ /* Update the hwcap and hwcap2 bits. */
+ if (disable)
+ {
+ /* Id is 1 for hwcap2 tunable. */
+ if (hwcap_tunables[i].id)
+ cpu_features->hwcap2 &= ~(hwcap_tunables[i].mask);
+ else
+ cpu_features->hwcap &= ~(hwcap_tunables[i].mask);
+ }
+ else
+ {
+ /* Enable the features and also checking that no unsupported
+ features were enabled by user. */
+ if (hwcap_tunables[i].id)
+ cpu_features->hwcap2 |= (tcbv_hwcap2 & hwcap_tunables[i].mask);
+ else
+ cpu_features->hwcap |= (tcbv_hwcap & hwcap_tunables[i].mask);
+ }
+ break;
+ }
+ offset = offset + STRLEN_DEFAULT (hwcap_name) + 1;
+ }
+ token += token_len;
+ /* ... and skip token separator for next round. */
+ if (*token == ',') token++;
+ }
+ while (*token != '\0');
+}
static inline void
-init_cpu_features (struct cpu_features *cpu_features)
+init_cpu_features (struct cpu_features *cpu_features, uint64_t hwcaps[])
{
+ /* Fill the cpu_features with the supported hwcaps
+ which are set by __tcb_parse_hwcap_and_convert_at_platform. */
+ cpu_features->hwcap = hwcaps[0];
+ cpu_features->hwcap2 = hwcaps[1];
/* Default is to use aligned memory access on optimized function unless
tunables is enable, since for this case user can explicit disable
unaligned optimizations. */
int32_t cached_memfunc = TUNABLE_GET (glibc, cpu, cached_memopt, int32_t,
NULL);
cpu_features->use_cached_memopt = (cached_memfunc > 0);
+ TUNABLE_GET (glibc, cpu, hwcaps, tunable_val_t *,
+ TUNABLE_CALLBACK (set_hwcaps));
}
@@ -19,10 +19,112 @@
# define __CPU_FEATURES_POWERPC_H
#include <stdbool.h>
+#include <sys/auxv.h>
struct cpu_features
{
bool use_cached_memopt;
+ unsigned long int hwcap;
+ unsigned long int hwcap2;
+};
+
+static const char hwcap_names[] = {
+ "4xxmac\0"
+ "altivec\0"
+ "arch_2_05\0"
+ "arch_2_06\0"
+ "archpmu\0"
+ "booke\0"
+ "cellbe\0"
+ "dfp\0"
+ "efpdouble\0"
+ "efpsingle\0"
+ "fpu\0"
+ "ic_snoop\0"
+ "mmu\0"
+ "notb\0"
+ "pa6t\0"
+ "power4\0"
+ "power5\0"
+ "power5+\0"
+ "power6x\0"
+ "ppc32\0"
+ "ppc601\0"
+ "ppc64\0"
+ "ppcle\0"
+ "smt\0"
+ "spe\0"
+ "true_le\0"
+ "ucache\0"
+ "vsx\0"
+ "arch_2_07\0"
+ "dscr\0"
+ "ebb\0"
+ "htm\0"
+ "htm-nosc\0"
+ "htm-no-suspend\0"
+ "isel\0"
+ "tar\0"
+ "vcrypto\0"
+ "arch_3_00\0"
+ "ieee128\0"
+ "darn\0"
+ "scv\0"
+ "arch_3_1\0"
+ "mma\0"
+};
+
+static const struct
+{
+ unsigned int mask;
+ bool id;
+} hwcap_tunables[] = {
+ /* AT_HWCAP tunable masks. */
+ { PPC_FEATURE_HAS_4xxMAC, 0 },
+ { PPC_FEATURE_HAS_ALTIVEC, 0 },
+ { PPC_FEATURE_ARCH_2_05, 0 },
+ { PPC_FEATURE_ARCH_2_06, 0 },
+ { PPC_FEATURE_PSERIES_PERFMON_COMPAT, 0 },
+ { PPC_FEATURE_BOOKE, 0 },
+ { PPC_FEATURE_CELL_BE, 0 },
+ { PPC_FEATURE_HAS_DFP, 0 },
+ { PPC_FEATURE_HAS_EFP_DOUBLE, 0 },
+ { PPC_FEATURE_HAS_EFP_SINGLE, 0 },
+ { PPC_FEATURE_HAS_FPU, 0 },
+ { PPC_FEATURE_ICACHE_SNOOP, 0 },
+ { PPC_FEATURE_HAS_MMU, 0 },
+ { PPC_FEATURE_NO_TB, 0 },
+ { PPC_FEATURE_PA6T, 0 },
+ { PPC_FEATURE_POWER4, 0 },
+ { PPC_FEATURE_POWER5, 0 },
+ { PPC_FEATURE_POWER5_PLUS, 0 },
+ { PPC_FEATURE_POWER6_EXT, 0 },
+ { PPC_FEATURE_32, 0 },
+ { PPC_FEATURE_601_INSTR, 0 },
+ { PPC_FEATURE_64, 0 },
+ { PPC_FEATURE_PPC_LE, 0 },
+ { PPC_FEATURE_SMT, 0 },
+ { PPC_FEATURE_HAS_SPE, 0 },
+ { PPC_FEATURE_TRUE_LE, 0 },
+ { PPC_FEATURE_UNIFIED_CACHE, 0 },
+ { PPC_FEATURE_HAS_VSX, 0 },
+
+ /* AT_HWCAP2 tunable masks. */
+ { PPC_FEATURE2_ARCH_2_07, 1 },
+ { PPC_FEATURE2_HAS_DSCR, 1 },
+ { PPC_FEATURE2_HAS_EBB, 1 },
+ { PPC_FEATURE2_HAS_HTM, 1 },
+ { PPC_FEATURE2_HTM_NOSC, 1 },
+ { PPC_FEATURE2_HTM_NO_SUSPEND, 1 },
+ { PPC_FEATURE2_HAS_ISEL, 1 },
+ { PPC_FEATURE2_HAS_TAR, 1 },
+ { PPC_FEATURE2_HAS_VEC_CRYPTO, 1 },
+ { PPC_FEATURE2_ARCH_3_00, 1 },
+ { PPC_FEATURE2_HAS_IEEE128, 1 },
+ { PPC_FEATURE2_DARN, 1 },
+ { PPC_FEATURE2_SCV, 1 },
+ { PPC_FEATURE2_ARCH_3_1, 1 },
+ { PPC_FEATURE2_MMA, 1 },
};
#endif /* __CPU_FEATURES_H */
@@ -24,5 +24,8 @@ glibc {
maxval: 1
default: 0
}
+ hwcaps {
+ type: STRING
+ }
}
}
@@ -19,6 +19,7 @@
#include <unistd.h>
#include <shlib-compat.h>
#include <dl-procinfo.h>
+#include <cpu-features.c>
tcbhead_t __tcb __attribute__ ((visibility ("hidden")));
@@ -63,6 +64,9 @@ __tcb_parse_hwcap_and_convert_at_platform (void)
else if (h1 & PPC_FEATURE_POWER5)
h1 |= PPC_FEATURE_POWER4;
+ uint64_t array_hwcaps[] = { h1, h2 };
+ init_cpu_features (&GLRO(dl_powerpc_cpu_features), array_hwcaps);
+
/* Consolidate both HWCAP and HWCAP2 into a single doubleword so that
we can read both in a single load later. */
__tcb.hwcap = (h1 << 32) | (h2 & 0xffffffff);
@@ -21,6 +21,7 @@
#include <wchar.h>
#include <ldsodefs.h>
#include <ifunc-impl-list.h>
+#include <cpu-features.h>
size_t
__libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
@@ -28,7 +29,8 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
{
size_t i = max;
- unsigned long int hwcap = GLRO(dl_hwcap);
+ const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
+ unsigned long int hwcap = features->hwcap;
/* hwcap contains only the latest supported ISA, the code checks which is
and fills the previous supported ones. */
if (hwcap & PPC_FEATURE_ARCH_2_06)
@@ -16,6 +16,7 @@
<https://www.gnu.org/licenses/>. */
#include <ldsodefs.h>
+#include <cpu-features.h>
/* The code checks if _rtld_global_ro was realocated before trying to access
the dl_hwcap field. The assembly is to make the compiler not optimize the
@@ -32,11 +33,12 @@
# define __GLRO(value) GLRO(value)
#endif
-/* dl_hwcap contains only the latest supported ISA, the macro checks which is
- and fills the previous ones. */
+/* Get the hardware information post the tunables set , the macro checks
+ it and fills the previous ones. */
#define INIT_ARCH() \
- unsigned long int hwcap = __GLRO(dl_hwcap); \
- unsigned long int __attribute__((unused)) hwcap2 = __GLRO(dl_hwcap2); \
+ const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features); \
+ unsigned long int hwcap = features->hwcap; \
+ unsigned long int __attribute__((unused)) hwcap2 = features->hwcap2; \
bool __attribute__((unused)) use_cached_memopt = \
__GLRO(dl_powerpc_cpu_features.use_cached_memopt); \
if (hwcap & PPC_FEATURE_ARCH_2_06) \
@@ -27,7 +27,6 @@
#include <dl-tls.h>
#include <sysdep.h>
#include <hwcapinfo.h>
-#include <cpu-features.c>
#include <dl-static-tls.h>
#include <dl-funcdesc.h>
#include <dl-machine-rel.h>
@@ -297,7 +296,6 @@ static inline void __attribute__ ((unused))
dl_platform_init (void)
{
__tcb_parse_hwcap_and_convert_at_platform ();
- init_cpu_features (&GLRO(dl_powerpc_cpu_features));
}
#endif
@@ -17,6 +17,7 @@
<https://www.gnu.org/licenses/>. */
#include <assert.h>
+#include <cpu-features.h>
#include <string.h>
#include <wchar.h>
#include <ldsodefs.h>
@@ -27,9 +28,9 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
size_t max)
{
size_t i = max;
-
- unsigned long int hwcap = GLRO(dl_hwcap);
- unsigned long int hwcap2 = GLRO(dl_hwcap2);
+ const struct cpu_features *features = &GLRO(dl_powerpc_cpu_features);
+ unsigned long int hwcap = features->hwcap;
+ unsigned long int hwcap2 = features->hwcap2;
#ifdef SHARED
int cacheline_size = GLRO(dl_cache_line_size);
#endif