diff mbox series

V2 [PATCH] x86: Adjust tst-cpu-features-supports.c for GCC 11

Message ID CAMe9rOqw_bWFjXYj-sLQPGZXQhjMHUigX6JgXpdsRk-FN__GOw@mail.gmail.com
State Committed
Headers
Series V2 [PATCH] x86: Adjust tst-cpu-features-supports.c for GCC 11 |

Commit Message

H.J. Lu Dec. 4, 2020, 1:07 p.m. UTC 
  On Fri, Dec 4, 2020 at 12:12 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > Set RDRAND usable if CPU supports RDRAND.  Also check HAS_CPU_FEATURE
> > instead of CPU_FEATURE_USABLE for FSGSBASE, LM and XSAVES since
> > FSGSBASE/LM require kernel support and XSAVES is supervisor-mode only.
>
> I think the commit subject is misleading because the RDRAND part is a
> glibc bug fix.  Rest looks okay to me.
>

Here is the V2 patch.  It changed IBT and SHSTK to check HAS_CPU_FEATURE
since they need OS support.

Comments

H.J. Lu Dec. 4, 2020, 1:12 p.m. UTC | #1 
On Fri, Dec 4, 2020 at 5:07 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Dec 4, 2020 at 12:12 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu via Libc-alpha:
> >
> > > Set RDRAND usable if CPU supports RDRAND.  Also check HAS_CPU_FEATURE
> > > instead of CPU_FEATURE_USABLE for FSGSBASE, LM and XSAVES since
> > > FSGSBASE/LM require kernel support and XSAVES is supervisor-mode only.
> >
> > I think the commit subject is misleading because the RDRAND part is a
> > glibc bug fix.  Rest looks okay to me.
> >
>
> Here is the V2 patch.  It changed IBT and SHSTK to check HAS_CPU_FEATURE
> since they need OS support.
>

I am checking in this.
Florian Weimer Dec. 4, 2020, 1:14 p.m. UTC | #2 
* H. J. Lu:

> On Fri, Dec 4, 2020 at 12:12 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > Set RDRAND usable if CPU supports RDRAND.  Also check HAS_CPU_FEATURE
>> > instead of CPU_FEATURE_USABLE for FSGSBASE, LM and XSAVES since
>> > FSGSBASE/LM require kernel support and XSAVES is supervisor-mode only.
>>
>> I think the commit subject is misleading because the RDRAND part is a
>> glibc bug fix.  Rest looks okay to me.
>>
>
> Here is the V2 patch.  It changed IBT and SHSTK to check HAS_CPU_FEATURE
> since they need OS support.

Patch is okay.  But I do wonder why __builtin_cpu_supports doesn't
handle this.  Based on the GCC documention, I would say it should
perform what you call a “usable” check.

Thanks,
Florian
H.J. Lu Dec. 4, 2020, 1:34 p.m. UTC | #3 
On Fri, Dec 4, 2020 at 5:14 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Dec 4, 2020 at 12:12 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu via Libc-alpha:
> >>
> >> > Set RDRAND usable if CPU supports RDRAND.  Also check HAS_CPU_FEATURE
> >> > instead of CPU_FEATURE_USABLE for FSGSBASE, LM and XSAVES since
> >> > FSGSBASE/LM require kernel support and XSAVES is supervisor-mode only.
> >>
> >> I think the commit subject is misleading because the RDRAND part is a
> >> glibc bug fix.  Rest looks okay to me.
> >>
> >
> > Here is the V2 patch.  It changed IBT and SHSTK to check HAS_CPU_FEATURE
> > since they need OS support.
>
> Patch is okay.  But I do wonder why __builtin_cpu_supports doesn't
> handle this.  Based on the GCC documention, I would say it should
> perform what you call a “usable” check.
>

There are no easy and OS independent ways to check if they are usable.  Glibc
doesn't set the usable bit for them.
diff mbox series

