>From 8a4573b7cf3a3e49b409ba3a504934de181c259d Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Mon, 9 May 2022 07:36:34 -0700 Subject: [PATCH 1/2] x86/crash: Disable virt in core NMI crash handler to avoid double list_add Disable virtualization in crash_nmi_callback() and skip the requested NMI shootdown if a shootdown has already occurred, i.e. a callback has been registered. The NMI crash shootdown path doesn't play nice with multiple invocations, e.g. attempting to register the NMI handler multiple times will trigger a double list_add() and hang the sytem (in addition to multiple other issues). If "crash_kexec_post_notifiers" is specified on the kernel command line, panic() will invoke crash_smp_send_stop() and result in a second call to nmi_shootdown_cpus() during native_machine_emergency_restart(). Invoke the callback _before_ disabling virtualization, as the current VMCS needs to be cleared before doing VMXOFF. Note, this results in a subtle change in ordering between disabling virtualization and stopping Intel PT on the responding CPUs. While VMX and Intel PT do interact, VMXOFF and writes to MSR_IA32_RTIT_CTL do not induce faults between one another, which is all that matters when panicking. WARN if nmi_shootdown_cpus() is called a second time with anything other than the reboot path's "nop" handler, as bailing means the requested isn't being invoked. Punt true handling of multiple shootdown callbacks until there's an actual use case for doing so (beyond disabling virtualization). Extract the disabling logic to a common helper to deduplicate code, and to prepare for doing the shootdown in the emergency reboot path if SVM is supported. Note, prior to commit ed72736183c4 ("x86/reboot: Force all cpus to exit VMX root if VMX is supported), nmi_shootdown_cpus() was subtly protected against a second invocation by a cpu_vmx_enabled() check as the kdump handler would disable VMX if it ran first. Fixes: ed72736183c4 ("x86/reboot: Force all cpus to exit VMX root if VMX is supported) Cc: stable@vger.kernel.org Reported-by: Guilherme G. Piccoli Signed-off-by: Sean Christopherson --- arch/x86/include/asm/reboot.h | 1 + arch/x86/kernel/crash.c | 16 +-------------- arch/x86/kernel/reboot.c | 38 ++++++++++++++++++++++++++++++++--- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h index 04c17be9b5fd..8f2da36435a6 100644 --- a/arch/x86/include/asm/reboot.h +++ b/arch/x86/include/asm/reboot.h @@ -25,6 +25,7 @@ void __noreturn machine_real_restart(unsigned int type); #define MRR_BIOS 0 #define MRR_APM 1 +void cpu_crash_disable_virtualization(void); typedef void (*nmi_shootdown_cb)(int, struct pt_regs*); void nmi_panic_self_stop(struct pt_regs *regs); void nmi_shootdown_cpus(nmi_shootdown_cb callback); diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index e8326a8d1c5d..fe0cf83843ba 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -81,15 +81,6 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs) */ cpu_crash_vmclear_loaded_vmcss(); - /* Disable VMX or SVM if needed. - * - * We need to disable virtualization on all CPUs. - * Having VMX or SVM enabled on any CPU may break rebooting - * after the kdump kernel has finished its task. - */ - cpu_emergency_vmxoff(); - cpu_emergency_svm_disable(); - /* * Disable Intel PT to stop its logging */ @@ -148,12 +139,7 @@ void native_machine_crash_shutdown(struct pt_regs *regs) */ cpu_crash_vmclear_loaded_vmcss(); - /* Booting kdump kernel with VMX or SVM enabled won't work, - * because (among other limitations) we can't disable paging - * with the virt flags. - */ - cpu_emergency_vmxoff(); - cpu_emergency_svm_disable(); + cpu_crash_disable_virtualization(); /* * Disable Intel PT to stop its logging diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index fa700b46588e..f9543a4e9b09 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -528,9 +528,9 @@ static inline void kb_wait(void) } } -static void vmxoff_nmi(int cpu, struct pt_regs *regs) +static void nmi_shootdown_nop(int cpu, struct pt_regs *regs) { - cpu_emergency_vmxoff(); + /* Nothing to do, the NMI shootdown handler disables virtualization. */ } /* Use NMIs as IPIs to tell all CPUs to disable virtualization */ @@ -554,7 +554,7 @@ static void emergency_vmx_disable_all(void) __cpu_emergency_vmxoff(); /* Halt and exit VMX root operation on the other CPUs. */ - nmi_shootdown_cpus(vmxoff_nmi); + nmi_shootdown_cpus(nmi_shootdown_nop); } } @@ -802,6 +802,18 @@ static nmi_shootdown_cb shootdown_callback; static atomic_t waiting_for_crash_ipi; static int crash_ipi_issued; +void cpu_crash_disable_virtualization(void) +{ + /* + * Disable virtualization, i.e. VMX or SVM, so that INIT is recognized + * during reboot. VMX blocks INIT if the CPU is post-VMXON, and SVM + * blocks INIT if GIF=0. Note, CLGI #UDs if SVM isn't enabled, so it's + * easier to just disable SVM unconditionally. + */ + cpu_emergency_vmxoff(); + cpu_emergency_svm_disable(); +} + static int crash_nmi_callback(unsigned int val, struct pt_regs *regs) { int cpu; @@ -819,6 +831,12 @@ static int crash_nmi_callback(unsigned int val, struct pt_regs *regs) shootdown_callback(cpu, regs); + /* + * Prepare the CPU for reboot _after_ invoking the callback so that the + * callback can safely use virtualization instructions, e.g. VMCLEAR. + */ + cpu_crash_disable_virtualization(); + atomic_dec(&waiting_for_crash_ipi); /* Assume hlt works */ halt(); @@ -840,6 +858,20 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback) unsigned long msecs; local_irq_disable(); + /* + * Invoking multiple callbacks is not currently supported, registering + * the NMI handler twice will cause a list_add() double add BUG(). + * The exception is the "nop" handler in the emergency reboot path, + * which can run after e.g. kdump's shootdown. Do nothing if the crash + * handler has already run, i.e. has already prepared other CPUs, the + * reboot path doesn't have any work of its to do, it just needs to + * ensure all CPUs have prepared for reboot. + */ + if (shootdown_callback) { + WARN_ON_ONCE(callback != nmi_shootdown_nop); + return; + } + /* Make a note of crashing cpu. Will be used in NMI callback. */ crashing_cpu = safe_smp_processor_id(); base-commit: 2764011106d0436cb44702cfb0981339d68c3509 -- 2.36.0.512.ge40c2bad7a-goog