* [PATCH] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
@ 2021-06-23 14:54 Adhemerval Zanella
2021-06-23 20:41 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-23 14:54 UTC (permalink / raw)
To: libc-alpha, H . J . Lu
AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
The SHSTK is also a opt-in feature, Ryzen 9 does support but the
kernel might not enable it.
The SSDB is also defined and implemented different on AMD [2],
and also a new AMD_SSDB flag is added. It should map to the
cpuinfo 'ssdb' on recent AMD cpus.
It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
on recent AMD cpus.
Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
[1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
[2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
---
sysdeps/x86/bits/platform/x86.h | 4 +++
sysdeps/x86/include/cpu-features.h | 12 +++++++++
sysdeps/x86/tst-cpu-features-cpuinfo.c | 35 ++++++++++++++++++++------
3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
index fe08d8a1b6..26e3b67ede 100644
--- a/sysdeps/x86/bits/platform/x86.h
+++ b/sysdeps/x86/bits/platform/x86.h
@@ -278,6 +278,10 @@ enum
+ cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
+ x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
+ x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
+ x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
+ x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
x86_cpu_index_7_ecx_1_eax
= (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
index d042a2ebef..4f1c4ee402 100644
--- a/sysdeps/x86/include/cpu-features.h
+++ b/sysdeps/x86/include/cpu-features.h
@@ -289,6 +289,10 @@ enum
/* EBX. */
#define bit_cpu_WBNOINVD (1u << 9)
+#define bit_cpu_AMD_IBPB (1u << 12)
+#define bit_cpu_AMD_IBRS (1u << 14)
+#define bit_cpu_AMD_STIBP (1u << 15)
+#define bit_cpu_AMD_SSBD (1u << 24)
/* CPUID_INDEX_7_ECX_1. */
@@ -519,6 +523,10 @@ enum
/* EBX. */
#define index_cpu_WBNOINVD CPUID_INDEX_80000008
+#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
+#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
+#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
+#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
/* CPUID_INDEX_7_ECX_1. */
@@ -749,6 +757,10 @@ enum
/* EBX. */
#define reg_WBNOINVD ebx
+#define reg_AMD_IBPB ebx
+#define reg_AMD_IBRS ebx
+#define reg_AMD_STIBP ebx
+#define reg_AMD_SSBD ebx
/* CPUID_INDEX_7_ECX_1. */
diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
index 75e7eb9352..6535fe50e8 100644
--- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
+++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
@@ -16,10 +16,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <sys/platform/x86.h>
+#include <cpu-features.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <stdbool.h>
static char *cpu_flags;
@@ -54,7 +55,7 @@ get_cpuinfo (void)
int
check_proc (const char *proc_name, const char *search_name, int flag,
- int usable, const char *name)
+ int usable, const char *name, bool optin)
{
int found = 0;
@@ -78,7 +79,7 @@ check_proc (const char *proc_name, const char *search_name, int flag,
if (found != flag)
{
- if (found || usable)
+ if (!optin && (found || usable))
printf (" *** failure ***\n");
else
{
@@ -93,12 +94,18 @@ check_proc (const char *proc_name, const char *search_name, int flag,
#define CHECK_PROC(str, name) \
check_proc (#str, " "#str" ", HAS_CPU_FEATURE (name), \
CPU_FEATURE_USABLE (name), \
- "HAS_CPU_FEATURE (" #name ")")
+ "HAS_CPU_FEATURE (" #name ")", false)
+
+#define CHECK_PROC_OPTIN(str, name) \
+ check_proc (#str, " "#str" ", HAS_CPU_FEATURE (name), \
+ CPU_FEATURE_USABLE (name), \
+ "HAS_CPU_FEATURE (" #name ")", true)
static int
do_test (int argc, char **argv)
{
int fails = 0;
+ const struct cpu_features *cpu_features = __get_cpu_features ();
get_cpuinfo ();
fails += CHECK_PROC (acpi, ACPI);
@@ -159,7 +166,17 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (hle, HLE);
fails += CHECK_PROC (ht, HTT);
fails += CHECK_PROC (hybrid, HYBRID);
- fails += CHECK_PROC (ibrs, IBRS_IBPB);
+ if (cpu_features->basic.kind == arch_kind_intel)
+ {
+ fails += CHECK_PROC (ibrs, IBRS_IBPB);
+ fails += CHECK_PROC (stibp, STIBP);
+ }
+ else if (cpu_features->basic.kind == arch_kind_amd)
+ {
+ fails += CHECK_PROC (ibpb, AMD_IBPB);
+ fails += CHECK_PROC (ibrs, AMD_IBRS);
+ fails += CHECK_PROC (stibp, AMD_STIBP);
+ }
fails += CHECK_PROC (ibt, IBT);
fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
fails += CHECK_PROC (invpcid, INVPCID);
@@ -216,12 +233,15 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (sgx, SGX);
fails += CHECK_PROC (sgx_lc, SGX_LC);
fails += CHECK_PROC (sha_ni, SHA);
- fails += CHECK_PROC (shstk, SHSTK);
+ fails += CHECK_PROC_OPTIN (shstk, SHSTK);
fails += CHECK_PROC (smap, SMAP);
fails += CHECK_PROC (smep, SMEP);
fails += CHECK_PROC (smx, SMX);
fails += CHECK_PROC (ss, SS);
- fails += CHECK_PROC (ssbd, SSBD);
+ if (cpu_features->basic.kind == arch_kind_intel)
+ fails += CHECK_PROC (ssbd, SSBD);
+ else if (cpu_features->basic.kind == arch_kind_amd)
+ fails += CHECK_PROC (ssbd, AMD_SSBD);
fails += CHECK_PROC (sse, SSE);
fails += CHECK_PROC (sse2, SSE2);
fails += CHECK_PROC (pni, SSE3);
@@ -229,7 +249,6 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (sse4_2, SSE4_2);
fails += CHECK_PROC (sse4a, SSE4A);
fails += CHECK_PROC (ssse3, SSSE3);
- fails += CHECK_PROC (stibp, STIBP);
fails += CHECK_PROC (svm, SVM);
#ifdef __x86_64__
/* NB: SYSCALL_SYSRET is 64-bit only. */
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
2021-06-23 14:54 [PATCH] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873) Adhemerval Zanella
@ 2021-06-23 20:41 ` H.J. Lu
2021-06-23 21:15 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2021-06-23 20:41 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Wed, Jun 23, 2021 at 7:54 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
> x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
>
> The SHSTK is also a opt-in feature, Ryzen 9 does support but the
> kernel might not enable it.
>
> The SSDB is also defined and implemented different on AMD [2],
> and also a new AMD_SSDB flag is added. It should map to the
> cpuinfo 'ssdb' on recent AMD cpus.
>
> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
> on recent AMD cpus.
>
> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
>
> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
> ---
> sysdeps/x86/bits/platform/x86.h | 4 +++
> sysdeps/x86/include/cpu-features.h | 12 +++++++++
> sysdeps/x86/tst-cpu-features-cpuinfo.c | 35 ++++++++++++++++++++------
> 3 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
> index fe08d8a1b6..26e3b67ede 100644
> --- a/sysdeps/x86/bits/platform/x86.h
> +++ b/sysdeps/x86/bits/platform/x86.h
> @@ -278,6 +278,10 @@ enum
> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
>
> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
>
> x86_cpu_index_7_ecx_1_eax
> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
> index d042a2ebef..4f1c4ee402 100644
> --- a/sysdeps/x86/include/cpu-features.h
> +++ b/sysdeps/x86/include/cpu-features.h
> @@ -289,6 +289,10 @@ enum
>
> /* EBX. */
> #define bit_cpu_WBNOINVD (1u << 9)
> +#define bit_cpu_AMD_IBPB (1u << 12)
> +#define bit_cpu_AMD_IBRS (1u << 14)
> +#define bit_cpu_AMD_STIBP (1u << 15)
> +#define bit_cpu_AMD_SSBD (1u << 24)
>
> /* CPUID_INDEX_7_ECX_1. */
>
> @@ -519,6 +523,10 @@ enum
>
> /* EBX. */
> #define index_cpu_WBNOINVD CPUID_INDEX_80000008
> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
>
> /* CPUID_INDEX_7_ECX_1. */
>
> @@ -749,6 +757,10 @@ enum
>
> /* EBX. */
> #define reg_WBNOINVD ebx
> +#define reg_AMD_IBPB ebx
> +#define reg_AMD_IBRS ebx
> +#define reg_AMD_STIBP ebx
> +#define reg_AMD_SSBD ebx
>
> /* CPUID_INDEX_7_ECX_1. */
>
> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> index 75e7eb9352..6535fe50e8 100644
> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> @@ -16,10 +16,11 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <sys/platform/x86.h>
> +#include <cpu-features.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <stdbool.h>
>
> static char *cpu_flags;
>
> @@ -54,7 +55,7 @@ get_cpuinfo (void)
>
> int
> check_proc (const char *proc_name, const char *search_name, int flag,
> - int usable, const char *name)
> + int usable, const char *name, bool optin)
> {
> int found = 0;
>
> @@ -78,7 +79,7 @@ check_proc (const char *proc_name, const char *search_name, int flag,
>
> if (found != flag)
> {
> - if (found || usable)
> + if (!optin && (found || usable))
> printf (" *** failure ***\n");
> else
> {
> @@ -93,12 +94,18 @@ check_proc (const char *proc_name, const char *search_name, int flag,
> #define CHECK_PROC(str, name) \
> check_proc (#str, " "#str" ", HAS_CPU_FEATURE (name), \
> CPU_FEATURE_USABLE (name), \
> - "HAS_CPU_FEATURE (" #name ")")
> + "HAS_CPU_FEATURE (" #name ")", false)
> +
> +#define CHECK_PROC_OPTIN(str, name) \
> + check_proc (#str, " "#str" ", HAS_CPU_FEATURE (name), \
> + CPU_FEATURE_USABLE (name), \
> + "HAS_CPU_FEATURE (" #name ")", true)
>
> static int
> do_test (int argc, char **argv)
> {
> int fails = 0;
> + const struct cpu_features *cpu_features = __get_cpu_features ();
>
> get_cpuinfo ();
> fails += CHECK_PROC (acpi, ACPI);
> @@ -159,7 +166,17 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (hle, HLE);
> fails += CHECK_PROC (ht, HTT);
> fails += CHECK_PROC (hybrid, HYBRID);
> - fails += CHECK_PROC (ibrs, IBRS_IBPB);
> + if (cpu_features->basic.kind == arch_kind_intel)
> + {
> + fails += CHECK_PROC (ibrs, IBRS_IBPB);
> + fails += CHECK_PROC (stibp, STIBP);
> + }
> + else if (cpu_features->basic.kind == arch_kind_amd)
> + {
> + fails += CHECK_PROC (ibpb, AMD_IBPB);
> + fails += CHECK_PROC (ibrs, AMD_IBRS);
> + fails += CHECK_PROC (stibp, AMD_STIBP);
> + }
> fails += CHECK_PROC (ibt, IBT);
> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
> fails += CHECK_PROC (invpcid, INVPCID);
> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (sgx, SGX);
> fails += CHECK_PROC (sgx_lc, SGX_LC);
> fails += CHECK_PROC (sha_ni, SHA);
> - fails += CHECK_PROC (shstk, SHSTK);
> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
Why do you need this? If kernel doesn't support SHSTK, it will be
turned off:
/* Check CET status. */
unsigned int cet_status = get_cet_status ();
if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
CPU_FEATURE_UNSET (cpu_features, IBT)
if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
CPU_FEATURE_UNSET (cpu_features, SHSTK)
> fails += CHECK_PROC (smap, SMAP);
> fails += CHECK_PROC (smep, SMEP);
> fails += CHECK_PROC (smx, SMX);
> fails += CHECK_PROC (ss, SS);
> - fails += CHECK_PROC (ssbd, SSBD);
> + if (cpu_features->basic.kind == arch_kind_intel)
> + fails += CHECK_PROC (ssbd, SSBD);
> + else if (cpu_features->basic.kind == arch_kind_amd)
> + fails += CHECK_PROC (ssbd, AMD_SSBD);
> fails += CHECK_PROC (sse, SSE);
> fails += CHECK_PROC (sse2, SSE2);
> fails += CHECK_PROC (pni, SSE3);
> @@ -229,7 +249,6 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (sse4_2, SSE4_2);
> fails += CHECK_PROC (sse4a, SSE4A);
> fails += CHECK_PROC (ssse3, SSSE3);
> - fails += CHECK_PROC (stibp, STIBP);
> fails += CHECK_PROC (svm, SVM);
> #ifdef __x86_64__
> /* NB: SYSCALL_SYSRET is 64-bit only. */
> --
> 2.30.2
>
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
2021-06-23 20:41 ` H.J. Lu
@ 2021-06-23 21:15 ` Adhemerval Zanella
2021-06-23 21:36 ` [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-23 21:15 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 23/06/2021 17:41, H.J. Lu wrote:
>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
>> fails += CHECK_PROC (sgx, SGX);
>> fails += CHECK_PROC (sgx_lc, SGX_LC);
>> fails += CHECK_PROC (sha_ni, SHA);
>> - fails += CHECK_PROC (shstk, SHSTK);
>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
>
> Why do you need this? If kernel doesn't support SHSTK, it will be
> turned off:
>
> /* Check CET status. */
> unsigned int cet_status = get_cet_status ();
>
> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> CPU_FEATURE_UNSET (cpu_features, IBT)
> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> CPU_FEATURE_UNSET (cpu_features, SHSTK)
The problem is this is only enabled for CET_ENABLED, the configuration I am using
does not define __CET__. So the CPU I am using does support SHSTK, but the bit
ended up not being cleared by glibc.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-23 21:15 ` Adhemerval Zanella
@ 2021-06-23 21:36 ` H.J. Lu
2021-06-24 0:30 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2021-06-23 21:36 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]
On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 23/06/2021 17:41, H.J. Lu wrote:
> >> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
> >> fails += CHECK_PROC (sgx, SGX);
> >> fails += CHECK_PROC (sgx_lc, SGX_LC);
> >> fails += CHECK_PROC (sha_ni, SHA);
> >> - fails += CHECK_PROC (shstk, SHSTK);
> >> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
> >
> > Why do you need this? If kernel doesn't support SHSTK, it will be
> > turned off:
> >
> > /* Check CET status. */
> > unsigned int cet_status = get_cet_status ();
> >
> > if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> > CPU_FEATURE_UNSET (cpu_features, IBT)
> > if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> > CPU_FEATURE_UNSET (cpu_features, SHSTK)
>
> The problem is this is only enabled for CET_ENABLED, the configuration I am using
> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
> ended up not being cleared by glibc.
IBT and SHSTK usable bits are copied from CPUID feature bits and later
cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
if CET is enabled so that they aren't set on CET capable processors
with non-CET enabled glibc.
Can you try this?
--
H.J.
[-- Attachment #2: 0001-x86-Copy-IBT-and-SHSTK-usable-only-if-CET-is-enabled.patch --]
[-- Type: text/x-patch, Size: 1958 bytes --]
From 976f4e2b9cb6e0766123a1cc3c2dc4c4339e0e75 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 23 Jun 2021 14:27:58 -0700
Subject: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
IBT and SHSTK usable bits are copied from CPUID feature bits and later
cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
if CET is enabled so that they aren't set on CET capable processors
with non-CET enabled glibc.
---
sysdeps/x86/cpu-features.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 81275dbdfa..a1d8d11cc4 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -76,7 +76,6 @@ update_usable (struct cpu_features *cpu_features)
CPU_FEATURE_SET_USABLE (cpu_features, PREFETCHWT1);
CPU_FEATURE_SET_USABLE (cpu_features, OSPKE);
CPU_FEATURE_SET_USABLE (cpu_features, WAITPKG);
- CPU_FEATURE_SET_USABLE (cpu_features, SHSTK);
CPU_FEATURE_SET_USABLE (cpu_features, GFNI);
CPU_FEATURE_SET_USABLE (cpu_features, RDPID);
CPU_FEATURE_SET_USABLE (cpu_features, RDRAND);
@@ -86,7 +85,6 @@ update_usable (struct cpu_features *cpu_features)
CPU_FEATURE_SET_USABLE (cpu_features, FSRM);
CPU_FEATURE_SET_USABLE (cpu_features, SERIALIZE);
CPU_FEATURE_SET_USABLE (cpu_features, TSXLDTRK);
- CPU_FEATURE_SET_USABLE (cpu_features, IBT);
CPU_FEATURE_SET_USABLE (cpu_features, LAHF64_SAHF64);
CPU_FEATURE_SET_USABLE (cpu_features, LZCNT);
CPU_FEATURE_SET_USABLE (cpu_features, SSE4A);
@@ -99,6 +97,11 @@ update_usable (struct cpu_features *cpu_features)
CPU_FEATURE_SET_USABLE (cpu_features, FSRCS);
CPU_FEATURE_SET_USABLE (cpu_features, PTWRITE);
+#if CET_ENABLED
+ CPU_FEATURE_SET_USABLE (cpu_features, IBT);
+ CPU_FEATURE_SET_USABLE (cpu_features, SHSTK);
+#endif
+
/* Can we call xgetbv? */
if (CPU_FEATURES_CPU_P (cpu_features, OSXSAVE))
{
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-23 21:36 ` [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled H.J. Lu
@ 2021-06-24 0:30 ` Adhemerval Zanella
2021-06-24 0:40 ` H.J. Lu
2021-06-24 7:56 ` Florian Weimer
0 siblings, 2 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-24 0:30 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 23/06/2021 18:36, H.J. Lu wrote:
> On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 23/06/2021 17:41, H.J. Lu wrote:
>>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
>>>> fails += CHECK_PROC (sgx, SGX);
>>>> fails += CHECK_PROC (sgx_lc, SGX_LC);
>>>> fails += CHECK_PROC (sha_ni, SHA);
>>>> - fails += CHECK_PROC (shstk, SHSTK);
>>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
>>>
>>> Why do you need this? If kernel doesn't support SHSTK, it will be
>>> turned off:
>>>
>>> /* Check CET status. */
>>> unsigned int cet_status = get_cet_status ();
>>>
>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
>>> CPU_FEATURE_UNSET (cpu_features, IBT)
>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
>>> CPU_FEATURE_UNSET (cpu_features, SHSTK)
>>
>> The problem is this is only enabled for CET_ENABLED, the configuration I am using
>> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
>> ended up not being cleared by glibc.
>
> IBT and SHSTK usable bits are copied from CPUID feature bits and later
> cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
> if CET is enabled so that they aren't set on CET capable processors
> with non-CET enabled glibc.
>
> Can you try this?
It fixes the SHSTK issue, but it still shows the following failures on
the Ryzen 9 cpu:
Checking HAS_CPU_FEATURE (IBRS_IBPB):
HAS_CPU_FEATURE (IBRS_IBPB): 0
cpuinfo (ibrs): 1
*** failure ***
Checking HAS_CPU_FEATURE (SSBD):
HAS_CPU_FEATURE (SSBD): 0
cpuinfo (ssbd): 1
*** failure ***
Checking HAS_CPU_FEATURE (STIBP):
HAS_CPU_FEATURE (STIBP): 0
cpuinfo (stibp): 1
*** failure ***
Below I update my patch, your look ok thanks!
---
[PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
The SSDB is also defined and implemented different on AMD [2],
and also a new AMD_SSDB flag is added. It should map to the
cpuinfo 'ssdb' on recent AMD cpus.
It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
on recent AMD cpus.
Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
[1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
[2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
---
sysdeps/x86/bits/platform/x86.h | 4 ++++
sysdeps/x86/include/cpu-features.h | 12 ++++++++++++
sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++----
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
index fe08d8a1b6..26e3b67ede 100644
--- a/sysdeps/x86/bits/platform/x86.h
+++ b/sysdeps/x86/bits/platform/x86.h
@@ -278,6 +278,10 @@ enum
+ cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
+ x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
+ x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
+ x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
+ x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
x86_cpu_index_7_ecx_1_eax
= (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
index d042a2ebef..4f1c4ee402 100644
--- a/sysdeps/x86/include/cpu-features.h
+++ b/sysdeps/x86/include/cpu-features.h
@@ -289,6 +289,10 @@ enum
/* EBX. */
#define bit_cpu_WBNOINVD (1u << 9)
+#define bit_cpu_AMD_IBPB (1u << 12)
+#define bit_cpu_AMD_IBRS (1u << 14)
+#define bit_cpu_AMD_STIBP (1u << 15)
+#define bit_cpu_AMD_SSBD (1u << 24)
/* CPUID_INDEX_7_ECX_1. */
@@ -519,6 +523,10 @@ enum
/* EBX. */
#define index_cpu_WBNOINVD CPUID_INDEX_80000008
+#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
+#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
+#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
+#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
/* CPUID_INDEX_7_ECX_1. */
@@ -749,6 +757,10 @@ enum
/* EBX. */
#define reg_WBNOINVD ebx
+#define reg_AMD_IBPB ebx
+#define reg_AMD_IBRS ebx
+#define reg_AMD_STIBP ebx
+#define reg_AMD_SSBD ebx
/* CPUID_INDEX_7_ECX_1. */
diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
index 75e7eb9352..f457e8677b 100644
--- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
+++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
@@ -16,10 +16,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <sys/platform/x86.h>
+#include <cpu-features.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <stdbool.h>
static char *cpu_flags;
@@ -99,6 +100,7 @@ static int
do_test (int argc, char **argv)
{
int fails = 0;
+ const struct cpu_features *cpu_features = __get_cpu_features ();
get_cpuinfo ();
fails += CHECK_PROC (acpi, ACPI);
@@ -159,7 +161,17 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (hle, HLE);
fails += CHECK_PROC (ht, HTT);
fails += CHECK_PROC (hybrid, HYBRID);
- fails += CHECK_PROC (ibrs, IBRS_IBPB);
+ if (cpu_features->basic.kind == arch_kind_intel)
+ {
+ fails += CHECK_PROC (ibrs, IBRS_IBPB);
+ fails += CHECK_PROC (stibp, STIBP);
+ }
+ else if (cpu_features->basic.kind == arch_kind_amd)
+ {
+ fails += CHECK_PROC (ibpb, AMD_IBPB);
+ fails += CHECK_PROC (ibrs, AMD_IBRS);
+ fails += CHECK_PROC (stibp, AMD_STIBP);
+ }
fails += CHECK_PROC (ibt, IBT);
fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
fails += CHECK_PROC (invpcid, INVPCID);
@@ -221,7 +233,10 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (smep, SMEP);
fails += CHECK_PROC (smx, SMX);
fails += CHECK_PROC (ss, SS);
- fails += CHECK_PROC (ssbd, SSBD);
+ if (cpu_features->basic.kind == arch_kind_intel)
+ fails += CHECK_PROC (ssbd, SSBD);
+ else if (cpu_features->basic.kind == arch_kind_amd)
+ fails += CHECK_PROC (ssbd, AMD_SSBD);
fails += CHECK_PROC (sse, SSE);
fails += CHECK_PROC (sse2, SSE2);
fails += CHECK_PROC (pni, SSE3);
@@ -229,7 +244,6 @@ do_test (int argc, char **argv)
fails += CHECK_PROC (sse4_2, SSE4_2);
fails += CHECK_PROC (sse4a, SSE4A);
fails += CHECK_PROC (ssse3, SSSE3);
- fails += CHECK_PROC (stibp, STIBP);
fails += CHECK_PROC (svm, SVM);
#ifdef __x86_64__
/* NB: SYSCALL_SYSRET is 64-bit only. */
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 0:30 ` Adhemerval Zanella
@ 2021-06-24 0:40 ` H.J. Lu
2021-06-24 12:17 ` Adhemerval Zanella
2021-06-24 7:56 ` Florian Weimer
1 sibling, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2021-06-24 0:40 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Wed, Jun 23, 2021 at 5:30 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 23/06/2021 18:36, H.J. Lu wrote:
> > On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 23/06/2021 17:41, H.J. Lu wrote:
> >>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
> >>>> fails += CHECK_PROC (sgx, SGX);
> >>>> fails += CHECK_PROC (sgx_lc, SGX_LC);
> >>>> fails += CHECK_PROC (sha_ni, SHA);
> >>>> - fails += CHECK_PROC (shstk, SHSTK);
> >>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
> >>>
> >>> Why do you need this? If kernel doesn't support SHSTK, it will be
> >>> turned off:
> >>>
> >>> /* Check CET status. */
> >>> unsigned int cet_status = get_cet_status ();
> >>>
> >>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> >>> CPU_FEATURE_UNSET (cpu_features, IBT)
> >>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> >>> CPU_FEATURE_UNSET (cpu_features, SHSTK)
> >>
> >> The problem is this is only enabled for CET_ENABLED, the configuration I am using
> >> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
> >> ended up not being cleared by glibc.
> >
> > IBT and SHSTK usable bits are copied from CPUID feature bits and later
> > cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
> > if CET is enabled so that they aren't set on CET capable processors
> > with non-CET enabled glibc.
> >
> > Can you try this?
>
> It fixes the SHSTK issue, but it still shows the following failures on
> the Ryzen 9 cpu:
>
> Checking HAS_CPU_FEATURE (IBRS_IBPB):
> HAS_CPU_FEATURE (IBRS_IBPB): 0
> cpuinfo (ibrs): 1
> *** failure ***
>
> Checking HAS_CPU_FEATURE (SSBD):
> HAS_CPU_FEATURE (SSBD): 0
> cpuinfo (ssbd): 1
> *** failure ***
>
> Checking HAS_CPU_FEATURE (STIBP):
> HAS_CPU_FEATURE (STIBP): 0
> cpuinfo (stibp): 1
> *** failure ***
>
> Below I update my patch, your look ok thanks!
>
> ---
>
> [PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
>
> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
> x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
>
> The SSDB is also defined and implemented different on AMD [2],
> and also a new AMD_SSDB flag is added. It should map to the
> cpuinfo 'ssdb' on recent AMD cpus.
>
> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
> on recent AMD cpus.
>
> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
>
> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
> ---
> sysdeps/x86/bits/platform/x86.h | 4 ++++
> sysdeps/x86/include/cpu-features.h | 12 ++++++++++++
> sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++----
> 3 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
> index fe08d8a1b6..26e3b67ede 100644
> --- a/sysdeps/x86/bits/platform/x86.h
> +++ b/sysdeps/x86/bits/platform/x86.h
> @@ -278,6 +278,10 @@ enum
> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
>
> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
>
> x86_cpu_index_7_ecx_1_eax
> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
> index d042a2ebef..4f1c4ee402 100644
> --- a/sysdeps/x86/include/cpu-features.h
> +++ b/sysdeps/x86/include/cpu-features.h
> @@ -289,6 +289,10 @@ enum
>
> /* EBX. */
> #define bit_cpu_WBNOINVD (1u << 9)
> +#define bit_cpu_AMD_IBPB (1u << 12)
> +#define bit_cpu_AMD_IBRS (1u << 14)
> +#define bit_cpu_AMD_STIBP (1u << 15)
> +#define bit_cpu_AMD_SSBD (1u << 24)
>
> /* CPUID_INDEX_7_ECX_1. */
>
> @@ -519,6 +523,10 @@ enum
>
> /* EBX. */
> #define index_cpu_WBNOINVD CPUID_INDEX_80000008
> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
>
> /* CPUID_INDEX_7_ECX_1. */
>
> @@ -749,6 +757,10 @@ enum
>
> /* EBX. */
> #define reg_WBNOINVD ebx
> +#define reg_AMD_IBPB ebx
> +#define reg_AMD_IBRS ebx
> +#define reg_AMD_STIBP ebx
> +#define reg_AMD_SSBD ebx
>
> /* CPUID_INDEX_7_ECX_1. */
>
> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> index 75e7eb9352..f457e8677b 100644
> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> @@ -16,10 +16,11 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <sys/platform/x86.h>
> +#include <cpu-features.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <stdbool.h>
>
> static char *cpu_flags;
>
> @@ -99,6 +100,7 @@ static int
> do_test (int argc, char **argv)
> {
> int fails = 0;
> + const struct cpu_features *cpu_features = __get_cpu_features ();
>
> get_cpuinfo ();
> fails += CHECK_PROC (acpi, ACPI);
> @@ -159,7 +161,17 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (hle, HLE);
> fails += CHECK_PROC (ht, HTT);
> fails += CHECK_PROC (hybrid, HYBRID);
> - fails += CHECK_PROC (ibrs, IBRS_IBPB);
> + if (cpu_features->basic.kind == arch_kind_intel)
> + {
> + fails += CHECK_PROC (ibrs, IBRS_IBPB);
> + fails += CHECK_PROC (stibp, STIBP);
> + }
> + else if (cpu_features->basic.kind == arch_kind_amd)
> + {
> + fails += CHECK_PROC (ibpb, AMD_IBPB);
> + fails += CHECK_PROC (ibrs, AMD_IBRS);
> + fails += CHECK_PROC (stibp, AMD_STIBP);
> + }
> fails += CHECK_PROC (ibt, IBT);
> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
> fails += CHECK_PROC (invpcid, INVPCID);
> @@ -221,7 +233,10 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (smep, SMEP);
> fails += CHECK_PROC (smx, SMX);
> fails += CHECK_PROC (ss, SS);
> - fails += CHECK_PROC (ssbd, SSBD);
> + if (cpu_features->basic.kind == arch_kind_intel)
> + fails += CHECK_PROC (ssbd, SSBD);
> + else if (cpu_features->basic.kind == arch_kind_amd)
> + fails += CHECK_PROC (ssbd, AMD_SSBD);
> fails += CHECK_PROC (sse, SSE);
> fails += CHECK_PROC (sse2, SSE2);
> fails += CHECK_PROC (pni, SSE3);
> @@ -229,7 +244,6 @@ do_test (int argc, char **argv)
> fails += CHECK_PROC (sse4_2, SSE4_2);
> fails += CHECK_PROC (sse4a, SSE4A);
> fails += CHECK_PROC (ssse3, SSSE3);
> - fails += CHECK_PROC (stibp, STIBP);
> fails += CHECK_PROC (svm, SVM);
> #ifdef __x86_64__
> /* NB: SYSCALL_SYSRET is 64-bit only. */
> --
> 2.30.2
LGTM.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 0:30 ` Adhemerval Zanella
2021-06-24 0:40 ` H.J. Lu
@ 2021-06-24 7:56 ` Florian Weimer
2021-06-24 12:07 ` Adhemerval Zanella
1 sibling, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2021-06-24 7:56 UTC (permalink / raw)
To: Adhemerval Zanella via Libc-alpha
* Adhemerval Zanella via Libc-alpha:
> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
Do these show up as USABLE automatically? The test suggests to me that
they do.
This points to a deeper problem elsewhere: new bits should not become
available automatically, they must be copied over explicitly because
ld.so does not know the semantics for a new bit in USABLE.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 7:56 ` Florian Weimer
@ 2021-06-24 12:07 ` Adhemerval Zanella
0 siblings, 0 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-24 12:07 UTC (permalink / raw)
To: Florian Weimer, Adhemerval Zanella via Libc-alpha
On 24/06/2021 04:56, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
>> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
>> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
>> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
>
> Do these show up as USABLE automatically? The test suggests to me that
> they do.
No, these are set not usable by CPU_FEATURE_USABLE. The test only checks
if kernel advertise through /proc/cpuinfo matches HAS_CPU_FEATURE.
>
> This points to a deeper problem elsewhere: new bits should not become
> available automatically, they must be copied over explicitly because
> ld.so does not know the semantics for a new bit in USABLE.
I think the cpu-features.c already does it for the usable fields. I tend
to agree that we should not enable unknown flags, we might now know prior
hand that a future flag might or not require glibc support.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 0:40 ` H.J. Lu
@ 2021-06-24 12:17 ` Adhemerval Zanella
2021-06-24 12:27 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-24 12:17 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 23/06/2021 21:40, H.J. Lu wrote:
> On Wed, Jun 23, 2021 at 5:30 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 23/06/2021 18:36, H.J. Lu wrote:
>>> On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 23/06/2021 17:41, H.J. Lu wrote:
>>>>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
>>>>>> fails += CHECK_PROC (sgx, SGX);
>>>>>> fails += CHECK_PROC (sgx_lc, SGX_LC);
>>>>>> fails += CHECK_PROC (sha_ni, SHA);
>>>>>> - fails += CHECK_PROC (shstk, SHSTK);
>>>>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
>>>>>
>>>>> Why do you need this? If kernel doesn't support SHSTK, it will be
>>>>> turned off:
>>>>>
>>>>> /* Check CET status. */
>>>>> unsigned int cet_status = get_cet_status ();
>>>>>
>>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
>>>>> CPU_FEATURE_UNSET (cpu_features, IBT)
>>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
>>>>> CPU_FEATURE_UNSET (cpu_features, SHSTK)
>>>>
>>>> The problem is this is only enabled for CET_ENABLED, the configuration I am using
>>>> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
>>>> ended up not being cleared by glibc.
>>>
>>> IBT and SHSTK usable bits are copied from CPUID feature bits and later
>>> cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
>>> if CET is enabled so that they aren't set on CET capable processors
>>> with non-CET enabled glibc.
>>>
>>> Can you try this?
>>
>> It fixes the SHSTK issue, but it still shows the following failures on
>> the Ryzen 9 cpu:
>>
>> Checking HAS_CPU_FEATURE (IBRS_IBPB):
>> HAS_CPU_FEATURE (IBRS_IBPB): 0
>> cpuinfo (ibrs): 1
>> *** failure ***
>>
>> Checking HAS_CPU_FEATURE (SSBD):
>> HAS_CPU_FEATURE (SSBD): 0
>> cpuinfo (ssbd): 1
>> *** failure ***
>>
>> Checking HAS_CPU_FEATURE (STIBP):
>> HAS_CPU_FEATURE (STIBP): 0
>> cpuinfo (stibp): 1
>> *** failure ***
>>
>> Below I update my patch, your look ok thanks!
>>
>> ---
>>
>> [PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
>>
>> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
>> x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
>>
>> The SSDB is also defined and implemented different on AMD [2],
>> and also a new AMD_SSDB flag is added. It should map to the
>> cpuinfo 'ssdb' on recent AMD cpus.
>>
>> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
>> on recent AMD cpus.
>>
>> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
>>
>> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
>> ---
>> sysdeps/x86/bits/platform/x86.h | 4 ++++
>> sysdeps/x86/include/cpu-features.h | 12 ++++++++++++
>> sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++----
>> 3 files changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
>> index fe08d8a1b6..26e3b67ede 100644
>> --- a/sysdeps/x86/bits/platform/x86.h
>> +++ b/sysdeps/x86/bits/platform/x86.h
>> @@ -278,6 +278,10 @@ enum
>> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
>>
>> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
>> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
>> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
>> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
>> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
>>
>> x86_cpu_index_7_ecx_1_eax
>> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
>> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
>> index d042a2ebef..4f1c4ee402 100644
>> --- a/sysdeps/x86/include/cpu-features.h
>> +++ b/sysdeps/x86/include/cpu-features.h
>> @@ -289,6 +289,10 @@ enum
>>
>> /* EBX. */
>> #define bit_cpu_WBNOINVD (1u << 9)
>> +#define bit_cpu_AMD_IBPB (1u << 12)
>> +#define bit_cpu_AMD_IBRS (1u << 14)
>> +#define bit_cpu_AMD_STIBP (1u << 15)
>> +#define bit_cpu_AMD_SSBD (1u << 24)
>>
>> /* CPUID_INDEX_7_ECX_1. */
>>
>> @@ -519,6 +523,10 @@ enum
>>
>> /* EBX. */
>> #define index_cpu_WBNOINVD CPUID_INDEX_80000008
>> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
>> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
>> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
>> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
>>
>> /* CPUID_INDEX_7_ECX_1. */
>>
>> @@ -749,6 +757,10 @@ enum
>>
>> /* EBX. */
>> #define reg_WBNOINVD ebx
>> +#define reg_AMD_IBPB ebx
>> +#define reg_AMD_IBRS ebx
>> +#define reg_AMD_STIBP ebx
>> +#define reg_AMD_SSBD ebx
>>
>> /* CPUID_INDEX_7_ECX_1. */
>>
>> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
>> index 75e7eb9352..f457e8677b 100644
>> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
>> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
>> @@ -16,10 +16,11 @@
>> License along with the GNU C Library; if not, see
>> <https://www.gnu.org/licenses/>. */
>>
>> -#include <sys/platform/x86.h>
>> +#include <cpu-features.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> +#include <stdbool.h>
>>
>> static char *cpu_flags;
>>
>> @@ -99,6 +100,7 @@ static int
>> do_test (int argc, char **argv)
>> {
>> int fails = 0;
>> + const struct cpu_features *cpu_features = __get_cpu_features ();
>>
>> get_cpuinfo ();
>> fails += CHECK_PROC (acpi, ACPI);
>> @@ -159,7 +161,17 @@ do_test (int argc, char **argv)
>> fails += CHECK_PROC (hle, HLE);
>> fails += CHECK_PROC (ht, HTT);
>> fails += CHECK_PROC (hybrid, HYBRID);
>> - fails += CHECK_PROC (ibrs, IBRS_IBPB);
>> + if (cpu_features->basic.kind == arch_kind_intel)
>> + {
>> + fails += CHECK_PROC (ibrs, IBRS_IBPB);
>> + fails += CHECK_PROC (stibp, STIBP);
>> + }
>> + else if (cpu_features->basic.kind == arch_kind_amd)
>> + {
>> + fails += CHECK_PROC (ibpb, AMD_IBPB);
>> + fails += CHECK_PROC (ibrs, AMD_IBRS);
>> + fails += CHECK_PROC (stibp, AMD_STIBP);
>> + }
>> fails += CHECK_PROC (ibt, IBT);
>> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
>> fails += CHECK_PROC (invpcid, INVPCID);
>> @@ -221,7 +233,10 @@ do_test (int argc, char **argv)
>> fails += CHECK_PROC (smep, SMEP);
>> fails += CHECK_PROC (smx, SMX);
>> fails += CHECK_PROC (ss, SS);
>> - fails += CHECK_PROC (ssbd, SSBD);
>> + if (cpu_features->basic.kind == arch_kind_intel)
>> + fails += CHECK_PROC (ssbd, SSBD);
>> + else if (cpu_features->basic.kind == arch_kind_amd)
>> + fails += CHECK_PROC (ssbd, AMD_SSBD);
>> fails += CHECK_PROC (sse, SSE);
>> fails += CHECK_PROC (sse2, SSE2);
>> fails += CHECK_PROC (pni, SSE3);
>> @@ -229,7 +244,6 @@ do_test (int argc, char **argv)
>> fails += CHECK_PROC (sse4_2, SSE4_2);
>> fails += CHECK_PROC (sse4a, SSE4A);
>> fails += CHECK_PROC (ssse3, SSSE3);
>> - fails += CHECK_PROC (stibp, STIBP);
>> fails += CHECK_PROC (svm, SVM);
>> #ifdef __x86_64__
>> /* NB: SYSCALL_SYSRET is 64-bit only. */
>> --
>> 2.30.2
>
> LGTM.
>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
I will also add the updated documentation:
diff --git a/manual/platform.texi b/manual/platform.texi
index a0b204b099..bf1483fe82 100644
--- a/manual/platform.texi
+++ b/manual/platform.texi
@@ -659,6 +659,18 @@ XSETBV/XGETBV instructions, and XCR0.
@item
@code{XTPRUPDCTRL} -- xTPR Update Control.
+@item
+@code{AMD_IBPB} -- Indirect branch predictor barrier (IBPB) for AMD cpus.
+
+@item
+@code{AMD_IBRS} -- Indirect branch restricted speculation (IBPB) for AMD cpus.
+
+@item
+@code{AMD_STIBP} -- Single thread indirect branch predictors (STIBP) for AMD cpus.
+
+@item
+@code{AMD_SSBD} -- Speculative Store Bypass Disable (SSBD) for AMD cpus.
+
@end itemize
You could query if a processor supports @code{AVX} with:
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 12:17 ` Adhemerval Zanella
@ 2021-06-24 12:27 ` H.J. Lu
2021-06-24 12:55 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2021-06-24 12:27 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Thu, Jun 24, 2021 at 5:17 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 23/06/2021 21:40, H.J. Lu wrote:
> > On Wed, Jun 23, 2021 at 5:30 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 23/06/2021 18:36, H.J. Lu wrote:
> >>> On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 23/06/2021 17:41, H.J. Lu wrote:
> >>>>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
> >>>>>> fails += CHECK_PROC (sgx, SGX);
> >>>>>> fails += CHECK_PROC (sgx_lc, SGX_LC);
> >>>>>> fails += CHECK_PROC (sha_ni, SHA);
> >>>>>> - fails += CHECK_PROC (shstk, SHSTK);
> >>>>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
> >>>>>
> >>>>> Why do you need this? If kernel doesn't support SHSTK, it will be
> >>>>> turned off:
> >>>>>
> >>>>> /* Check CET status. */
> >>>>> unsigned int cet_status = get_cet_status ();
> >>>>>
> >>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> >>>>> CPU_FEATURE_UNSET (cpu_features, IBT)
> >>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> >>>>> CPU_FEATURE_UNSET (cpu_features, SHSTK)
> >>>>
> >>>> The problem is this is only enabled for CET_ENABLED, the configuration I am using
> >>>> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
> >>>> ended up not being cleared by glibc.
> >>>
> >>> IBT and SHSTK usable bits are copied from CPUID feature bits and later
> >>> cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
> >>> if CET is enabled so that they aren't set on CET capable processors
> >>> with non-CET enabled glibc.
> >>>
> >>> Can you try this?
> >>
> >> It fixes the SHSTK issue, but it still shows the following failures on
> >> the Ryzen 9 cpu:
> >>
> >> Checking HAS_CPU_FEATURE (IBRS_IBPB):
> >> HAS_CPU_FEATURE (IBRS_IBPB): 0
> >> cpuinfo (ibrs): 1
> >> *** failure ***
> >>
> >> Checking HAS_CPU_FEATURE (SSBD):
> >> HAS_CPU_FEATURE (SSBD): 0
> >> cpuinfo (ssbd): 1
> >> *** failure ***
> >>
> >> Checking HAS_CPU_FEATURE (STIBP):
> >> HAS_CPU_FEATURE (STIBP): 0
> >> cpuinfo (stibp): 1
> >> *** failure ***
> >>
> >> Below I update my patch, your look ok thanks!
> >>
> >> ---
> >>
> >> [PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
> >>
> >> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
> >> x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
> >>
> >> The SSDB is also defined and implemented different on AMD [2],
> >> and also a new AMD_SSDB flag is added. It should map to the
> >> cpuinfo 'ssdb' on recent AMD cpus.
> >>
> >> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
> >> on recent AMD cpus.
> >>
> >> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
> >>
> >> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
> >> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
> >> ---
> >> sysdeps/x86/bits/platform/x86.h | 4 ++++
> >> sysdeps/x86/include/cpu-features.h | 12 ++++++++++++
> >> sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++----
> >> 3 files changed, 34 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
> >> index fe08d8a1b6..26e3b67ede 100644
> >> --- a/sysdeps/x86/bits/platform/x86.h
> >> +++ b/sysdeps/x86/bits/platform/x86.h
> >> @@ -278,6 +278,10 @@ enum
> >> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
> >>
> >> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
> >> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
> >> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
> >> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
> >> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
> >>
> >> x86_cpu_index_7_ecx_1_eax
> >> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
> >> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
> >> index d042a2ebef..4f1c4ee402 100644
> >> --- a/sysdeps/x86/include/cpu-features.h
> >> +++ b/sysdeps/x86/include/cpu-features.h
> >> @@ -289,6 +289,10 @@ enum
> >>
> >> /* EBX. */
> >> #define bit_cpu_WBNOINVD (1u << 9)
> >> +#define bit_cpu_AMD_IBPB (1u << 12)
> >> +#define bit_cpu_AMD_IBRS (1u << 14)
> >> +#define bit_cpu_AMD_STIBP (1u << 15)
> >> +#define bit_cpu_AMD_SSBD (1u << 24)
> >>
> >> /* CPUID_INDEX_7_ECX_1. */
> >>
> >> @@ -519,6 +523,10 @@ enum
> >>
> >> /* EBX. */
> >> #define index_cpu_WBNOINVD CPUID_INDEX_80000008
> >> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
> >> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
> >> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
> >> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
> >>
> >> /* CPUID_INDEX_7_ECX_1. */
> >>
> >> @@ -749,6 +757,10 @@ enum
> >>
> >> /* EBX. */
> >> #define reg_WBNOINVD ebx
> >> +#define reg_AMD_IBPB ebx
> >> +#define reg_AMD_IBRS ebx
> >> +#define reg_AMD_STIBP ebx
> >> +#define reg_AMD_SSBD ebx
> >>
> >> /* CPUID_INDEX_7_ECX_1. */
> >>
> >> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> >> index 75e7eb9352..f457e8677b 100644
> >> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
> >> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
> >> @@ -16,10 +16,11 @@
> >> License along with the GNU C Library; if not, see
> >> <https://www.gnu.org/licenses/>. */
> >>
> >> -#include <sys/platform/x86.h>
> >> +#include <cpu-features.h>
> >> #include <stdio.h>
> >> #include <stdlib.h>
> >> #include <string.h>
> >> +#include <stdbool.h>
> >>
> >> static char *cpu_flags;
> >>
> >> @@ -99,6 +100,7 @@ static int
> >> do_test (int argc, char **argv)
> >> {
> >> int fails = 0;
> >> + const struct cpu_features *cpu_features = __get_cpu_features ();
> >>
> >> get_cpuinfo ();
> >> fails += CHECK_PROC (acpi, ACPI);
> >> @@ -159,7 +161,17 @@ do_test (int argc, char **argv)
> >> fails += CHECK_PROC (hle, HLE);
> >> fails += CHECK_PROC (ht, HTT);
> >> fails += CHECK_PROC (hybrid, HYBRID);
> >> - fails += CHECK_PROC (ibrs, IBRS_IBPB);
> >> + if (cpu_features->basic.kind == arch_kind_intel)
> >> + {
> >> + fails += CHECK_PROC (ibrs, IBRS_IBPB);
> >> + fails += CHECK_PROC (stibp, STIBP);
> >> + }
> >> + else if (cpu_features->basic.kind == arch_kind_amd)
> >> + {
> >> + fails += CHECK_PROC (ibpb, AMD_IBPB);
> >> + fails += CHECK_PROC (ibrs, AMD_IBRS);
> >> + fails += CHECK_PROC (stibp, AMD_STIBP);
> >> + }
> >> fails += CHECK_PROC (ibt, IBT);
> >> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
> >> fails += CHECK_PROC (invpcid, INVPCID);
> >> @@ -221,7 +233,10 @@ do_test (int argc, char **argv)
> >> fails += CHECK_PROC (smep, SMEP);
> >> fails += CHECK_PROC (smx, SMX);
> >> fails += CHECK_PROC (ss, SS);
> >> - fails += CHECK_PROC (ssbd, SSBD);
> >> + if (cpu_features->basic.kind == arch_kind_intel)
> >> + fails += CHECK_PROC (ssbd, SSBD);
> >> + else if (cpu_features->basic.kind == arch_kind_amd)
> >> + fails += CHECK_PROC (ssbd, AMD_SSBD);
> >> fails += CHECK_PROC (sse, SSE);
> >> fails += CHECK_PROC (sse2, SSE2);
> >> fails += CHECK_PROC (pni, SSE3);
> >> @@ -229,7 +244,6 @@ do_test (int argc, char **argv)
> >> fails += CHECK_PROC (sse4_2, SSE4_2);
> >> fails += CHECK_PROC (sse4a, SSE4A);
> >> fails += CHECK_PROC (ssse3, SSSE3);
> >> - fails += CHECK_PROC (stibp, STIBP);
> >> fails += CHECK_PROC (svm, SVM);
> >> #ifdef __x86_64__
> >> /* NB: SYSCALL_SYSRET is 64-bit only. */
> >> --
> >> 2.30.2
> >
> > LGTM.
> >
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> I will also add the updated documentation:
>
> diff --git a/manual/platform.texi b/manual/platform.texi
> index a0b204b099..bf1483fe82 100644
> --- a/manual/platform.texi
> +++ b/manual/platform.texi
> @@ -659,6 +659,18 @@ XSETBV/XGETBV instructions, and XCR0.
> @item
> @code{XTPRUPDCTRL} -- xTPR Update Control.
>
> +@item
> +@code{AMD_IBPB} -- Indirect branch predictor barrier (IBPB) for AMD cpus.
> +
> +@item
> +@code{AMD_IBRS} -- Indirect branch restricted speculation (IBPB) for AMD cpus.
> +
> +@item
> +@code{AMD_STIBP} -- Single thread indirect branch predictors (STIBP) for AMD cpus.
> +
> +@item
> +@code{AMD_SSBD} -- Speculative Store Bypass Disable (SSBD) for AMD cpus.
> +
> @end itemize
>
> You could query if a processor supports @code{AVX} with:
>
Please sort new features by name.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled
2021-06-24 12:27 ` H.J. Lu
@ 2021-06-24 12:55 ` Adhemerval Zanella
0 siblings, 0 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2021-06-24 12:55 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 24/06/2021 09:27, H.J. Lu wrote:
> On Thu, Jun 24, 2021 at 5:17 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 23/06/2021 21:40, H.J. Lu wrote:
>>> On Wed, Jun 23, 2021 at 5:30 PM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 23/06/2021 18:36, H.J. Lu wrote:
>>>>> On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella
>>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 23/06/2021 17:41, H.J. Lu wrote:
>>>>>>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv)
>>>>>>>> fails += CHECK_PROC (sgx, SGX);
>>>>>>>> fails += CHECK_PROC (sgx_lc, SGX_LC);
>>>>>>>> fails += CHECK_PROC (sha_ni, SHA);
>>>>>>>> - fails += CHECK_PROC (shstk, SHSTK);
>>>>>>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK);
>>>>>>>
>>>>>>> Why do you need this? If kernel doesn't support SHSTK, it will be
>>>>>>> turned off:
>>>>>>>
>>>>>>> /* Check CET status. */
>>>>>>> unsigned int cet_status = get_cet_status ();
>>>>>>>
>>>>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
>>>>>>> CPU_FEATURE_UNSET (cpu_features, IBT)
>>>>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
>>>>>>> CPU_FEATURE_UNSET (cpu_features, SHSTK)
>>>>>>
>>>>>> The problem is this is only enabled for CET_ENABLED, the configuration I am using
>>>>>> does not define __CET__. So the CPU I am using does support SHSTK, but the bit
>>>>>> ended up not being cleared by glibc.
>>>>>
>>>>> IBT and SHSTK usable bits are copied from CPUID feature bits and later
>>>>> cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only
>>>>> if CET is enabled so that they aren't set on CET capable processors
>>>>> with non-CET enabled glibc.
>>>>>
>>>>> Can you try this?
>>>>
>>>> It fixes the SHSTK issue, but it still shows the following failures on
>>>> the Ryzen 9 cpu:
>>>>
>>>> Checking HAS_CPU_FEATURE (IBRS_IBPB):
>>>> HAS_CPU_FEATURE (IBRS_IBPB): 0
>>>> cpuinfo (ibrs): 1
>>>> *** failure ***
>>>>
>>>> Checking HAS_CPU_FEATURE (SSBD):
>>>> HAS_CPU_FEATURE (SSBD): 0
>>>> cpuinfo (ssbd): 1
>>>> *** failure ***
>>>>
>>>> Checking HAS_CPU_FEATURE (STIBP):
>>>> HAS_CPU_FEATURE (STIBP): 0
>>>> cpuinfo (stibp): 1
>>>> *** failure ***
>>>>
>>>> Below I update my patch, your look ok thanks!
>>>>
>>>> ---
>>>>
>>>> [PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873)
>>>>
>>>> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new
>>>> x86_64_cpu are added and IBRS_IBPB is only tested for Intel.
>>>>
>>>> The SSDB is also defined and implemented different on AMD [2],
>>>> and also a new AMD_SSDB flag is added. It should map to the
>>>> cpuinfo 'ssdb' on recent AMD cpus.
>>>>
>>>> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static
>>>> on recent AMD cpus.
>>>>
>>>> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X.
>>>>
>>>> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf
>>>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889
>>>> ---
>>>> sysdeps/x86/bits/platform/x86.h | 4 ++++
>>>> sysdeps/x86/include/cpu-features.h | 12 ++++++++++++
>>>> sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++----
>>>> 3 files changed, 34 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h
>>>> index fe08d8a1b6..26e3b67ede 100644
>>>> --- a/sysdeps/x86/bits/platform/x86.h
>>>> +++ b/sysdeps/x86/bits/platform/x86.h
>>>> @@ -278,6 +278,10 @@ enum
>>>> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)),
>>>>
>>>> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9,
>>>> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12,
>>>> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14,
>>>> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15,
>>>> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24,
>>>>
>>>> x86_cpu_index_7_ecx_1_eax
>>>> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int)
>>>> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
>>>> index d042a2ebef..4f1c4ee402 100644
>>>> --- a/sysdeps/x86/include/cpu-features.h
>>>> +++ b/sysdeps/x86/include/cpu-features.h
>>>> @@ -289,6 +289,10 @@ enum
>>>>
>>>> /* EBX. */
>>>> #define bit_cpu_WBNOINVD (1u << 9)
>>>> +#define bit_cpu_AMD_IBPB (1u << 12)
>>>> +#define bit_cpu_AMD_IBRS (1u << 14)
>>>> +#define bit_cpu_AMD_STIBP (1u << 15)
>>>> +#define bit_cpu_AMD_SSBD (1u << 24)
>>>>
>>>> /* CPUID_INDEX_7_ECX_1. */
>>>>
>>>> @@ -519,6 +523,10 @@ enum
>>>>
>>>> /* EBX. */
>>>> #define index_cpu_WBNOINVD CPUID_INDEX_80000008
>>>> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008
>>>> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008
>>>> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008
>>>> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008
>>>>
>>>> /* CPUID_INDEX_7_ECX_1. */
>>>>
>>>> @@ -749,6 +757,10 @@ enum
>>>>
>>>> /* EBX. */
>>>> #define reg_WBNOINVD ebx
>>>> +#define reg_AMD_IBPB ebx
>>>> +#define reg_AMD_IBRS ebx
>>>> +#define reg_AMD_STIBP ebx
>>>> +#define reg_AMD_SSBD ebx
>>>>
>>>> /* CPUID_INDEX_7_ECX_1. */
>>>>
>>>> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c
>>>> index 75e7eb9352..f457e8677b 100644
>>>> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c
>>>> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c
>>>> @@ -16,10 +16,11 @@
>>>> License along with the GNU C Library; if not, see
>>>> <https://www.gnu.org/licenses/>. */
>>>>
>>>> -#include <sys/platform/x86.h>
>>>> +#include <cpu-features.h>
>>>> #include <stdio.h>
>>>> #include <stdlib.h>
>>>> #include <string.h>
>>>> +#include <stdbool.h>
>>>>
>>>> static char *cpu_flags;
>>>>
>>>> @@ -99,6 +100,7 @@ static int
>>>> do_test (int argc, char **argv)
>>>> {
>>>> int fails = 0;
>>>> + const struct cpu_features *cpu_features = __get_cpu_features ();
>>>>
>>>> get_cpuinfo ();
>>>> fails += CHECK_PROC (acpi, ACPI);
>>>> @@ -159,7 +161,17 @@ do_test (int argc, char **argv)
>>>> fails += CHECK_PROC (hle, HLE);
>>>> fails += CHECK_PROC (ht, HTT);
>>>> fails += CHECK_PROC (hybrid, HYBRID);
>>>> - fails += CHECK_PROC (ibrs, IBRS_IBPB);
>>>> + if (cpu_features->basic.kind == arch_kind_intel)
>>>> + {
>>>> + fails += CHECK_PROC (ibrs, IBRS_IBPB);
>>>> + fails += CHECK_PROC (stibp, STIBP);
>>>> + }
>>>> + else if (cpu_features->basic.kind == arch_kind_amd)
>>>> + {
>>>> + fails += CHECK_PROC (ibpb, AMD_IBPB);
>>>> + fails += CHECK_PROC (ibrs, AMD_IBRS);
>>>> + fails += CHECK_PROC (stibp, AMD_STIBP);
>>>> + }
>>>> fails += CHECK_PROC (ibt, IBT);
>>>> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC);
>>>> fails += CHECK_PROC (invpcid, INVPCID);
>>>> @@ -221,7 +233,10 @@ do_test (int argc, char **argv)
>>>> fails += CHECK_PROC (smep, SMEP);
>>>> fails += CHECK_PROC (smx, SMX);
>>>> fails += CHECK_PROC (ss, SS);
>>>> - fails += CHECK_PROC (ssbd, SSBD);
>>>> + if (cpu_features->basic.kind == arch_kind_intel)
>>>> + fails += CHECK_PROC (ssbd, SSBD);
>>>> + else if (cpu_features->basic.kind == arch_kind_amd)
>>>> + fails += CHECK_PROC (ssbd, AMD_SSBD);
>>>> fails += CHECK_PROC (sse, SSE);
>>>> fails += CHECK_PROC (sse2, SSE2);
>>>> fails += CHECK_PROC (pni, SSE3);
>>>> @@ -229,7 +244,6 @@ do_test (int argc, char **argv)
>>>> fails += CHECK_PROC (sse4_2, SSE4_2);
>>>> fails += CHECK_PROC (sse4a, SSE4A);
>>>> fails += CHECK_PROC (ssse3, SSSE3);
>>>> - fails += CHECK_PROC (stibp, STIBP);
>>>> fails += CHECK_PROC (svm, SVM);
>>>> #ifdef __x86_64__
>>>> /* NB: SYSCALL_SYSRET is 64-bit only. */
>>>> --
>>>> 2.30.2
>>>
>>> LGTM.
>>>
>>> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>>
>> I will also add the updated documentation:
>>
>> diff --git a/manual/platform.texi b/manual/platform.texi
>> index a0b204b099..bf1483fe82 100644
>> --- a/manual/platform.texi
>> +++ b/manual/platform.texi
>> @@ -659,6 +659,18 @@ XSETBV/XGETBV instructions, and XCR0.
>> @item
>> @code{XTPRUPDCTRL} -- xTPR Update Control.
>>
>> +@item
>> +@code{AMD_IBPB} -- Indirect branch predictor barrier (IBPB) for AMD cpus.
>> +
>> +@item
>> +@code{AMD_IBRS} -- Indirect branch restricted speculation (IBPB) for AMD cpus.
>> +
>> +@item
>> +@code{AMD_STIBP} -- Single thread indirect branch predictors (STIBP) for AMD cpus.
>> +
>> +@item
>> +@code{AMD_SSBD} -- Speculative Store Bypass Disable (SSBD) for AMD cpus.
>> +
>> @end itemize
>>
>> You could query if a processor supports @code{AVX} with:
>>
>
> Please sort new features by name.
Ack.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-24 12:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 14:54 [PATCH] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873) Adhemerval Zanella
2021-06-23 20:41 ` H.J. Lu
2021-06-23 21:15 ` Adhemerval Zanella
2021-06-23 21:36 ` [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled H.J. Lu
2021-06-24 0:30 ` Adhemerval Zanella
2021-06-24 0:40 ` H.J. Lu
2021-06-24 12:17 ` Adhemerval Zanella
2021-06-24 12:27 ` H.J. Lu
2021-06-24 12:55 ` Adhemerval Zanella
2021-06-24 7:56 ` Florian Weimer
2021-06-24 12:07 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).