CPUID and FPU fixes

From: Borislav Petkov <[email protected]>

Add a synthetic CPUID flag denoting whether the CPU sports the CPUID instruction or not. This will come useful later when accomodating CPUID-less CPUs.

Signed-off-by Borislav Petkov <[email protected]>

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/include/asm/cpufeatures.h | 2 +-
 arch/x86/kernel/cpu/common.c       | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

arch/x86/include/asm/cpufeatures.h +1 -1

index a4f9aee62217..e6be43b2679a 100644
@@ -100,7 +100,7 @@
 #define X86_FEATURE_XTOPOLOGY	( 3*32+22) /* cpu topology enum extensions */
 #define X86_FEATURE_TSC_RELIABLE ( 3*32+23) /* TSC is known to be reliable */
 #define X86_FEATURE_NONSTOP_TSC	( 3*32+24) /* TSC does not stop in C states */
-/* free, was #define X86_FEATURE_CLFLUSH_MONITOR ( 3*32+25) * "" clflush reqd with monitor */
+#define X86_FEATURE_CPUID	( 3*32+25) /* CPU has CPUID instruction itself */
 #define X86_FEATURE_EXTD_APICID	( 3*32+26) /* has extended APICID (8 bits) */
 #define X86_FEATURE_AMD_DCM     ( 3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF	( 3*32+28) /* APERFMPERF */

arch/x86/kernel/cpu/common.c +1 -3

index cc9e980c68ec..37f031f39e27 100644
@@ -800,14 +800,12 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
 	c->extended_cpuid_level = 0;
 
-	if (!have_cpuid_p())
-		identify_cpu_without_cpuid(c);
-
 	/* cyrix could have cpuid enabled via c_identify()*/
 	if (have_cpuid_p()) {
 		cpu_detect(c);
 		get_cpu_vendor(c);
 		get_cpu_cap(c);
+		setup_force_cpu_cap(X86_FEATURE_CPUID);
 
 		if (this_cpu->c_early_init)
 			this_cpu->c_early_init(c);

There are multiple call sites that apply forced CPU caps. Factor them into a helper.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/cpu/common.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

arch/x86/kernel/cpu/common.c +0 -0

index 37f031f39e27..347ae0a19380 100644
@@ -655,6 +655,17 @@ void cpu_detect(struct cpuinfo_x86 *c)
 	}
 }

arch/x86/kernel/cpu/common.c +1 -0

+}

Calling get_cpu_cap() will reset a bunch of CPU features. This will cause the system to lose track of force-set and force-cleared featueres in the words that are reset until the end of CPU initialization. This can cause X86_FEATURE_FPU, for example, to change back and forth during boot and potentially confuse CPU setup.

To minimize the chance of confusion, re-apply forced caps every time get_cpu_cap() is called.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/cpu/common.c | 7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/kernel/cpu/common.c +0 -0

index 347ae0a19380..24e1e4004d42 100644

That message isn't at all clear -- what does "Legacy x87" even mean?

Clarify it. If there's no FPU, say "x86/fpu: No FPU detected". If there's an FPU that doesn't have XSAVE, say "x86/fpu: Pre-XSAVE x87 FPU detected".

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/fpu/xstate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

arch/x86/kernel/fpu/xstate.c +0 -0

index 1d7770447b3e..2d592b1c75e4 100644
@@ -705,8 +705,13 @@ void __init fpu__init_system_xstate(void)
 	WARN_ON_FPU(!on_boot_cpu);

The old code didn't work at all because it adjusted the current caps instead of the forced caps. Anything it did would be undone later during cpu identification. Fix that and, while we're at it, improve the logging and don't bother running it if CPUID is available.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/fpu/init.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

arch/x86/kernel/fpu/init.c +0 -0

index 60dece392b3a..75e1bf3b0319 100644
@@ -50,29 +50,33 @@ void fpu__init_cpu(void)
 
 /*
  * The earliest FPU detection code.

arch/x86/kernel/fpu/init.c +6 -4

+		/*
+		 * Set the X86_FEATURE_FPU CPU-capability bit based on
+		 * trying to execute an actual sequence of FPU instructions:
+		 */
 
-	fsw = fcw = 0xffff;
+		unsigned long cr0;
+		u16 fsw, fcw;
 
