2019-07-03 01:04:10

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] x86/fpu: Fix nofxsr regression

From: Andi Kleen <[email protected]>

Vegard Nossum reports:

The commit for this patch in mainline
(ccb18db2ab9d ("x86/fpu: Make XSAVE check ...")) causes the kernel to hang on
boot when passing the "nofxsr" option:

$ kvm -cpu host -kernel arch/x86/boot/bzImage -append "console=ttyS0 nofxsr
earlyprintk=ttyS0" -serial stdio -display none -smp 2
early console in extract_kernel
input_data: 0x0000000001dea276
input_len: 0x0000000000500704
output: 0x0000000001000000
output_len: 0x00000000012c79b4
kernel_total_size: 0x0000000000f24000
booted via startup_32()
Physical KASLR using RDRAND RDTSC...
Virtual KASLR using RDRAND RDTSC...

Decompressing Linux... Parsing ELF... Performing relocations... done.
Booting the kernel.
[..hang..]

<<<

Sebastian Siewior did the following analysis:

as a result of nofxsr we do:
[0] setup_clear_cpu_cap(X86_FEATURE_FXSR);
[1] setup_clear_cpu_cap(X86_FEATURE_FXSR_OPT);
[2] setup_clear_cpu_cap(X86_FEATURE_XMM);

the commit in question removes then XFEATURE_MASK_SSE from
`xfeatures_mask'.
Boot stops in fpu__init_cpu_xstate() / xsetbv() due to #GP:
|If an attempt is made to set XCR0[2:1] to 10b.
(from Vol. 2C).

[1] is "harmless". Dropping [2] does not fix the issue because [0]
still clears all three flags due to
| static const struct cpuid_dep cpuid_deps[] = {

| { X86_FEATURE_XMM, X86_FEATURE_FXSR },

Clearing additionally XMM2 (and adding the missing bits to
xsave_cpuid_features/xfeature_names) would boot further.
Later it crashes in raid6 while probing for AVX/2 code…

Disabling XMM+XMM2 in order get (and fixing it up for AVX+AVX2) would
give use XSAVE instead of FSAVE.
This won't work on 64bit userland because it expects SSE to be around
(and FXSR to save the SSE bits).
Even my 32bit Debian Wheezy doesn't work because it wants FXSR :)

So if it is unlikely to have XSAVE but no FXSR I would suggest to add
"fpu__xstate_clear_all_cpu_caps()" to nofxsr and behave like "nofxsr
noxsave".

<<<

Also nofxsr is useless on 64bit kernels because 64bit user space
always uses SSE2, and without FXSR there is no SSE support.

This patch:
- Makes nofxsr 32bit only
It was already documented to be 32bit only, but not implemented this
way.

- Implements Sebastian's suggestion of calling
fpu__xstate_clear_all_cpu_caps() for nofxsr to clear all depending
bits.

With this a 32bit kernel boots on qemu with nofxsr upto user space
crashing (I don't have a 32bit image that doesn't need SSE2),
and a 64bit kernel also fully boots with nofxsr (by ignoring
the option)

Fixes: ccb18db2ab9d ("x86/fpu: Make XSAVE check ...)
Reported-by: Vegard Nossum <[email protected]>
Suggested-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/fpu/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index ef0030e3fe6b..81c730af7454 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -255,7 +255,9 @@ static void __init fpu__init_parse_early_param(void)
if (cmdline_find_option_bool(boot_command_line, "no387"))
setup_clear_cpu_cap(X86_FEATURE_FPU);

- if (cmdline_find_option_bool(boot_command_line, "nofxsr")) {
+ if (!IS_ENABLED(CONFIG_64BIT) &&
+ cmdline_find_option_bool(boot_command_line, "nofxsr")) {
+ fpu__xstate_clear_all_cpu_caps();
setup_clear_cpu_cap(X86_FEATURE_FXSR);
setup_clear_cpu_cap(X86_FEATURE_FXSR_OPT);
setup_clear_cpu_cap(X86_FEATURE_XMM);
--
2.20.1