x86/boot: Delay BSP init until after FPU initialization
diff mbox series

Message ID 20200920010310.10961-1-mh@glandium.org
State New, archived
Headers show
Series
  • x86/boot: Delay BSP init until after FPU initialization
Related show

Commit Message

Mike Hommey Sept. 20, 2020, 1:03 a.m. UTC
FPU initialization handles the clearcpuid command line argument. If it
comes after BSP init, clearcpuid cannot be used to disable features that
trigger some parts of the BSP init code.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 arch/x86/kernel/cpu/common.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

I was trying to use clearcpuid=440 to disable X86_FEATURES_AMD_SSBD to
reproduce the behavior that happens on Zen/Zen+ on a Zen2 machine, but
that didn't work because the command line is handled after the setup for
X86_FEATURE_LS_CFG_SSBD.

I tought about either moving the command line handling earlier, but it
seems there wasn't a specific reason for BSP init being earlier than FPU
initialization so I went with reordering those instead.

Comments

Borislav Petkov Sept. 20, 2020, 8:36 a.m. UTC | #1
On Sun, Sep 20, 2020 at 10:03:10AM +0900, Mike Hommey wrote:
> FPU initialization handles the clearcpuid command line argument. If it
> comes after BSP init, clearcpuid cannot be used to disable features that
> trigger some parts of the BSP init code.
> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  arch/x86/kernel/cpu/common.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> I was trying to use clearcpuid=440 to disable X86_FEATURES_AMD_SSBD to
> reproduce the behavior that happens on Zen/Zen+ on a Zen2 machine, but
> that didn't work because the command line is handled after the setup for
> X86_FEATURE_LS_CFG_SSBD.
> 
> I tought about either moving the command line handling earlier, but it
> seems there wasn't a specific reason for BSP init being earlier than FPU
> initialization so I went with reordering those instead.

Our boot order is fragile and the functionality in
fpu__init_parse_early_param() which does the clearcpuid= parsing should be
independent from FPU, as your use case shows.

So I'd prefer if you moved that function perhaps to right after the call

  setup_force_cpu_cap(X86_FEATURE_CPUID);

in early_identify_cpu() and renamed it to something generic instead.

Thx.

Patch
diff mbox series

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index c5d6f17d9b9d..c3bbca91a14b 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1261,9 +1261,6 @@  static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
 		c->cpu_index = 0;
 		filter_cpuid_features(c, false);
-
-		if (this_cpu->c_bsp_init)
-			this_cpu->c_bsp_init(c);
 	} else {
 		setup_clear_cpu_cap(X86_FEATURE_CPUID);
 	}
@@ -1276,6 +1273,10 @@  static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
 	fpu__init_system(c);
 
+	if (have_cpuid_p()) {
+		if (this_cpu->c_bsp_init)
+			this_cpu->c_bsp_init(c);
+	}
 #ifdef CONFIG_X86_32
 	/*
 	 * Regardless of whether PCID is enumerated, the SDM says