A few tests that require running CPUID do so with a private
implementation of a wrapper for CPUID. This duplication of
the CPUID wrapper should be avoided but having one is also
unnecessary because of the existence of a macro that can
be used instead.
This series replaces private CPUID wrappers with calls
to the __cpuid_count() macro from cpuid.h as made available
by gcc and clang/llvm.
Cc: Dave Hansen <[email protected]>
Cc: Ram Pai <[email protected]>
Cc: Sandipan Das <[email protected]>
Cc: Florian Weimer <[email protected]>
Cc: "Desnes A. Nunes do Rosario" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thiago Jung Bauermann <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Michal Suchanek <[email protected]>
Cc: [email protected]
Cc: Chang S. Bae <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: Andy Lutomirski <[email protected]>
Reinette Chatre (3):
selftests/vm/pkeys: Use existing __cpuid_count() macro
selftests/x86/amx: Use existing __cpuid_count() macro
selftests/x86/corrupt_xstate_header: Use existing __cpuid_count()
macro
tools/testing/selftests/vm/pkey-x86.h | 22 +++---------------
tools/testing/selftests/x86/amx.c | 23 +++++--------------
.../selftests/x86/corrupt_xstate_header.c | 17 ++------------
3 files changed, 11 insertions(+), 51 deletions(-)
--
2.25.1
gcc as well as clang makes the __cpuid_count() macro available
via cpuid.h to conveniently call the CPUID instruction.
Below is a copy of the macro as found in cpuid.h:
#define __cpuid_count(level, count, a, b, c, d) \
__asm__ ("cpuid\n\t" \
: "=a" (a), "=b" (b), "=c" (c), "=d" (d) \
: "0" (level), "2" (count))
The corrupt_xstate_header test contains a local function
used as wrapper to the CPUID instruction. One difference between
the corrupt_xstate_header implementation and the macro from cpuid.h
is that the corrupt_xstate_header implementation provides the
"volatile" qualifier to the asm() call. The "volatile" qualifier
is necessary when CPUID has side effects and thus any
optimizations should be avoided. CPUID is used in the
corrupt_xstate_header test to query the system for its
XSAVE/XRSTOR support, a query without side effect, the
"volatile" qualifier is thus not required and the macro from
cpuid.h can be used instead.
Remove the duplicated wrapper to CPUID and use __cpuid_count()
from cpuid.h instead.
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Signed-off-by: Reinette Chatre <[email protected]>
---
.../selftests/x86/corrupt_xstate_header.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/x86/corrupt_xstate_header.c b/tools/testing/selftests/x86/corrupt_xstate_header.c
index ab8599c10ce5..7a877612bc98 100644
--- a/tools/testing/selftests/x86/corrupt_xstate_header.c
+++ b/tools/testing/selftests/x86/corrupt_xstate_header.c
@@ -7,6 +7,7 @@
#define _GNU_SOURCE
+#include <cpuid.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
@@ -17,25 +18,11 @@
#include <stdint.h>
#include <sys/wait.h>
-static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
- unsigned int *ecx, unsigned int *edx)
-{
- asm volatile(
- "cpuid;"
- : "=a" (*eax),
- "=b" (*ebx),
- "=c" (*ecx),
- "=d" (*edx)
- : "0" (*eax), "2" (*ecx));
-}
-
static inline int xsave_enabled(void)
{
unsigned int eax, ebx, ecx, edx;
- eax = 0x1;
- ecx = 0x0;
- __cpuid(&eax, &ebx, &ecx, &edx);
+ __cpuid_count(0x1, 0x0, eax, ebx, ecx, edx);
/* Is CR4.OSXSAVE enabled ? */
return ecx & (1U << 27);
--
2.25.1
Hi Shuah,
On 2/7/2022 10:00 AM, Shuah Khan wrote:
>
> This will work fine on newer versions of gcc/clang. However this could
> fail when mainline kselftest is used on stable releases on test rings
> and so on, especially if they have older versions of gcc/clang.
Indeed. It thus seems that kselftest has a minimal required version for
gcc/clang that is not the current mainline minimal version but the
minimal version of the oldest supported stable kernel, which is v4.9.
__cpuid_count() was added to gcc in commit:
cb0dee885cb30b4e9beeef070cf000baa7d09abe and thus available since
gcc 4.4.
Looking at Documentation/Changes or later Documentation/process/changes.rst
kernels v4.9 and v4.14 have the minimal required version of
gcc of 3.2. So this change would encounter an issue if mainline
kselftest is used to test a v4.9 or v4.14 kernel on a system that only
supports its minimal gcc.
Kernel v4.19 moved the gcc minimal required version to 4.6 that does
contain this macro.
There does not seem to be a minimum required version of clang/LLVM
in v4.19. The first time I see a minimal version for Clang/LLVM
for a stable kernel is in kernel v5.10 with Clang/LLVM minimal
version 10.0.1 and from what I can tell the __cpuid_count() macro
was added to Clang/LLVM in version 3.4.0
(commit 4dcb5dbb53ea4fbeab48bc6bc3c4d392361dabc1).
>
> We will have to find a solution for this. Instead of deleting the local
> define, let's keep it under ifndef __cpuid_count
>
> /usr/lib/gcc/x86_64-linux-gnu/11/include/cpuid.h
>
> #define __cpuid_count(level, count, a, b, c, d) \
> __asm__ __volatile__ ("cpuid\n\t" \
> : "=a" (a), "=b" (b), "=c" (c), "=d" (d) \
> : "0" (level), "2" (count))
>
Will do. I see that gcc obtained the volatile qualifier in v11.1 so I can use
the most recent macro as you have here.
Reinette
On 2/4/22 5:11 PM, Reinette Chatre wrote:
> Hi Shuah,
>
> On 2/4/2022 3:39 PM, Shuah Khan wrote:
>> On 2/4/22 12:17 PM, Reinette Chatre wrote:
>>> A few tests that require running CPUID do so with a private
>>> implementation of a wrapper for CPUID. This duplication of
>>> the CPUID wrapper should be avoided but having one is also
>>> unnecessary because of the existence of a macro that can
>>> be used instead.
>>>
>>> This series replaces private CPUID wrappers with calls
>>> to the __cpuid_count() macro from cpuid.h as made available
>>> by gcc and clang/llvm.
>>>
>>> Cc: Dave Hansen <[email protected]>
>>> Cc: Ram Pai <[email protected]>
>>> Cc: Sandipan Das <[email protected]>
>>> Cc: Florian Weimer <[email protected]>
>>> Cc: "Desnes A. Nunes do Rosario" <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Thiago Jung Bauermann <[email protected]>
>>> Cc: Michael Ellerman <[email protected]>
>>> Cc: Michal Suchanek <[email protected]>
>>> Cc: [email protected]
>>> Cc: Chang S. Bae <[email protected]>
>>> Cc: Borislav Petkov <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> Cc: [email protected]
>>> Cc: Andy Lutomirski <[email protected]>
>>>
>>> Reinette Chatre (3):
>>> selftests/vm/pkeys: Use existing __cpuid_count() macro
>>> selftests/x86/amx: Use existing __cpuid_count() macro
>>> selftests/x86/corrupt_xstate_header: Use existing __cpuid_count()
>>> macro
>>>
>>> tools/testing/selftests/vm/pkey-x86.h | 22 +++---------------
>>> tools/testing/selftests/x86/amx.c | 23 +++++--------------
>>> .../selftests/x86/corrupt_xstate_header.c | 17 ++------------
>>> 3 files changed, 11 insertions(+), 51 deletions(-)
>>>
>>
>> I am all for this cleanup. However, I am not finding __cpuid_count()
>> marco on my system with gcc:
>>
>> gcc --version
>> gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0
>>
>> My concern is regression on older gcc versions.
>
> Please see this message from our earlier thread where you were able
> to find it on your system:
> https://lore.kernel.org/linux-kselftest/[email protected]/
>
Right. After I sent off the response, I was thinking we discussed
this before. Thanks for the refresh.
> As mentioned in that thread, on my system it arrived via user space's
> libgcc-dev package. This does not seem to be the first time including
> files from this source - I did a quick check and from what I can tell
> existing kselftest includes like stdarg.h, stdbool.h, stdatomic.h,
> unwind.h, x86intrin.h ... arrive via libgcc-dev.
>
This will work fine on newer versions of gcc/clang. However this could
fail when mainline kselftest is used on stable releases on test rings
and so on, especially if they have older versions of gcc/clang.
We will have to find a solution for this. Instead of deleting the local
define, let's keep it under ifndef __cpuid_count
/usr/lib/gcc/x86_64-linux-gnu/11/include/cpuid.h
#define __cpuid_count(level, count, a, b, c, d) \
__asm__ __volatile__ ("cpuid\n\t" \
: "=a" (a), "=b" (b), "=c" (c), "=d" (d) \
: "0" (level), "2" (count))
thanks,
-- Shuah
Hi Shuah,
On 2/4/2022 3:39 PM, Shuah Khan wrote:
> On 2/4/22 12:17 PM, Reinette Chatre wrote:
>> A few tests that require running CPUID do so with a private
>> implementation of a wrapper for CPUID. This duplication of
>> the CPUID wrapper should be avoided but having one is also
>> unnecessary because of the existence of a macro that can
>> be used instead.
>>
>> This series replaces private CPUID wrappers with calls
>> to the __cpuid_count() macro from cpuid.h as made available
>> by gcc and clang/llvm.
>>
>> Cc: Dave Hansen <[email protected]>
>> Cc: Ram Pai <[email protected]>
>> Cc: Sandipan Das <[email protected]>
>> Cc: Florian Weimer <[email protected]>
>> Cc: "Desnes A. Nunes do Rosario" <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thiago Jung Bauermann <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Michal Suchanek <[email protected]>
>> Cc: [email protected]
>> Cc: Chang S. Bae <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: [email protected]
>> Cc: Andy Lutomirski <[email protected]>
>>
>> Reinette Chatre (3):
>> selftests/vm/pkeys: Use existing __cpuid_count() macro
>> selftests/x86/amx: Use existing __cpuid_count() macro
>> selftests/x86/corrupt_xstate_header: Use existing __cpuid_count()
>> macro
>>
>> tools/testing/selftests/vm/pkey-x86.h | 22 +++---------------
>> tools/testing/selftests/x86/amx.c | 23 +++++--------------
>> .../selftests/x86/corrupt_xstate_header.c | 17 ++------------
>> 3 files changed, 11 insertions(+), 51 deletions(-)
>>
>
> I am all for this cleanup. However, I am not finding __cpuid_count()
> marco on my system with gcc:
>
> gcc --version
> gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0
>
> My concern is regression on older gcc versions.
Please see this message from our earlier thread where you were able
to find it on your system:
https://lore.kernel.org/linux-kselftest/[email protected]/
As mentioned in that thread, on my system it arrived via user space's
libgcc-dev package. This does not seem to be the first time including
files from this source - I did a quick check and from what I can tell
existing kselftest includes like stdarg.h, stdbool.h, stdatomic.h,
unwind.h, x86intrin.h ... arrive via libgcc-dev.
Reinette