-	cr0 = read_cr0();
-	cr0 &= ~(X86_CR0_TS | X86_CR0_EM);
-	write_cr0(cr0);

Quoted 16 lines · Andy Lutomirski

The old code didn't work at all because it adjusted the current caps instead of the forced caps. Anything it did would be undone later during cpu identification. Fix that and, while we're at it, improve the logging and don't bother running it if CPUID is available.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/fpu/init.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

arch/x86/kernel/fpu/init.c +0 -0

index 60dece392b3a..75e1bf3b0319 100644
@@ -50,29 +50,33 @@ void fpu__init_cpu(void)
 

arch/x86/kernel/fpu/init.c void fpu__init_cpu(void) · line 57

Quoted 12 lines

arch/x86/kernel/fpu/init.c +2 -5

 /*
  * The earliest FPU detection code.
- *
- * Set the X86_FEATURE_FPU CPU-capability bit based on
- * trying to execute an actual sequence of FPU instructions:
  */
 static void fpu__init_system_early_generic(struct cpuinfo_x86 *c)
 {
-     unsigned long cr0;
-     u16 fsw, fcw;
+     if (!boot_cpu_has(X86_FEATURE_CPUID) &&
+         !test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {

Flip test and save an indentation level.

Quoted 19 lines

arch/x86/kernel/fpu/init.c +11 -4

+             /*
+              * Set the X86_FEATURE_FPU CPU-capability bit based on
+              * trying to execute an actual sequence of FPU instructions:
+              */
 
-     fsw = fcw = 0xffff;
+             unsigned long cr0;
+             u16 fsw, fcw;
 
-     cr0 = read_cr0();
-     cr0 &= ~(X86_CR0_TS | X86_CR0_EM);
-     write_cr0(cr0);
+             fsw = fcw = 0xffff;
+
+             cr0 = read_cr0();
+             cr0 &= ~(X86_CR0_TS | X86_CR0_EM);
+             write_cr0(cr0);
 

arch/x86/kernel/fpu/init.c void fpu__init_cpu(void) · line 74

Quoted 4 lines

arch/x86/kernel/fpu/init.c +1 -1

-     if (!test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {
              asm volatile("fninit ; fnstsw %0 ; fnstcw %1"
                           : "+m" (fsw), "+m" (fcw));
+             pr_info("x86/fpu: FSW=0x%04hx FCW=0x%04hx\n", fsw, fcw);

Why do we want those in dmesg? Maybe KERN_DEBUG?

Quoted 18 lines · Andy Lutomirski

That message isn't at all clear -- what does "Legacy x87" even mean?

Clarify it. If there's no FPU, say "x86/fpu: No FPU detected". If there's an FPU that doesn't have XSAVE, say "x86/fpu: Pre-XSAVE x87 FPU detected".

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/fpu/xstate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

arch/x86/kernel/fpu/xstate.c +0 -0

index 1d7770447b3e..2d592b1c75e4 100644
@@ -705,8 +705,13 @@ void __init fpu__init_system_xstate(void)
      WARN_ON_FPU(!on_boot_cpu);

arch/x86/kernel/fpu/xstate.c void __init fpu__init_system_xstate(void) · line 714

Quoted 10 lines

arch/x86/kernel/fpu/xstate.c +6 -1

      on_boot_cpu = 0;
 
+     if (!boot_cpu_has(X86_FEATURE_FPU)) {
+             pr_info("x86/fpu: No FPU detected.\n");
+             return;
+     }
+
      if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
-             pr_info("x86/fpu: Legacy x87 FPU detected.\n");
+             pr_info("x86/fpu: Pre-XSAVE x87 FPU detected.\n");

Well, there's also FXSAVE and FSAVE. The legacy thing kinda made sense to me here.

Maybe say something like this:

pr_info("x86/fpu: FPU detected: FSAVE|FXSAVE|XSAVE support\n", ...

and use X86_FEATURE_XSAVE and X86_FEATURE_FXSR to print the respective *SAVE string...? FSAVE we issue by default.

Hmm.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

I did something like this for FSAVE vs FXSAVE. XSAVE has its own pile of printouts and I don't feel like I need to add another.

How? There's that bit at the bottom to worry about.

I kind of line the idea of being able to ask users of these ancient CPUs to just send in their logs.