Patch

From e7f391efefb919a9c5ad856b504874150b82fc01 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 3 Dec 2020 15:02:44 -0800
Subject: [PATCH] x86: Adjust tst-cpu-features-supports.c for GCC 11

Check HAS_CPU_FEATURE instead of CPU_FEATURE_USABLE for FSGSBASE, IBT,
LM, SHSTK and XSAVES since FSGSBASE/IBT/SHSTK/LM require kernel support
and XSAVES is supervisor-mode only.
---
 sysdeps/x86/tst-cpu-features-supports.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sysdeps/x86/tst-cpu-features-supports.c b/sysdeps/x86/tst-cpu-features-supports.c
index bf881b531f..287cf01fbd 100644
--- a/sysdeps/x86/tst-cpu-features-supports.c
+++ b/sysdeps/x86/tst-cpu-features-supports.c
@@ -40,6 +40,11 @@  check_supports (int supports, int usable, const char *supports_name,
 #define CHECK_SUPPORTS(str, name) \
   check_supports (__builtin_cpu_supports (#str), \
 		  CPU_FEATURE_USABLE (name), \
+		  #str, "CPU_FEATURE_USABLE (" #name ")");
+
+#define CHECK_CPU_SUPPORTS(str, name) \
+  check_supports (__builtin_cpu_supports (#str), \
+		  HAS_CPU_FEATURE (name), \
 		  #str, "HAS_CPU_FEATURE (" #name ")");
 
 static int
@@ -118,7 +123,7 @@  do_test (int argc, char **argv)
   fails += CHECK_SUPPORTS (fma4, FMA4);
 #endif
 #if __GNUC_PREREQ (11, 0)
-  fails += CHECK_SUPPORTS (fsgsbase, FSGSBASE);
+  fails += CHECK_CPU_SUPPORTS (fsgsbase, FSGSBASE);
   fails += CHECK_SUPPORTS (fxsave, FXSR);
 #endif
 #if __GNUC_PREREQ (8, 0)
@@ -126,9 +131,9 @@  do_test (int argc, char **argv)
 #endif
 #if __GNUC_PREREQ (11, 0)
   fails += CHECK_SUPPORTS (hle, HLE);
-  fails += CHECK_SUPPORTS (ibt, IBT);
+  fails += CHECK_CPU_SUPPORTS (ibt, IBT);
   fails += CHECK_SUPPORTS (lahf_lm, LAHF64_SAHF64);
-  fails += CHECK_SUPPORTS (lm, LM);
+  fails += CHECK_CPU_SUPPORTS (lm, LM);
   fails += CHECK_SUPPORTS (lwp, LWP);
   fails += CHECK_SUPPORTS (lzcnt, LZCNT);
 #endif
@@ -150,7 +155,7 @@  do_test (int argc, char **argv)
   fails += CHECK_SUPPORTS (rtm, RTM);
   fails += CHECK_SUPPORTS (serialize, SERIALIZE);
   fails += CHECK_SUPPORTS (sha, SHA);
-  fails += CHECK_SUPPORTS (shstk, SHSTK);
+  fails += CHECK_CPU_SUPPORTS (shstk, SHSTK);
 #endif
   fails += CHECK_SUPPORTS (sse, SSE);
   fails += CHECK_SUPPORTS (sse2, SSE2);
@@ -180,7 +185,7 @@  do_test (int argc, char **argv)
   fails += CHECK_SUPPORTS (xsave, XSAVE);
   fails += CHECK_SUPPORTS (xsavec, XSAVEC);
   fails += CHECK_SUPPORTS (xsaveopt, XSAVEOPT);
-  fails += CHECK_SUPPORTS (xsaves, XSAVES);
+  fails += CHECK_CPU_SUPPORTS (xsaves, XSAVES);
 #endif
 
   printf ("%d differences between __builtin_cpu_supports and glibc code.\n",
-- 
2.28.0