diff mbox series

x86: Copy IBT and SHSTK usable only if CET is enabled

Message ID CAMe9rOoo9=7LM7b4wfqWjwBqYVnXVS4jkC2i-Em-TXt4WB6a0A@mail.gmail.com
State Committed
Headers show
Series x86: Copy IBT and SHSTK usable only if CET is enabled | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

H.J. Lu June 23, 2021, 9:36 p.m. UTC
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?

Comments

Adhemerval Zanella June 24, 2021, 12:30 a.m. UTC | #1
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.  */
H.J. Lu June 24, 2021, 12:40 a.m. UTC | #2
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.
Florian Weimer June 24, 2021, 7:56 a.m. UTC | #3
* 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
Adhemerval Zanella June 24, 2021, 12:07 p.m. UTC | #4
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.
Adhemerval Zanella June 24, 2021, 12:17 p.m. UTC | #5
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:
H.J. Lu June 24, 2021, 12:27 p.m. UTC | #6
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.
Adhemerval Zanella June 24, 2021, 12:55 p.m. UTC | #7
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.
diff mbox series

Patch

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