x86/fpu: Fix irq_fpu_usable() to return false during CPU onlining
Subsystems (2)
PATCH v2 1/1 x86/fpu: Fix irq_fpu_usable() to return false during CPU onlining
From: Eric Biggers <[email protected]>
irq_fpu_usable() incorrectly returned true before the FPU is initialized. The x86 CPU onlining code can call sha256() to checksum AMD microcode images, before the FPU is initialized. Since sha256() recently gained a kernel-mode FPU optimized code path, a crash occurred in kernel_fpu_begin_mask() during hotplug CPU onlining.
(The crash did not occur during boot-time CPU onlining, since the optimized sha256() code is not enabled until subsys_initcalls run.)
Fix this by making irq_fpu_usable() return false before fpu__init_cpu() has run. To do this without adding any additional overhead to irq_fpu_usable(), replace the existing per-CPU bool in_kernel_fpu with kernel_fpu_allowed which tracks both initialization and usage rather than just usage. The initial state is false; FPU initialization sets it to true; kernel-mode FPU sections toggle it to false and then back to true; and CPU offlining restores it to the initial state of false.
---
v2:
- Add WARN_ON_FPU when kernel_fpu_allowed has unexpected value in fpu__init_cpu() or cpu_disable_common(). Since WARN_ON_FPU is defined in fpu/internal.h which should not be included by smpboot.c, made cpu_disable_common() call a new function fpu__disable_cpu().
- Added Tested-by.
arch/x86/include/asm/fpu/api.h | 1 + arch/x86/kernel/fpu/core.c | 34 +++++++++++++++++++++------------- arch/x86/kernel/fpu/init.c | 13 +++++++++++++ arch/x86/kernel/fpu/internal.h | 2 ++ arch/x86/kernel/smpboot.c | 6 ++++++ 5 files changed, 43 insertions(+), 13 deletions(-)
arch/x86/include/asm/fpu/api.h +1 -0
@@ -116,10 +116,11 @@ extern void fpu_reset_from_exception_fixup(void);/* Boot, hotplug and resume */ extern void fpu__init_cpu (void );extern void fpu__init_system (void );extern void fpu__init_check_bugs (void );extern void fpu__resume_cpu (void ); +extern void fpu__disable_cpu (void );#ifdef CONFIG_MATH_EMULATION extern void fpstate_init_soft (struct swregs_state *soft);#else static inline void fpstate_init_soft (struct swregs_state *soft) {}
arch/x86/kernel/fpu/core.c +21 -13
@@ -42,12 +42,15 @@ struct fpu_state_config fpu_user_cfg __ro_after_init;* Represents the initial FPU state. It's mostly (but not completely) zeroes, * depending on the FPU hardware format: */ struct fpstate init_fpstate__ro_after_init ; -/* Track in-kernel FPU usage */ -static DEFINE_PER_CPU(bool, in_kernel_fpu); +/* +* Track FPU initialization and kernel-mode usage. 'true' means the FPU is +* initialized and is not currently being used by the kernel: +*/ +DEFINE_PER_CPU (bool, kernel_fpu_allowed);/* * Track which context is using the FPU on the CPU: */ DEFINE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
@@ -70,19 +73,22 @@ bool irq_fpu_usable(void) {if (WARN_ON_ONCE (in_nmi ()))return false;/* -* In kernel FPU usage already active? This detects any explicitly -* nested usage in task or softirq context, which is unsupported. It -* also detects attempted usage in a hardirq that has interrupted a -* kernel-mode FPU section. +* Return false in the following cases: +* +* - FPU is not yet initialized. This can happen only when the call is +* coming from CPU onlining, for example for microcode checksumming. +* - The kernel is already using the FPU, either because of explicit +* nesting (which should never be done), or because of implicit +* nesting when a hardirq interrupted a kernel-mode FPU section. +* +* The single boolean check below handles both cases: */ -if (this_cpu_read (in_kernel_fpu)) { -WARN_ON_FPU (!in_hardirq ()); +if (!this_cpu_read (kernel_fpu_allowed))return false; - }/* * When not in NMI or hard interrupt context, FPU can be used in: * * - Task context except from within fpregs_lock()'ed critical
@@ -437,13 +443,14 @@ void kernel_fpu_begin_mask(unsigned int kfpu_mask) {if (!irqs_disabled ())fpregs_lock ();WARN_ON_FPU (!irq_fpu_usable ()); -WARN_ON_FPU (this_cpu_read (in_kernel_fpu)); -this_cpu_write (in_kernel_fpu, true); +/* Toggle kernel_fpu_allowed to false: */ +WARN_ON_FPU (!this_cpu_read (kernel_fpu_allowed)); +this_cpu_write (kernel_fpu_allowed, false);if (!(current->flags & (PF_KTHREAD |PF_USER_WORKER )) && !test_thread_flag (TIF_NEED_FPU_LOAD )) {set_thread_flag (TIF_NEED_FPU_LOAD );save_fpregs_to_fpstate (x86_task_fpu (current));
@@ -459,13 +466,14 @@ void kernel_fpu_begin_mask(unsigned int kfpu_mask) }EXPORT_SYMBOL_GPL (kernel_fpu_begin_mask);void kernel_fpu_end (void ) { -WARN_ON_FPU (!this_cpu_read (in_kernel_fpu)); +/* Toggle kernel_fpu_allowed back to true: */ +WARN_ON_FPU (this_cpu_read (kernel_fpu_allowed)); +this_cpu_write (kernel_fpu_allowed, true); -this_cpu_write (in_kernel_fpu, false);if (!irqs_disabled ())fpregs_unlock (); }EXPORT_SYMBOL_GPL (kernel_fpu_end);
arch/x86/kernel/fpu/init.c +13 -0
@@ -49,10 +49,23 @@ static void fpu__init_cpu_generic(void)*/ void fpu__init_cpu (void ) {fpu__init_cpu_generic ();fpu__init_cpu_xstate (); + +/* Start allowing kernel-mode FPU: */ +WARN_ON_FPU (this_cpu_read (kernel_fpu_allowed)); +this_cpu_write (kernel_fpu_allowed, true); +} + +/* +* Stop allowing kernel-mode FPU. Called when a CPU is brought offline: +*/ +void fpu__disable_cpu (void ) +{ +WARN_ON_FPU (!this_cpu_read (kernel_fpu_allowed)); +this_cpu_write (kernel_fpu_allowed, false); }static bool __init fpu__probe_without_cpuid (void ) {unsigned long cr0;
arch/x86/kernel/fpu/internal.h +2 -0
@@ -2,10 +2,12 @@#ifndef __X86_KERNEL_FPU_INTERNAL_H#define __X86_KERNEL_FPU_INTERNAL_Hextern struct fpstate init_fpstate; +DECLARE_PER_CPU (bool, kernel_fpu_allowed); +/* CPU feature check wrappers */ static __always_inline __pure bool use_xsave(void) {return cpu_feature_enabled (X86_FEATURE_XSAVE ); }
arch/x86/kernel/smpboot.c +6 -0
@@ -1186,10 +1186,16 @@ void cpu_disable_common(void) {int cpu =smp_processor_id ();remove_siblinginfo (cpu); +/* +* Stop allowing kernel-mode FPU. This is needed so that if the CPU is +* brought online again, the initial state is not allowed: +*/ +fpu__disable_cpu (); +/* It's now safe to remove this processor from the online map */ lock_vector_lock ();remove_cpu_from_maps (cpu);unlock_vector_lock ();fixup_irqs ();
base-commit: 3ee84e3dd88e39b55b534e17a7b9a181f1d46809
Signature · 1 line
2.49.0