[PATCH v3 00/18] x86/reboot: KVM: Clean up "emergency" virt code

[PATCH v3 00/18] x86/reboot: KVM: Clean up "emergency" virt code

[Sean Christopherson]

Instead of having the reboot code blindly try to disable virtualization
during an emergency, use the existing callback into KVM to disable virt
as "needed".  In quotes because KVM still somewhat blindly attempts to
disable virt, e.g. if KVM is loaded but doesn't have active VMs and thus
hasn't enabled hardware.  That could theoretically be "fixed", but due to
the callback being invoked from NMI context, I'm not convinced it would
be worth the complexity.  E.g. false positives would still be possible,
and KVM would have to play games with the per-CPU hardware_enabled flag
to ensure there are no false negatives.

The callback is currently used only to VMCLEAR the per-CPU list of VMCSes,
but not using the callback to disable virt isn't intentional.  Arguably, a
callback should have been used in the initial "disable virt" code added by
commit d176720d34c7 ("x86: disable VMX on all CPUs on reboot").  And the
kexec logic added (much later) by commit f23d1f4a1160 ("x86/kexec: VMCLEAR
VMCSs loaded on all cpus if necessary") simply missed the opportunity to
use the callback for all virtualization needs.

Once KVM handles disabling virt, move all of the helpers provided by
virtext.h into KVM proper.

There's one outlier patch, "Make KVM_AMD depend on CPU_SUP_AMD or
CPU_SUP_HYGON", that I included here because it felt weird to pull in the
"must be AMD or Hygon" check without KVM demanding that at build time.

v3:
 - Massage changelogs to avoid talking about out-of-tree hypervisors. [Kai]
 - Move #ifdef "KVM" addition later. [Kai]

v2:
 - https://lore.kernel.org/all/20230310214232.806108-1-seanjc@google.com
 - Disable task migration when probing basic SVM and VMX support to avoid
   logging misleading info (wrong CPU) if probing fails.

v1: https://lore.kernel.org/all/20221201232655.290720-1-seanjc@google.com

Sean Christopherson (18):
  x86/reboot: VMCLEAR active VMCSes before emergency reboot
  x86/reboot: Harden virtualization hooks for emergency reboot
  x86/reboot: KVM: Handle VMXOFF in KVM's reboot callback
  x86/reboot: KVM: Disable SVM during reboot via virt/KVM reboot
    callback
  x86/reboot: Disable virtualization during reboot iff callback is
    registered
  x86/reboot: Assert that IRQs are disabled when turning off
    virtualization
  x86/reboot: Hoist "disable virt" helpers above "emergency reboot" path
  x86/reboot: Expose VMCS crash hooks if and only if KVM_{INTEL,AMD} is
    enabled
  x86/virt: KVM: Open code cpu_has_vmx() in KVM VMX
  x86/virt: KVM: Move VMXOFF helpers into KVM VMX
  KVM: SVM: Make KVM_AMD depend on CPU_SUP_AMD or CPU_SUP_HYGON
  x86/virt: Drop unnecessary check on extended CPUID level in
    cpu_has_svm()
  x86/virt: KVM: Open code cpu_has_svm() into kvm_is_svm_supported()
  KVM: SVM: Check that the current CPU supports SVM in
    kvm_is_svm_supported()
  KVM: VMX: Ensure CPU is stable when probing basic VMX support
  x86/virt: KVM: Move "disable SVM" helper into KVM SVM
  KVM: x86: Force kvm_rebooting=true during emergency reboot/crash
  KVM: SVM: Use "standard" stgi() helper when disabling SVM

 arch/x86/include/asm/kexec.h   |   2 -
 arch/x86/include/asm/reboot.h  |   7 ++
 arch/x86/include/asm/virtext.h | 154 ---------------------------------
 arch/x86/kernel/crash.c        |  31 -------
 arch/x86/kernel/reboot.c       |  66 ++++++++++----
 arch/x86/kvm/Kconfig           |   2 +-
 arch/x86/kvm/svm/svm.c         |  70 ++++++++++++---
 arch/x86/kvm/vmx/vmx.c         |  68 +++++++++++----
 8 files changed, 168 insertions(+), 232 deletions(-)
 delete mode 100644 arch/x86/include/asm/virtext.h


base-commit: b3c98052d46948a8d65d2778c7f306ff38366aac
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 01/18] x86/reboot: VMCLEAR active VMCSes before emergency reboot

[Sean Christopherson]

VMCLEAR active VMCSes before any emergency reboot, not just if the kernel
may kexec into a new kernel after a crash.  Per Intel's SDM, the VMX
architecture doesn't require the CPU to flush the VMCS cache on INIT.  If
an emergency reboot doesn't RESET CPUs, cached VMCSes could theoretically
be kept and only be written back to memory after the new kernel is booted,
i.e. could effectively corrupt memory after reboot.

Opportunistically remove the setting of the global pointer to NULL to make
checkpatch happy.

Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kexec.h  |  2 --
 arch/x86/include/asm/reboot.h |  2 ++
 arch/x86/kernel/crash.c       | 31 -------------------------------
 arch/x86/kernel/reboot.c      | 22 ++++++++++++++++++++++
 arch/x86/kvm/vmx/vmx.c        | 10 +++-------
 5 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index a3760ca796aa..256eee99afc8 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -208,8 +208,6 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image);
 #endif
 #endif
 
-typedef void crash_vmclear_fn(void);
-extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
 extern void kdump_nmi_shootdown_cpus(void);
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index bc5b4d788c08..2551baec927d 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -25,6 +25,8 @@ void __noreturn machine_real_restart(unsigned int type);
 #define MRR_BIOS	0
 #define MRR_APM		1
 
+typedef void crash_vmclear_fn(void);
+extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
 void cpu_emergency_disable_virtualization(void);
 
 typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index cdd92ab43cda..54cd959cb316 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -48,38 +48,12 @@ struct crash_memmap_data {
 	unsigned int type;
 };
 
-/*
- * This is used to VMCLEAR all VMCSs loaded on the
- * processor. And when loading kvm_intel module, the
- * callback function pointer will be assigned.
- *
- * protected by rcu.
- */
-crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss = NULL;
-EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
-
-static inline void cpu_crash_vmclear_loaded_vmcss(void)
-{
-	crash_vmclear_fn *do_vmclear_operation = NULL;
-
-	rcu_read_lock();
-	do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
-	if (do_vmclear_operation)
-		do_vmclear_operation();
-	rcu_read_unlock();
-}
-
 #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
 
 static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
 {
 	crash_save_cpu(regs, cpu);
 
-	/*
-	 * VMCLEAR VMCSs loaded on all cpus if needed.
-	 */
-	cpu_crash_vmclear_loaded_vmcss();
-
 	/*
 	 * Disable Intel PT to stop its logging
 	 */
@@ -133,11 +107,6 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
 
 	crash_smp_send_stop();
 
-	/*
-	 * VMCLEAR VMCSs loaded on this cpu if needed.
-	 */
-	cpu_crash_vmclear_loaded_vmcss();
-
 	cpu_emergency_disable_virtualization();
 
 	/*
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index d03c551defcc..299b970e5f82 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -787,6 +787,26 @@ void machine_crash_shutdown(struct pt_regs *regs)
 }
 #endif
 
+/*
+ * This is used to VMCLEAR all VMCSs loaded on the
+ * processor. And when loading kvm_intel module, the
+ * callback function pointer will be assigned.
+ *
+ * protected by rcu.
+ */
+crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
+EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
+
+static inline void cpu_crash_vmclear_loaded_vmcss(void)
+{
+	crash_vmclear_fn *do_vmclear_operation = NULL;
+
+	rcu_read_lock();
+	do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
+	if (do_vmclear_operation)
+		do_vmclear_operation();
+	rcu_read_unlock();
+}
 
 /* This is the CPU performing the emergency shutdown work. */
 int crashing_cpu = -1;
@@ -798,6 +818,8 @@ int crashing_cpu = -1;
  */
 void cpu_emergency_disable_virtualization(void)
 {
+	cpu_crash_vmclear_loaded_vmcss();
+
 	cpu_emergency_vmxoff();
 	cpu_emergency_svm_disable();
 }
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 44fb619803b8..317f72baf0c3 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -41,7 +41,7 @@
 #include <asm/idtentry.h>
 #include <asm/io.h>
 #include <asm/irq_remapping.h>
-#include <asm/kexec.h>
+#include <asm/reboot.h>
 #include <asm/perf_event.h>
 #include <asm/mmu_context.h>
 #include <asm/mshyperv.h>
@@ -744,7 +744,6 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
 	return ret;
 }
 
-#ifdef CONFIG_KEXEC_CORE
 static void crash_vmclear_local_loaded_vmcss(void)
 {
 	int cpu = raw_smp_processor_id();
@@ -754,7 +753,6 @@ static void crash_vmclear_local_loaded_vmcss(void)
 			    loaded_vmcss_on_cpu_link)
 		vmcs_clear(v->vmcs);
 }
-#endif /* CONFIG_KEXEC_CORE */
 
 static void __loaded_vmcs_clear(void *arg)
 {
@@ -8549,10 +8547,9 @@ static void __vmx_exit(void)
 {
 	allow_smaller_maxphyaddr = false;
 
-#ifdef CONFIG_KEXEC_CORE
 	RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
 	synchronize_rcu();
-#endif
+
 	vmx_cleanup_l1d_flush();
 }
 
@@ -8601,10 +8598,9 @@ static int __init vmx_init(void)
 		pi_init_cpu(cpu);
 	}
 
-#ifdef CONFIG_KEXEC_CORE
 	rcu_assign_pointer(crash_vmclear_loaded_vmcss,
 			   crash_vmclear_local_loaded_vmcss);
-#endif
+
 	vmx_check_vmcs12_offsets();
 
 	/*
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 02/18] x86/reboot: Harden virtualization hooks for emergency reboot

[Sean Christopherson]

Provide dedicated helpers to (un)register virt hooks used during an
emergency crash/reboot, and WARN if there is an attempt to overwrite
the registered callback, or an attempt to do an unpaired unregister.

Opportunsitically use rcu_assign_pointer() instead of RCU_INIT_POINTER(),
mainly so that the set/unset paths are more symmetrical, but also because
any performance gains from using RCU_INIT_POINTER() are meaningless for
this code.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/reboot.h |  5 +++--
 arch/x86/kernel/reboot.c      | 30 ++++++++++++++++++++++++------
 arch/x86/kvm/vmx/vmx.c        |  6 ++----
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index 2551baec927d..d9a38d379d18 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -25,8 +25,9 @@ void __noreturn machine_real_restart(unsigned int type);
 #define MRR_BIOS	0
 #define MRR_APM		1
 
-typedef void crash_vmclear_fn(void);
-extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
+typedef void (cpu_emergency_virt_cb)(void);
+void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback);
+void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback);
 void cpu_emergency_disable_virtualization(void);
 
 typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 299b970e5f82..739e09527dbb 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -794,17 +794,35 @@ void machine_crash_shutdown(struct pt_regs *regs)
  *
  * protected by rcu.
  */
-crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
-EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
+static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
+
+void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
+{
+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
+		return;
+
+	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
+}
+EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
+
+void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
+{
+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
+		return;
+
+	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
 
 static inline void cpu_crash_vmclear_loaded_vmcss(void)
 {
-	crash_vmclear_fn *do_vmclear_operation = NULL;
+	cpu_emergency_virt_cb *callback;
 
 	rcu_read_lock();
-	do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
-	if (do_vmclear_operation)
-		do_vmclear_operation();
+	callback = rcu_dereference(cpu_emergency_virt_callback);
+	if (callback)
+		callback();
 	rcu_read_unlock();
 }
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 317f72baf0c3..fc9cdb4114cc 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -8547,8 +8547,7 @@ static void __vmx_exit(void)
 {
 	allow_smaller_maxphyaddr = false;
 
-	RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
-	synchronize_rcu();
+	cpu_emergency_unregister_virt_callback(crash_vmclear_local_loaded_vmcss);
 
 	vmx_cleanup_l1d_flush();
 }
@@ -8598,8 +8597,7 @@ static int __init vmx_init(void)
 		pi_init_cpu(cpu);
 	}
 
-	rcu_assign_pointer(crash_vmclear_loaded_vmcss,
-			   crash_vmclear_local_loaded_vmcss);
+	cpu_emergency_register_virt_callback(crash_vmclear_local_loaded_vmcss);
 
 	vmx_check_vmcs12_offsets();
 
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 03/18] x86/reboot: KVM: Handle VMXOFF in KVM's reboot callback

[Sean Christopherson]

Use KVM VMX's reboot/crash callback to do VMXOFF in an emergency instead
of manually and blindly doing VMXOFF.  There's no need to attempt VMXOFF
if a hypervisor, i.e. KVM, isn't loaded/active, i.e. if the CPU can't
possibly be post-VMXON.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 10 ----------
 arch/x86/kernel/reboot.c       | 29 +++++++++--------------------
 arch/x86/kvm/vmx/vmx.c         |  8 +++++---
 3 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index 3b12e6b99412..5bc29fab15da 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -70,16 +70,6 @@ static inline void __cpu_emergency_vmxoff(void)
 		cpu_vmxoff();
 }
 
-/** Disable VMX if it is supported and enabled on the current CPU
- */
-static inline void cpu_emergency_vmxoff(void)
-{
-	if (cpu_has_vmx())
-		__cpu_emergency_vmxoff();
-}
-
-
-
 
 /*
  * SVM functions:
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 739e09527dbb..0cf2261c2dec 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -787,13 +787,7 @@ void machine_crash_shutdown(struct pt_regs *regs)
 }
 #endif
 
-/*
- * This is used to VMCLEAR all VMCSs loaded on the
- * processor. And when loading kvm_intel module, the
- * callback function pointer will be assigned.
- *
- * protected by rcu.
- */
+/* RCU-protected callback to disable virtualization prior to reboot. */
 static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
 
 void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
@@ -815,17 +809,6 @@ void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
 }
 EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
 
-static inline void cpu_crash_vmclear_loaded_vmcss(void)
-{
-	cpu_emergency_virt_cb *callback;
-
-	rcu_read_lock();
-	callback = rcu_dereference(cpu_emergency_virt_callback);
-	if (callback)
-		callback();
-	rcu_read_unlock();
-}
-
 /* This is the CPU performing the emergency shutdown work. */
 int crashing_cpu = -1;
 
@@ -836,9 +819,15 @@ int crashing_cpu = -1;
  */
 void cpu_emergency_disable_virtualization(void)
 {
-	cpu_crash_vmclear_loaded_vmcss();
+	cpu_emergency_virt_cb *callback;
 
-	cpu_emergency_vmxoff();
+	rcu_read_lock();
+	callback = rcu_dereference(cpu_emergency_virt_callback);
+	if (callback)
+		callback();
+	rcu_read_unlock();
+
+	/* KVM_AMD doesn't yet utilize the common callback. */
 	cpu_emergency_svm_disable();
 }
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fc9cdb4114cc..76cdb189f1b5 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -744,7 +744,7 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
 	return ret;
 }
 
-static void crash_vmclear_local_loaded_vmcss(void)
+static void vmx_emergency_disable(void)
 {
 	int cpu = raw_smp_processor_id();
 	struct loaded_vmcs *v;
@@ -752,6 +752,8 @@ static void crash_vmclear_local_loaded_vmcss(void)
 	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
 			    loaded_vmcss_on_cpu_link)
 		vmcs_clear(v->vmcs);
+
+	__cpu_emergency_vmxoff();
 }
 
 static void __loaded_vmcs_clear(void *arg)
@@ -8547,7 +8549,7 @@ static void __vmx_exit(void)
 {
 	allow_smaller_maxphyaddr = false;
 
-	cpu_emergency_unregister_virt_callback(crash_vmclear_local_loaded_vmcss);
+	cpu_emergency_unregister_virt_callback(vmx_emergency_disable);
 
 	vmx_cleanup_l1d_flush();
 }
@@ -8597,7 +8599,7 @@ static int __init vmx_init(void)
 		pi_init_cpu(cpu);
 	}
 
-	cpu_emergency_register_virt_callback(crash_vmclear_local_loaded_vmcss);
+	cpu_emergency_register_virt_callback(vmx_emergency_disable);
 
 	vmx_check_vmcs12_offsets();
 
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 04/18] x86/reboot: KVM: Disable SVM during reboot via virt/KVM reboot callback

[Sean Christopherson]

Use the virt callback to disable SVM (and set GIF=1) during an emergency
instead of blindly attempting to disable SVM.  Like the VMX case, if a
hypervisor, i.e. KVM, isn't loaded/active, SVM can't be in use.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h |  8 --------
 arch/x86/kernel/reboot.c       |  3 ---
 arch/x86/kvm/svm/svm.c         | 19 +++++++++++++++++--
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index 5bc29fab15da..aaed66249ccf 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -133,12 +133,4 @@ static inline void cpu_svm_disable(void)
 	}
 }
 
-/** Makes sure SVM is disabled, if it is supported on the CPU
- */
-static inline void cpu_emergency_svm_disable(void)
-{
-	if (cpu_has_svm(NULL))
-		cpu_svm_disable();
-}
-
 #endif /* _ASM_X86_VIRTEX_H */
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 0cf2261c2dec..92b380e199a3 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -826,9 +826,6 @@ void cpu_emergency_disable_virtualization(void)
 	if (callback)
 		callback();
 	rcu_read_unlock();
-
-	/* KVM_AMD doesn't yet utilize the common callback. */
-	cpu_emergency_svm_disable();
 }
 
 #if defined(CONFIG_SMP)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index eb308c9994f9..0f0d04900bf2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -38,6 +38,7 @@
 #include <asm/spec-ctrl.h>
 #include <asm/cpu_device_id.h>
 #include <asm/traps.h>
+#include <asm/reboot.h>
 #include <asm/fpu/api.h>
 
 #include <asm/virtext.h>
@@ -568,6 +569,11 @@ void __svm_write_tsc_multiplier(u64 multiplier)
 	preempt_enable();
 }
 
+static void svm_emergency_disable(void)
+{
+	cpu_svm_disable();
+}
+
 static void svm_hardware_disable(void)
 {
 	/* Make sure we clean up behind us */
@@ -5184,6 +5190,13 @@ static struct kvm_x86_init_ops svm_init_ops __initdata = {
 	.pmu_ops = &amd_pmu_ops,
 };
 
+static void __svm_exit(void)
+{
+	kvm_x86_vendor_exit();
+
+	cpu_emergency_unregister_virt_callback(svm_emergency_disable);
+}
+
 static int __init svm_init(void)
 {
 	int r;
@@ -5197,6 +5210,8 @@ static int __init svm_init(void)
 	if (r)
 		return r;
 
+	cpu_emergency_register_virt_callback(svm_emergency_disable);
+
 	/*
 	 * Common KVM initialization _must_ come last, after this, /dev/kvm is
 	 * exposed to userspace!
@@ -5209,14 +5224,14 @@ static int __init svm_init(void)
 	return 0;
 
 err_kvm_init:
-	kvm_x86_vendor_exit();
+	__svm_exit();
 	return r;
 }
 
 static void __exit svm_exit(void)
 {
 	kvm_exit();
-	kvm_x86_vendor_exit();
+	__svm_exit();
 }
 
 module_init(svm_init)
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 05/18] x86/reboot: Disable virtualization during reboot iff callback is registered

[Sean Christopherson]

Attempt to disable virtualization during an emergency reboot if and only
if there is a registered virt callback, i.e. iff a hypervisor (KVM) is
active.  If there's no active hypervisor, then the CPU can't be operating
with VMX or SVM enabled (barring an egregious bug).

Note, IRQs are disabled, which prevents KVM from coming along and enabling
virtualization after the fact.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kernel/reboot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 92b380e199a3..20f7bdabc52e 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -22,7 +22,6 @@
 #include <asm/reboot_fixups.h>
 #include <asm/reboot.h>
 #include <asm/pci_x86.h>
-#include <asm/virtext.h>
 #include <asm/cpu.h>
 #include <asm/nmi.h>
 #include <asm/smp.h>
@@ -545,7 +544,7 @@ static void emergency_reboot_disable_virtualization(void)
 	 * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
 	 * other CPUs may have virtualization enabled.
 	 */
-	if (cpu_has_vmx() || cpu_has_svm(NULL)) {
+	if (rcu_access_pointer(cpu_emergency_virt_callback)) {
 		/* Safely force _this_ CPU out of VMX/SVM operation. */
 		cpu_emergency_disable_virtualization();
 
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 06/18] x86/reboot: Assert that IRQs are disabled when turning off virtualization

[Sean Christopherson]

Assert that IRQs are disabled when turning off virtualization in an
emergency.  KVM enables hardware via on_each_cpu(), i.e. could re-enable
hardware if a pending IPI were delivered after disabling virtualization.

Remove a misleading comment from emergency_reboot_disable_virtualization()
about "just" needing to guarantee the CPU is stable (see above).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kernel/reboot.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 20f7bdabc52e..fddfea5f1d20 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -531,7 +531,6 @@ static inline void nmi_shootdown_cpus_on_restart(void);
 
 static void emergency_reboot_disable_virtualization(void)
 {
-	/* Just make sure we won't change CPUs while doing this */
 	local_irq_disable();
 
 	/*
@@ -820,6 +819,13 @@ void cpu_emergency_disable_virtualization(void)
 {
 	cpu_emergency_virt_cb *callback;
 
+	/*
+	 * IRQs must be disabled as KVM enables virtualization in hardware via
+	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
+	 * virtualization stays disabled.
+	 */
+	lockdep_assert_irqs_disabled();
+
 	rcu_read_lock();
 	callback = rcu_dereference(cpu_emergency_virt_callback);
 	if (callback)
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 07/18] x86/reboot: Hoist "disable virt" helpers above "emergency reboot" path

[Sean Christopherson]

Move the various "disable virtualization" helpers above the emergency
reboot path so that emergency_reboot_disable_virtualization() can be
stubbed out in a future patch if neither KVM_INTEL nor KVM_AMD is enabled,
i.e. if there is no in-tree user of CPU virtualization.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kernel/reboot.c | 90 ++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index fddfea5f1d20..493d66e59a4f 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -529,6 +529,51 @@ static inline void kb_wait(void)
 
 static inline void nmi_shootdown_cpus_on_restart(void);
 
+/* RCU-protected callback to disable virtualization prior to reboot. */
+static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
+
+void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
+{
+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
+		return;
+
+	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
+}
+EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
+
+void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
+{
+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
+		return;
+
+	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
+
+/*
+ * Disable virtualization, i.e. VMX or SVM, to ensure INIT is recognized during
+ * reboot.  VMX blocks INIT if the CPU is post-VMXON, and SVM blocks INIT if
+ * GIF=0, i.e. if the crash occurred between CLGI and STGI.
+ */
+void cpu_emergency_disable_virtualization(void)
+{
+	cpu_emergency_virt_cb *callback;
+
+	/*
+	 * IRQs must be disabled as KVM enables virtualization in hardware via
+	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
+	 * virtualization stays disabled.
+	 */
+	lockdep_assert_irqs_disabled();
+
+	rcu_read_lock();
+	callback = rcu_dereference(cpu_emergency_virt_callback);
+	if (callback)
+		callback();
+	rcu_read_unlock();
+}
+
 static void emergency_reboot_disable_virtualization(void)
 {
 	local_irq_disable();
@@ -785,54 +830,9 @@ void machine_crash_shutdown(struct pt_regs *regs)
 }
 #endif
 
-/* RCU-protected callback to disable virtualization prior to reboot. */
-static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
-
-void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
-{
-	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
-		return;
-
-	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
-}
-EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
-
-void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
-{
-	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
-		return;
-
-	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
-	synchronize_rcu();
-}
-EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
-
 /* This is the CPU performing the emergency shutdown work. */
 int crashing_cpu = -1;
 
-/*
- * Disable virtualization, i.e. VMX or SVM, to ensure INIT is recognized during
- * reboot.  VMX blocks INIT if the CPU is post-VMXON, and SVM blocks INIT if
- * GIF=0, i.e. if the crash occurred between CLGI and STGI.
- */
-void cpu_emergency_disable_virtualization(void)
-{
-	cpu_emergency_virt_cb *callback;
-
-	/*
-	 * IRQs must be disabled as KVM enables virtualization in hardware via
-	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
-	 * virtualization stays disabled.
-	 */
-	lockdep_assert_irqs_disabled();
-
-	rcu_read_lock();
-	callback = rcu_dereference(cpu_emergency_virt_callback);
-	if (callback)
-		callback();
-	rcu_read_unlock();
-}
-
 #if defined(CONFIG_SMP)
 
 static nmi_shootdown_cb shootdown_callback;
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 08/18] x86/reboot: Expose VMCS crash hooks if and only if KVM_{INTEL,AMD} is enabled

[Sean Christopherson]

Expose the crash/reboot hooks used by KVM to disable virtualization in
hardware and unblock INIT only if there's a potential in-tree user,
i.e. either KVM_INTEL or KVM_AMD is enabled.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/reboot.h | 4 ++++
 arch/x86/kernel/reboot.c      | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index d9a38d379d18..9cf0b21bf495 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -25,10 +25,14 @@ void __noreturn machine_real_restart(unsigned int type);
 #define MRR_BIOS	0
 #define MRR_APM		1
 
+#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
 typedef void (cpu_emergency_virt_cb)(void);
 void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback);
 void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback);
 void cpu_emergency_disable_virtualization(void);
+#else
+static inline void cpu_emergency_disable_virtualization(void) {}
+#endif /* CONFIG_KVM_INTEL || CONFIG_KVM_AMD */
 
 typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
 void nmi_panic_self_stop(struct pt_regs *regs);
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 493d66e59a4f..6818d8de85a1 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -529,6 +529,7 @@ static inline void kb_wait(void)
 
 static inline void nmi_shootdown_cpus_on_restart(void);
 
+#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
 /* RCU-protected callback to disable virtualization prior to reboot. */
 static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
 
@@ -596,7 +597,9 @@ static void emergency_reboot_disable_virtualization(void)
 		nmi_shootdown_cpus_on_restart();
 	}
 }
-
+#else
+static void emergency_reboot_disable_virtualization(void) { }
+#endif /* CONFIG_KVM_INTEL || CONFIG_KVM_AMD */
 
 void __attribute__((weak)) mach_reboot_fixups(void)
 {
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 09/18] x86/virt: KVM: Open code cpu_has_vmx() in KVM VMX

[Sean Christopherson]

Fold the raw CPUID check for VMX into kvm_is_vmx_supported(), its sole
user.  Keep the check even though KVM also checks X86_FEATURE_VMX, as the
intent is to provide a unique error message if VMX is unsupported by
hardware, whereas X86_FEATURE_VMX may be clear due to firmware and/or
kernel actions.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 10 ----------
 arch/x86/kvm/vmx/vmx.c         |  2 +-
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index aaed66249ccf..b1171a5ad452 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -22,14 +22,6 @@
 /*
  * VMX functions:
  */
-
-static inline int cpu_has_vmx(void)
-{
-	unsigned long ecx = cpuid_ecx(1);
-	return test_bit(5, &ecx); /* CPUID.1:ECX.VMX[bit 5] -> VT */
-}
-
-
 /**
  * cpu_vmxoff() - Disable VMX on the current CPU
  *
@@ -61,8 +53,6 @@ static inline int cpu_vmx_enabled(void)
 }
 
 /** Disable VMX if it is enabled on the current CPU
- *
- * You shouldn't call this if cpu_has_vmx() returns 0.
  */
 static inline void __cpu_emergency_vmxoff(void)
 {
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 76cdb189f1b5..f44f93772b4f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2721,7 +2721,7 @@ static bool kvm_is_vmx_supported(void)
 {
 	int cpu = raw_smp_processor_id();
 
-	if (!cpu_has_vmx()) {
+	if (!(cpuid_ecx(1) & feature_bit(VMX))) {
 		pr_err("VMX not supported by CPU %d\n", cpu);
 		return false;
 	}
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 10/18] x86/virt: KVM: Move VMXOFF helpers into KVM VMX

[Sean Christopherson]

Now that VMX is disabled in emergencies via the virt callbacks, move the
VMXOFF helpers into KVM, the only remaining user.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 42 ----------------------------------
 arch/x86/kvm/vmx/vmx.c         | 29 ++++++++++++++++++++---
 2 files changed, 26 insertions(+), 45 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index b1171a5ad452..a27801f2bc71 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -19,48 +19,6 @@
 #include <asm/svm.h>
 #include <asm/tlbflush.h>
 
-/*
- * VMX functions:
- */
-/**
- * cpu_vmxoff() - Disable VMX on the current CPU
- *
- * Disable VMX and clear CR4.VMXE (even if VMXOFF faults)
- *
- * Note, VMXOFF causes a #UD if the CPU is !post-VMXON, but it's impossible to
- * atomically track post-VMXON state, e.g. this may be called in NMI context.
- * Eat all faults as all other faults on VMXOFF faults are mode related, i.e.
- * faults are guaranteed to be due to the !post-VMXON check unless the CPU is
- * magically in RM, VM86, compat mode, or at CPL>0.
- */
-static inline int cpu_vmxoff(void)
-{
-	asm_volatile_goto("1: vmxoff\n\t"
-			  _ASM_EXTABLE(1b, %l[fault])
-			  ::: "cc", "memory" : fault);
-
-	cr4_clear_bits(X86_CR4_VMXE);
-	return 0;
-
-fault:
-	cr4_clear_bits(X86_CR4_VMXE);
-	return -EIO;
-}
-
-static inline int cpu_vmx_enabled(void)
-{
-	return __read_cr4() & X86_CR4_VMXE;
-}
-
-/** Disable VMX if it is enabled on the current CPU
- */
-static inline void __cpu_emergency_vmxoff(void)
-{
-	if (cpu_vmx_enabled())
-		cpu_vmxoff();
-}
-
-
 /*
  * SVM functions:
  */
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f44f93772b4f..e00dba166a9e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -47,7 +47,6 @@
 #include <asm/mshyperv.h>
 #include <asm/mwait.h>
 #include <asm/spec-ctrl.h>
-#include <asm/virtext.h>
 #include <asm/vmx.h>
 
 #include "capabilities.h"
@@ -744,6 +743,29 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
 	return ret;
 }
 
+/*
+ * Disable VMX and clear CR4.VMXE (even if VMXOFF faults)
+ *
+ * Note, VMXOFF causes a #UD if the CPU is !post-VMXON, but it's impossible to
+ * atomically track post-VMXON state, e.g. this may be called in NMI context.
+ * Eat all faults as all other faults on VMXOFF faults are mode related, i.e.
+ * faults are guaranteed to be due to the !post-VMXON check unless the CPU is
+ * magically in RM, VM86, compat mode, or at CPL>0.
+ */
+static int kvm_cpu_vmxoff(void)
+{
+	asm_volatile_goto("1: vmxoff\n\t"
+			  _ASM_EXTABLE(1b, %l[fault])
+			  ::: "cc", "memory" : fault);
+
+	cr4_clear_bits(X86_CR4_VMXE);
+	return 0;
+
+fault:
+	cr4_clear_bits(X86_CR4_VMXE);
+	return -EIO;
+}
+
 static void vmx_emergency_disable(void)
 {
 	int cpu = raw_smp_processor_id();
@@ -753,7 +775,8 @@ static void vmx_emergency_disable(void)
 			    loaded_vmcss_on_cpu_link)
 		vmcs_clear(v->vmcs);
 
-	__cpu_emergency_vmxoff();
+	if (__read_cr4() & X86_CR4_VMXE)
+		kvm_cpu_vmxoff();
 }
 
 static void __loaded_vmcs_clear(void *arg)
@@ -2821,7 +2844,7 @@ static void vmx_hardware_disable(void)
 {
 	vmclear_local_loaded_vmcss();
 
-	if (cpu_vmxoff())
+	if (kvm_cpu_vmxoff())
 		kvm_spurious_fault();
 
 	hv_reset_evmcs();
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 11/18] KVM: SVM: Make KVM_AMD depend on CPU_SUP_AMD or CPU_SUP_HYGON

[Sean Christopherson]

Make building KVM SVM support depend on support for AMD or Hygon.  KVM
already effectively restricts SVM support to AMD and Hygon by virtue of
the vendor string checks in cpu_has_svm(), and KVM VMX supports depends
on one of its three known vendors (Intel, Centaur, or Zhaoxin).

Add the CPU_SUP_HYGON clause even though CPU_SUP_HYGON selects CPU_SUP_AMD
to document that KVM SVM support isn't just for AMD CPUs, and to prevent
breakage should Hygon support ever become a standalone thing.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 8e578311ca9d..0d403e9b6a47 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -102,7 +102,7 @@ config X86_SGX_KVM
 
 config KVM_AMD
 	tristate "KVM for AMD processors support"
-	depends on KVM
+	depends on KVM && (CPU_SUP_AMD || CPU_SUP_HYGON)
 	help
 	  Provides support for KVM on AMD processors equipped with the AMD-V
 	  (SVM) extensions.
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 12/18] x86/virt: Drop unnecessary check on extended CPUID level in cpu_has_svm()

[Sean Christopherson]

Drop the explicit check on the extended CPUID level in cpu_has_svm(), the
kernel's cached CPUID info will leave the entire SVM leaf unset if said
leaf is not supported by hardware.  Prior to using cached information,
the check was needed to avoid false positives due to Intel's rather crazy
CPUID behavior of returning the values of the maximum supported leaf if
the specified leaf is unsupported.

Fixes: 682a8108872f ("x86/kvm/svm: Simplify cpu_has_svm()")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index a27801f2bc71..be50c414efe4 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -39,12 +39,6 @@ static inline int cpu_has_svm(const char **msg)
 		return 0;
 	}
 
-	if (boot_cpu_data.extended_cpuid_level < SVM_CPUID_FUNC) {
-		if (msg)
-			*msg = "can't execute cpuid_8000000a";
-		return 0;
-	}
-
 	if (!boot_cpu_has(X86_FEATURE_SVM)) {
 		if (msg)
 			*msg = "svm not available";
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 13/18] x86/virt: KVM: Open code cpu_has_svm() into kvm_is_svm_supported()

[Sean Christopherson]

Fold the guts of cpu_has_svm() into kvm_is_svm_supported(), its sole
remaining user.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 28 ----------------------------
 arch/x86/kvm/svm/svm.c         | 11 ++++++++---
 2 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index be50c414efe4..632575e257d8 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -22,35 +22,7 @@
 /*
  * SVM functions:
  */
-
-/** Check if the CPU has SVM support
- *
- * You can use the 'msg' arg to get a message describing the problem,
- * if the function returns zero. Simply pass NULL if you are not interested
- * on the messages; gcc should take care of not generating code for
- * the messages on this case.
- */
-static inline int cpu_has_svm(const char **msg)
-{
-	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
-	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
-		if (msg)
-			*msg = "not amd or hygon";
-		return 0;
-	}
-
-	if (!boot_cpu_has(X86_FEATURE_SVM)) {
-		if (msg)
-			*msg = "svm not available";
-		return 0;
-	}
-	return 1;
-}
-
-
 /** Disable SVM on the current CPU
- *
- * You should call this only if cpu_has_svm() returned true.
  */
 static inline void cpu_svm_disable(void)
 {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 0f0d04900bf2..b347173b4e29 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -526,11 +526,16 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu)
 static bool kvm_is_svm_supported(void)
 {
 	int cpu = raw_smp_processor_id();
-	const char *msg;
 	u64 vm_cr;
 
-	if (!cpu_has_svm(&msg)) {
-		pr_err("SVM not supported by CPU %d, %s\n", cpu, msg);
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
+	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
+		pr_err("CPU %d isn't AMD or Hygon\n", cpu);
+		return false;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_SVM)) {
+		pr_err("SVM not supported by CPU %d\n", cpu);
 		return false;
 	}
 
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 14/18] KVM: SVM: Check that the current CPU supports SVM in kvm_is_svm_supported()

[Sean Christopherson]

Check "this" CPU instead of the boot CPU when querying SVM support so that
the per-CPU checks done during hardware enabling actually function as
intended, i.e. will detect issues where SVM isn't support on all CPUs.

Disable migration for the use from svm_init() mostly so that the standard
accessors for the per-CPU data can be used without getting yelled at by
CONFIG_DEBUG_PREEMPT=y sanity checks.  Preventing the "disabled by BIOS"
error message from reporting the wrong CPU is largely a bonus, as ensuring
a stable CPU during module load is a non-goal for KVM.

Link: https://lore.kernel.org/all/ZAdxNgv0M6P63odE@google.com
Cc: Kai Huang <kai.huang@intel.com>
Cc: Chao Gao <chao.gao@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index b347173b4e29..cf5f3880751b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -523,18 +523,20 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu)
 		vcpu->arch.osvw.status |= 1;
 }
 
-static bool kvm_is_svm_supported(void)
+static bool __kvm_is_svm_supported(void)
 {
-	int cpu = raw_smp_processor_id();
+	int cpu = smp_processor_id();
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
+
 	u64 vm_cr;
 
-	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
-	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
+	if (c->x86_vendor != X86_VENDOR_AMD &&
+	    c->x86_vendor != X86_VENDOR_HYGON) {
 		pr_err("CPU %d isn't AMD or Hygon\n", cpu);
 		return false;
 	}
 
-	if (!boot_cpu_has(X86_FEATURE_SVM)) {
+	if (!cpu_has(c, X86_FEATURE_SVM)) {
 		pr_err("SVM not supported by CPU %d\n", cpu);
 		return false;
 	}
@@ -553,9 +555,20 @@ static bool kvm_is_svm_supported(void)
 	return true;
 }
 
+static bool kvm_is_svm_supported(void)
+{
+	bool supported;
+
+	migrate_disable();
+	supported = __kvm_is_svm_supported();
+	migrate_enable();
+
+	return supported;
+}
+
 static int svm_check_processor_compat(void)
 {
-	if (!kvm_is_svm_supported())
+	if (!__kvm_is_svm_supported())
 		return -EIO;
 
 	return 0;
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 15/18] KVM: VMX: Ensure CPU is stable when probing basic VMX support

[Sean Christopherson]

Disable migration when probing VMX support during module load to ensure
the CPU is stable, mostly to match similar SVM logic, where allowing
migration effective requires deliberately writing buggy code.  As a bonus,
KVM won't report the wrong CPU to userspace if VMX is unsupported, but in
practice that is a very, very minor bonus as the only way that reporting
the wrong CPU would actually matter is if hardware is broken or if the
system is misconfigured, i.e. if KVM gets migrated from a CPU that _does_
support VMX to a CPU that does _not_ support VMX.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e00dba166a9e..008914396180 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2740,9 +2740,9 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 	return 0;
 }
 
-static bool kvm_is_vmx_supported(void)
+static bool __kvm_is_vmx_supported(void)
 {
-	int cpu = raw_smp_processor_id();
+	int cpu = smp_processor_id();
 
 	if (!(cpuid_ecx(1) & feature_bit(VMX))) {
 		pr_err("VMX not supported by CPU %d\n", cpu);
@@ -2758,13 +2758,24 @@ static bool kvm_is_vmx_supported(void)
 	return true;
 }
 
+static bool kvm_is_vmx_supported(void)
+{
+	bool supported;
+
+	migrate_disable();
+	supported = __kvm_is_vmx_supported();
+	migrate_enable();
+
+	return supported;
+}
+
 static int vmx_check_processor_compat(void)
 {
 	int cpu = raw_smp_processor_id();
 	struct vmcs_config vmcs_conf;
 	struct vmx_capability vmx_cap;
 
-	if (!kvm_is_vmx_supported())
+	if (!__kvm_is_vmx_supported())
 		return -EIO;
 
 	if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0) {
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 16/18] x86/virt: KVM: Move "disable SVM" helper into KVM SVM

[Sean Christopherson]

Move cpu_svm_disable() into KVM proper now that all hardware
virtualization management is routed through KVM.  Remove the now-empty
virtext.h.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/virtext.h | 50 ----------------------------------
 arch/x86/kvm/svm/svm.c         | 28 +++++++++++++++++--
 2 files changed, 25 insertions(+), 53 deletions(-)
 delete mode 100644 arch/x86/include/asm/virtext.h

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
deleted file mode 100644
index 632575e257d8..000000000000
--- a/arch/x86/include/asm/virtext.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/* CPU virtualization extensions handling
- *
- * This should carry the code for handling CPU virtualization extensions
- * that needs to live in the kernel core.
- *
- * Author: Eduardo Habkost <ehabkost@redhat.com>
- *
- * Copyright (C) 2008, Red Hat Inc.
- *
- * Contains code from KVM, Copyright (C) 2006 Qumranet, Inc.
- */
-#ifndef _ASM_X86_VIRTEX_H
-#define _ASM_X86_VIRTEX_H
-
-#include <asm/processor.h>
-
-#include <asm/vmx.h>
-#include <asm/svm.h>
-#include <asm/tlbflush.h>
-
-/*
- * SVM functions:
- */
-/** Disable SVM on the current CPU
- */
-static inline void cpu_svm_disable(void)
-{
-	uint64_t efer;
-
-	wrmsrl(MSR_VM_HSAVE_PA, 0);
-	rdmsrl(MSR_EFER, efer);
-	if (efer & EFER_SVME) {
-		/*
-		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
-		 * aren't blocked, e.g. if a fatal error occurred between CLGI
-		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
-		 * context between reading EFER and executing STGI.  In that
-		 * case, GIF must already be set, otherwise the NMI would have
-		 * been blocked, so just eat the fault.
-		 */
-		asm_volatile_goto("1: stgi\n\t"
-				  _ASM_EXTABLE(1b, %l[fault])
-				  ::: "memory" : fault);
-fault:
-		wrmsrl(MSR_EFER, efer & ~EFER_SVME);
-	}
-}
-
-#endif /* _ASM_X86_VIRTEX_H */
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index cf5f3880751b..2cc195d95d32 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -41,7 +41,6 @@
 #include <asm/reboot.h>
 #include <asm/fpu/api.h>
 
-#include <asm/virtext.h>
 #include "trace.h"
 
 #include "svm.h"
@@ -587,9 +586,32 @@ void __svm_write_tsc_multiplier(u64 multiplier)
 	preempt_enable();
 }
 
+static inline void kvm_cpu_svm_disable(void)
+{
+	uint64_t efer;
+
+	wrmsrl(MSR_VM_HSAVE_PA, 0);
+	rdmsrl(MSR_EFER, efer);
+	if (efer & EFER_SVME) {
+		/*
+		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
+		 * aren't blocked, e.g. if a fatal error occurred between CLGI
+		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
+		 * context between reading EFER and executing STGI.  In that
+		 * case, GIF must already be set, otherwise the NMI would have
+		 * been blocked, so just eat the fault.
+		 */
+		asm_volatile_goto("1: stgi\n\t"
+				  _ASM_EXTABLE(1b, %l[fault])
+				  ::: "memory" : fault);
+fault:
+		wrmsrl(MSR_EFER, efer & ~EFER_SVME);
+	}
+}
+
 static void svm_emergency_disable(void)
 {
-	cpu_svm_disable();
+	kvm_cpu_svm_disable();
 }
 
 static void svm_hardware_disable(void)
@@ -598,7 +620,7 @@ static void svm_hardware_disable(void)
 	if (tsc_scaling)
 		__svm_write_tsc_multiplier(SVM_TSC_RATIO_DEFAULT);
 
-	cpu_svm_disable();
+	kvm_cpu_svm_disable();
 
 	amd_pmu_disable_virt();
 }
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 17/18] KVM: x86: Force kvm_rebooting=true during emergency reboot/crash

[Sean Christopherson]

Set kvm_rebooting when virtualization is disabled in an emergency so that
KVM eats faults on virtualization instructions even if kvm_reboot() isn't
reached.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 2 ++
 arch/x86/kvm/vmx/vmx.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2cc195d95d32..d00da133b14f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -611,6 +611,8 @@ static inline void kvm_cpu_svm_disable(void)
 
 static void svm_emergency_disable(void)
 {
+	kvm_rebooting = true;
+
 	kvm_cpu_svm_disable();
 }
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 008914396180..1dec932aff21 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -771,6 +771,8 @@ static void vmx_emergency_disable(void)
 	int cpu = raw_smp_processor_id();
 	struct loaded_vmcs *v;
 
+	kvm_rebooting = true;
+
 	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
 			    loaded_vmcss_on_cpu_link)
 		vmcs_clear(v->vmcs);
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v3 18/18] KVM: SVM: Use "standard" stgi() helper when disabling SVM

[Sean Christopherson]

Now that kvm_rebooting is guaranteed to be true prior to disabling SVM
in an emergency, use the existing stgi() helper instead of open coding
STGI.  In effect, eat faults on STGI if and only if kvm_rebooting==true.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d00da133b14f..d94132898431 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -594,17 +594,10 @@ static inline void kvm_cpu_svm_disable(void)
 	rdmsrl(MSR_EFER, efer);
 	if (efer & EFER_SVME) {
 		/*
-		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
-		 * aren't blocked, e.g. if a fatal error occurred between CLGI
-		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
-		 * context between reading EFER and executing STGI.  In that
-		 * case, GIF must already be set, otherwise the NMI would have
-		 * been blocked, so just eat the fault.
+		 * Force GIF=1 prior to disabling SVM, e.g. to ensure INIT and
+		 * NMI aren't blocked.
 		 */
-		asm_volatile_goto("1: stgi\n\t"
-				  _ASM_EXTABLE(1b, %l[fault])
-				  ::: "memory" : fault);
-fault:
+		stgi();
 		wrmsrl(MSR_EFER, efer & ~EFER_SVME);
 	}
 }
-- 
2.40.1.606.ga4b1b128d6-goog

Re: [PATCH v3 02/18] x86/reboot: Harden virtualization hooks for emergency reboot

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Provide dedicated helpers to (un)register virt hooks used during an
> emergency crash/reboot, and WARN if there is an attempt to overwrite
> the registered callback, or an attempt to do an unpaired unregister.
> 
> Opportunsitically use rcu_assign_pointer() instead of RCU_INIT_POINTER(),
> mainly so that the set/unset paths are more symmetrical, but also because
> any performance gains from using RCU_INIT_POINTER() are meaningless for
> this code.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/reboot.h |  5 +++--
>  arch/x86/kernel/reboot.c      | 30 ++++++++++++++++++++++++------
>  arch/x86/kvm/vmx/vmx.c        |  6 ++----
>  3 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
> index 2551baec927d..d9a38d379d18 100644
> --- a/arch/x86/include/asm/reboot.h
> +++ b/arch/x86/include/asm/reboot.h
> @@ -25,8 +25,9 @@ void __noreturn machine_real_restart(unsigned int type);
>  #define MRR_BIOS	0
>  #define MRR_APM		1
>  
> -typedef void crash_vmclear_fn(void);
> -extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> +typedef void (cpu_emergency_virt_cb)(void);
> +void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback);
> +void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback);
>  void cpu_emergency_disable_virtualization(void);
>  
>  typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 299b970e5f82..739e09527dbb 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -794,17 +794,35 @@ void machine_crash_shutdown(struct pt_regs *regs)
>   *
>   * protected by rcu.
>   */
> -crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> -EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> +static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
> +
> +void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
> +{
> +	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
> +		return;
> +
> +	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
> +}
> +EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
> +
> +void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
> +{
> +	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
> +		return;
> +
> +	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
> +	synchronize_rcu();
> +}
> +EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
>  
>  static inline void cpu_crash_vmclear_loaded_vmcss(void)
>  {
> -	crash_vmclear_fn *do_vmclear_operation = NULL;
> +	cpu_emergency_virt_cb *callback;
>  
>  	rcu_read_lock();
> -	do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
> -	if (do_vmclear_operation)
> -		do_vmclear_operation();
> +	callback = rcu_dereference(cpu_emergency_virt_callback);
> +	if (callback)
> +		callback();
>  	rcu_read_unlock();
>  }
>  
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 317f72baf0c3..fc9cdb4114cc 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -8547,8 +8547,7 @@ static void __vmx_exit(void)
>  {
>  	allow_smaller_maxphyaddr = false;
>  
> -	RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
> -	synchronize_rcu();
> +	cpu_emergency_unregister_virt_callback(crash_vmclear_local_loaded_vmcss);
>  
>  	vmx_cleanup_l1d_flush();
>  }
> @@ -8598,8 +8597,7 @@ static int __init vmx_init(void)
>  		pi_init_cpu(cpu);
>  	}
>  
> -	rcu_assign_pointer(crash_vmclear_loaded_vmcss,
> -			   crash_vmclear_local_loaded_vmcss);
> +	cpu_emergency_register_virt_callback(crash_vmclear_local_loaded_vmcss);
>  
>  	vmx_check_vmcs12_offsets();
>  
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 03/18] x86/reboot: KVM: Handle VMXOFF in KVM's reboot callback

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Use KVM VMX's reboot/crash callback to do VMXOFF in an emergency instead
> of manually and blindly doing VMXOFF.  There's no need to attempt VMXOFF
> if a hypervisor, i.e. KVM, isn't loaded/active, i.e. if the CPU can't
> possibly be post-VMXON.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/virtext.h | 10 ----------
>  arch/x86/kernel/reboot.c       | 29 +++++++++--------------------
>  arch/x86/kvm/vmx/vmx.c         |  8 +++++---
>  3 files changed, 14 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> index 3b12e6b99412..5bc29fab15da 100644
> --- a/arch/x86/include/asm/virtext.h
> +++ b/arch/x86/include/asm/virtext.h
> @@ -70,16 +70,6 @@ static inline void __cpu_emergency_vmxoff(void)
>  		cpu_vmxoff();
>  }
>  
> -/** Disable VMX if it is supported and enabled on the current CPU
> - */
> -static inline void cpu_emergency_vmxoff(void)
> -{
> -	if (cpu_has_vmx())
> -		__cpu_emergency_vmxoff();
> -}
> -
> -
> -
>  
>  /*
>   * SVM functions:
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 739e09527dbb..0cf2261c2dec 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -787,13 +787,7 @@ void machine_crash_shutdown(struct pt_regs *regs)
>  }
>  #endif
>  
> -/*
> - * This is used to VMCLEAR all VMCSs loaded on the
> - * processor. And when loading kvm_intel module, the
> - * callback function pointer will be assigned.
> - *
> - * protected by rcu.
> - */
> +/* RCU-protected callback to disable virtualization prior to reboot. */
>  static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
>  
>  void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
> @@ -815,17 +809,6 @@ void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
>  }
>  EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
>  
> -static inline void cpu_crash_vmclear_loaded_vmcss(void)
> -{
> -	cpu_emergency_virt_cb *callback;
> -
> -	rcu_read_lock();
> -	callback = rcu_dereference(cpu_emergency_virt_callback);
> -	if (callback)
> -		callback();
> -	rcu_read_unlock();
> -}
> -
>  /* This is the CPU performing the emergency shutdown work. */
>  int crashing_cpu = -1;
>  
> @@ -836,9 +819,15 @@ int crashing_cpu = -1;
>   */
>  void cpu_emergency_disable_virtualization(void)
>  {
> -	cpu_crash_vmclear_loaded_vmcss();
> +	cpu_emergency_virt_cb *callback;
>  
> -	cpu_emergency_vmxoff();
> +	rcu_read_lock();
> +	callback = rcu_dereference(cpu_emergency_virt_callback);
> +	if (callback)
> +		callback();
> +	rcu_read_unlock();
> +
> +	/* KVM_AMD doesn't yet utilize the common callback. */
>  	cpu_emergency_svm_disable();
>  }
>  
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index fc9cdb4114cc..76cdb189f1b5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -744,7 +744,7 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
>  	return ret;
>  }
>  
> -static void crash_vmclear_local_loaded_vmcss(void)
> +static void vmx_emergency_disable(void)
>  {
>  	int cpu = raw_smp_processor_id();
>  	struct loaded_vmcs *v;
> @@ -752,6 +752,8 @@ static void crash_vmclear_local_loaded_vmcss(void)
>  	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
>  			    loaded_vmcss_on_cpu_link)
>  		vmcs_clear(v->vmcs);
> +
> +	__cpu_emergency_vmxoff();

__cpu_emergency_vmxoff() internally checks whether VMX is enabled in CR4.  
Logically, looks it's more reasonable to do such check before VMCLEAR active
VMCSes, although in practice there should be no problem I think.

But this problem inherits from the existing code in  upstream, so not sure
whether it is worth fixing.

>  }
>  
>  static void __loaded_vmcs_clear(void *arg)
> @@ -8547,7 +8549,7 @@ static void __vmx_exit(void)
>  {
>  	allow_smaller_maxphyaddr = false;
>  
> -	cpu_emergency_unregister_virt_callback(crash_vmclear_local_loaded_vmcss);
> +	cpu_emergency_unregister_virt_callback(vmx_emergency_disable);
>  
>  	vmx_cleanup_l1d_flush();
>  }
> @@ -8597,7 +8599,7 @@ static int __init vmx_init(void)
>  		pi_init_cpu(cpu);
>  	}
>  
> -	cpu_emergency_register_virt_callback(crash_vmclear_local_loaded_vmcss);
> +	cpu_emergency_register_virt_callback(vmx_emergency_disable);
>  
>  	vmx_check_vmcs12_offsets();
>  
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 04/18] x86/reboot: KVM: Disable SVM during reboot via virt/KVM reboot callback

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Use the virt callback to disable SVM (and set GIF=1) during an emergency
> instead of blindly attempting to disable SVM.  Like the VMX case, if a
> hypervisor, i.e. KVM, isn't loaded/active, SVM can't be in use.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Acked-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/virtext.h |  8 --------
>  arch/x86/kernel/reboot.c       |  3 ---
>  arch/x86/kvm/svm/svm.c         | 19 +++++++++++++++++--
>  3 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> index 5bc29fab15da..aaed66249ccf 100644
> --- a/arch/x86/include/asm/virtext.h
> +++ b/arch/x86/include/asm/virtext.h
> @@ -133,12 +133,4 @@ static inline void cpu_svm_disable(void)
>  	}
>  }
>  
> -/** Makes sure SVM is disabled, if it is supported on the CPU
> - */
> -static inline void cpu_emergency_svm_disable(void)
> -{
> -	if (cpu_has_svm(NULL))
> -		cpu_svm_disable();
> -}
> -
>  #endif /* _ASM_X86_VIRTEX_H */
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 0cf2261c2dec..92b380e199a3 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -826,9 +826,6 @@ void cpu_emergency_disable_virtualization(void)
>  	if (callback)
>  		callback();
>  	rcu_read_unlock();
> -
> -	/* KVM_AMD doesn't yet utilize the common callback. */
> -	cpu_emergency_svm_disable();
>  }
>  
>  #if defined(CONFIG_SMP)
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index eb308c9994f9..0f0d04900bf2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -38,6 +38,7 @@
>  #include <asm/spec-ctrl.h>
>  #include <asm/cpu_device_id.h>
>  #include <asm/traps.h>
> +#include <asm/reboot.h>
>  #include <asm/fpu/api.h>
>  
>  #include <asm/virtext.h>
> @@ -568,6 +569,11 @@ void __svm_write_tsc_multiplier(u64 multiplier)
>  	preempt_enable();
>  }
>  
> +static void svm_emergency_disable(void)
> +{
> +	cpu_svm_disable();
> +}
> +
>  static void svm_hardware_disable(void)
>  {
>  	/* Make sure we clean up behind us */
> @@ -5184,6 +5190,13 @@ static struct kvm_x86_init_ops svm_init_ops __initdata = {
>  	.pmu_ops = &amd_pmu_ops,
>  };
>  
> +static void __svm_exit(void)
> +{
> +	kvm_x86_vendor_exit();
> +
> +	cpu_emergency_unregister_virt_callback(svm_emergency_disable);
> +}
> +
>  static int __init svm_init(void)
>  {
>  	int r;
> @@ -5197,6 +5210,8 @@ static int __init svm_init(void)
>  	if (r)
>  		return r;
>  
> +	cpu_emergency_register_virt_callback(svm_emergency_disable);
> +
>  	/*
>  	 * Common KVM initialization _must_ come last, after this, /dev/kvm is
>  	 * exposed to userspace!
> @@ -5209,14 +5224,14 @@ static int __init svm_init(void)
>  	return 0;
>  
>  err_kvm_init:
> -	kvm_x86_vendor_exit();
> +	__svm_exit();
>  	return r;
>  }
>  
>  static void __exit svm_exit(void)
>  {
>  	kvm_exit();
> -	kvm_x86_vendor_exit();
> +	__svm_exit();
>  }
>  
>  module_init(svm_init)
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 05/18] x86/reboot: Disable virtualization during reboot iff callback is registered

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Attempt to disable virtualization during an emergency reboot if and only
> if there is a registered virt callback, i.e. iff a hypervisor (KVM) is
> active.  If there's no active hypervisor, then the CPU can't be operating
> with VMX or SVM enabled (barring an egregious bug).
> 
> Note, IRQs are disabled, which prevents KVM from coming along and enabling
> virtualization after the fact.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kernel/reboot.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 92b380e199a3..20f7bdabc52e 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -22,7 +22,6 @@
>  #include <asm/reboot_fixups.h>
>  #include <asm/reboot.h>
>  #include <asm/pci_x86.h>
> -#include <asm/virtext.h>
>  #include <asm/cpu.h>
>  #include <asm/nmi.h>
>  #include <asm/smp.h>
> @@ -545,7 +544,7 @@ static void emergency_reboot_disable_virtualization(void)
>  	 * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
>  	 * other CPUs may have virtualization enabled.
>  	 */
> -	if (cpu_has_vmx() || cpu_has_svm(NULL)) {
> +	if (rcu_access_pointer(cpu_emergency_virt_callback)) {
>  		/* Safely force _this_ CPU out of VMX/SVM operation. */
>  		cpu_emergency_disable_virtualization();


IIUC, for cpu_emergency_disable_virtualization() itself, looks it's OK to not
having the pointer check, since it internally will do rcu_dereference() inside
RCU critical section anyway.

But nmi_shootdown_cpus_on_restart() is called after
cpu_emergency_disable_virtualization(), and having the pointer check here can
avoid sending NMI to remote cpus if there's no active hypervisor.

Am I missing something?  If not, is it worth to call this out in changelog?

>  
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 06/18] x86/reboot: Assert that IRQs are disabled when turning off virtualization

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Assert that IRQs are disabled when turning off virtualization in an
> emergency.  KVM enables hardware via on_each_cpu(), i.e. could re-enable
> hardware if a pending IPI were delivered after disabling virtualization.
> 
> Remove a misleading comment from emergency_reboot_disable_virtualization()
> about "just" needing to guarantee the CPU is stable (see above).
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kernel/reboot.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 20f7bdabc52e..fddfea5f1d20 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -531,7 +531,6 @@ static inline void nmi_shootdown_cpus_on_restart(void);
>  
>  static void emergency_reboot_disable_virtualization(void)
>  {
> -	/* Just make sure we won't change CPUs while doing this */
>  	local_irq_disable();
>  
>  	/*
> @@ -820,6 +819,13 @@ void cpu_emergency_disable_virtualization(void)
>  {
>  	cpu_emergency_virt_cb *callback;
>  
> +	/*
> +	 * IRQs must be disabled as KVM enables virtualization in hardware via
> +	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
> +	 * virtualization stays disabled.
> +	 */
> +	lockdep_assert_irqs_disabled();
> +
>  	rcu_read_lock();
>  	callback = rcu_dereference(cpu_emergency_virt_callback);
>  	if (callback)
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 07/18] x86/reboot: Hoist "disable virt" helpers above "emergency reboot" path

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Move the various "disable virtualization" helpers above the emergency
> reboot path so that emergency_reboot_disable_virtualization() can be
> stubbed out in a future patch if neither KVM_INTEL nor KVM_AMD is enabled,
> i.e. if there is no in-tree user of CPU virtualization.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Personally I think this patch can be merged to next one.  Doing so may also
result in a more readable patch but I am not sure.

Anyway,

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kernel/reboot.c | 90 ++++++++++++++++++++--------------------
>  1 file changed, 45 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index fddfea5f1d20..493d66e59a4f 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -529,6 +529,51 @@ static inline void kb_wait(void)
>  
>  static inline void nmi_shootdown_cpus_on_restart(void);
>  
> +/* RCU-protected callback to disable virtualization prior to reboot. */
> +static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
> +
> +void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
> +{
> +	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
> +		return;
> +
> +	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
> +}
> +EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
> +
> +void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
> +{
> +	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
> +		return;
> +
> +	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
> +	synchronize_rcu();
> +}
> +EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
> +
> +/*
> + * Disable virtualization, i.e. VMX or SVM, to ensure INIT is recognized during
> + * reboot.  VMX blocks INIT if the CPU is post-VMXON, and SVM blocks INIT if
> + * GIF=0, i.e. if the crash occurred between CLGI and STGI.
> + */
> +void cpu_emergency_disable_virtualization(void)
> +{
> +	cpu_emergency_virt_cb *callback;
> +
> +	/*
> +	 * IRQs must be disabled as KVM enables virtualization in hardware via
> +	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
> +	 * virtualization stays disabled.
> +	 */
> +	lockdep_assert_irqs_disabled();
> +
> +	rcu_read_lock();
> +	callback = rcu_dereference(cpu_emergency_virt_callback);
> +	if (callback)
> +		callback();
> +	rcu_read_unlock();
> +}
> +
>  static void emergency_reboot_disable_virtualization(void)
>  {
>  	local_irq_disable();
> @@ -785,54 +830,9 @@ void machine_crash_shutdown(struct pt_regs *regs)
>  }
>  #endif
>  
> -/* RCU-protected callback to disable virtualization prior to reboot. */
> -static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
> -
> -void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
> -{
> -	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
> -		return;
> -
> -	rcu_assign_pointer(cpu_emergency_virt_callback, callback);
> -}
> -EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
> -
> -void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
> -{
> -	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
> -		return;
> -
> -	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
> -	synchronize_rcu();
> -}
> -EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
> -
>  /* This is the CPU performing the emergency shutdown work. */
>  int crashing_cpu = -1;
>  
> -/*
> - * Disable virtualization, i.e. VMX or SVM, to ensure INIT is recognized during
> - * reboot.  VMX blocks INIT if the CPU is post-VMXON, and SVM blocks INIT if
> - * GIF=0, i.e. if the crash occurred between CLGI and STGI.
> - */
> -void cpu_emergency_disable_virtualization(void)
> -{
> -	cpu_emergency_virt_cb *callback;
> -
> -	/*
> -	 * IRQs must be disabled as KVM enables virtualization in hardware via
> -	 * function call IPIs, i.e. IRQs need to be disabled to guarantee
> -	 * virtualization stays disabled.
> -	 */
> -	lockdep_assert_irqs_disabled();
> -
> -	rcu_read_lock();
> -	callback = rcu_dereference(cpu_emergency_virt_callback);
> -	if (callback)
> -		callback();
> -	rcu_read_unlock();
> -}
> -
>  #if defined(CONFIG_SMP)
>  
>  static nmi_shootdown_cb shootdown_callback;
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 08/18] x86/reboot: Expose VMCS crash hooks if and only if KVM_{INTEL,AMD} is enabled

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Expose the crash/reboot hooks used by KVM to disable virtualization in
> hardware and unblock INIT only if there's a potential in-tree user,
> i.e. either KVM_INTEL or KVM_AMD is enabled.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/reboot.h | 4 ++++
>  arch/x86/kernel/reboot.c      | 5 ++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
> index d9a38d379d18..9cf0b21bf495 100644
> --- a/arch/x86/include/asm/reboot.h
> +++ b/arch/x86/include/asm/reboot.h
> @@ -25,10 +25,14 @@ void __noreturn machine_real_restart(unsigned int type);
>  #define MRR_BIOS	0
>  #define MRR_APM		1
>  
> +#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
>  typedef void (cpu_emergency_virt_cb)(void);
>  void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback);
>  void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback);
>  void cpu_emergency_disable_virtualization(void);
> +#else
> +static inline void cpu_emergency_disable_virtualization(void) {}
> +#endif /* CONFIG_KVM_INTEL || CONFIG_KVM_AMD */
>  
>  typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
>  void nmi_panic_self_stop(struct pt_regs *regs);
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 493d66e59a4f..6818d8de85a1 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -529,6 +529,7 @@ static inline void kb_wait(void)
>  
>  static inline void nmi_shootdown_cpus_on_restart(void);
>  
> +#if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
>  /* RCU-protected callback to disable virtualization prior to reboot. */
>  static cpu_emergency_virt_cb __rcu *cpu_emergency_virt_callback;
>  
> @@ -596,7 +597,9 @@ static void emergency_reboot_disable_virtualization(void)
>  		nmi_shootdown_cpus_on_restart();
>  	}
>  }
> -
> +#else
> +static void emergency_reboot_disable_virtualization(void) { }
> +#endif /* CONFIG_KVM_INTEL || CONFIG_KVM_AMD */
>  
>  void __attribute__((weak)) mach_reboot_fixups(void)
>  {
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 10/18] x86/virt: KVM: Move VMXOFF helpers into KVM VMX

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Now that VMX is disabled in emergencies via the virt callbacks, move the
> VMXOFF helpers into KVM, the only remaining user.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

One nit ...

[...]


>  static void vmx_emergency_disable(void)
>  {
>  	int cpu = raw_smp_processor_id();
> @@ -753,7 +775,8 @@ static void vmx_emergency_disable(void)
>  			    loaded_vmcss_on_cpu_link)
>  		vmcs_clear(v->vmcs);
>  
> -	__cpu_emergency_vmxoff();
> +	if (__read_cr4() & X86_CR4_VMXE)
> +		kvm_cpu_vmxoff();
>  }

As replied to patch 3, is it better to move CR4.VMXE bit check before VMCLEAR
(can be done in either patch 3, or here)?

Re: [PATCH v3 05/18] x86/reboot: Disable virtualization during reboot iff callback is registered

[Sean Christopherson]

On Mon, May 22, 2023, Kai Huang wrote:
> On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> > Attempt to disable virtualization during an emergency reboot if and only
> > if there is a registered virt callback, i.e. iff a hypervisor (KVM) is
> > active.  If there's no active hypervisor, then the CPU can't be operating
> > with VMX or SVM enabled (barring an egregious bug).
> > 
> > Note, IRQs are disabled, which prevents KVM from coming along and enabling
> > virtualization after the fact.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kernel/reboot.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> > index 92b380e199a3..20f7bdabc52e 100644
> > --- a/arch/x86/kernel/reboot.c
> > +++ b/arch/x86/kernel/reboot.c
> > @@ -22,7 +22,6 @@
> >  #include <asm/reboot_fixups.h>
> >  #include <asm/reboot.h>
> >  #include <asm/pci_x86.h>
> > -#include <asm/virtext.h>
> >  #include <asm/cpu.h>
> >  #include <asm/nmi.h>
> >  #include <asm/smp.h>
> > @@ -545,7 +544,7 @@ static void emergency_reboot_disable_virtualization(void)
> >  	 * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
> >  	 * other CPUs may have virtualization enabled.
> >  	 */
> > -	if (cpu_has_vmx() || cpu_has_svm(NULL)) {
> > +	if (rcu_access_pointer(cpu_emergency_virt_callback)) {
> >  		/* Safely force _this_ CPU out of VMX/SVM operation. */
> >  		cpu_emergency_disable_virtualization();
> 
> 
> IIUC, for cpu_emergency_disable_virtualization() itself, looks it's OK to not
> having the pointer check, since it internally will do rcu_dereference() inside
> RCU critical section anyway.
> 
> But nmi_shootdown_cpus_on_restart() is called after
> cpu_emergency_disable_virtualization(), and having the pointer check here can
> avoid sending NMI to remote cpus if there's no active hypervisor.
> 
> Am I missing something?  If not, is it worth to call this out in changelog?

No, you're not missing anything.  I agree it's worth a line in the changelog.
Dropping the "spurious" NMI should be a-ok, but explicitly calling out the side
effect could be helpful for debug if something is silently relying on the NMI.

Re: [PATCH v3 03/18] x86/reboot: KVM: Handle VMXOFF in KVM's reboot callback

[Sean Christopherson]

On Mon, May 22, 2023, Kai Huang wrote:
> On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> > Use KVM VMX's reboot/crash callback to do VMXOFF in an emergency instead
> > of manually and blindly doing VMXOFF.  There's no need to attempt VMXOFF
> > if a hypervisor, i.e. KVM, isn't loaded/active, i.e. if the CPU can't
> > possibly be post-VMXON.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index fc9cdb4114cc..76cdb189f1b5 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -744,7 +744,7 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
> >  	return ret;
> >  }
> >  
> > -static void crash_vmclear_local_loaded_vmcss(void)
> > +static void vmx_emergency_disable(void)
> >  {
> >  	int cpu = raw_smp_processor_id();
> >  	struct loaded_vmcs *v;
> > @@ -752,6 +752,8 @@ static void crash_vmclear_local_loaded_vmcss(void)
> >  	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
> >  			    loaded_vmcss_on_cpu_link)
> >  		vmcs_clear(v->vmcs);
> > +
> > +	__cpu_emergency_vmxoff();
> 
> __cpu_emergency_vmxoff() internally checks whether VMX is enabled in CR4.  
> Logically, looks it's more reasonable to do such check before VMCLEAR active
> VMCSes, although in practice there should be no problem I think.
> 
> But this problem inherits from the existing code in  upstream, so not sure
> whether it is worth fixing.

Hmm, I think it's worth fixing, if only to avoid confusing future readers.  Blindly
doing VMCLEAR but then conditionally executing VMXOFF is nonsensical.  I'll tack on
a patch, and also add a comment to call out that CR4.VMXE can be _cleared_
asynchronously by NMI, but can't be set after being checked.  I.e. explain that
checking CR4.VMXE is a "best effort" sort of thing.

Re: [PATCH v3 03/18] x86/reboot: KVM: Handle VMXOFF in KVM's reboot callback

[Huang, Kai]

On Mon, 2023-05-22 at 10:58 -0700, Sean Christopherson wrote:
> On Mon, May 22, 2023, Kai Huang wrote:
> > On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> > > Use KVM VMX's reboot/crash callback to do VMXOFF in an emergency instead
> > > of manually and blindly doing VMXOFF.  There's no need to attempt VMXOFF
> > > if a hypervisor, i.e. KVM, isn't loaded/active, i.e. if the CPU can't
> > > possibly be post-VMXON.
> > > 
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > index fc9cdb4114cc..76cdb189f1b5 100644
> > > --- a/arch/x86/kvm/vmx/vmx.c
> > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > @@ -744,7 +744,7 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
> > >  	return ret;
> > >  }
> > >  
> > > -static void crash_vmclear_local_loaded_vmcss(void)
> > > +static void vmx_emergency_disable(void)
> > >  {
> > >  	int cpu = raw_smp_processor_id();
> > >  	struct loaded_vmcs *v;
> > > @@ -752,6 +752,8 @@ static void crash_vmclear_local_loaded_vmcss(void)
> > >  	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
> > >  			    loaded_vmcss_on_cpu_link)
> > >  		vmcs_clear(v->vmcs);
> > > +
> > > +	__cpu_emergency_vmxoff();
> > 
> > __cpu_emergency_vmxoff() internally checks whether VMX is enabled in CR4.  
> > Logically, looks it's more reasonable to do such check before VMCLEAR active
> > VMCSes, although in practice there should be no problem I think.
> > 
> > But this problem inherits from the existing code in  upstream, so not sure
> > whether it is worth fixing.
> 
> Hmm, I think it's worth fixing, if only to avoid confusing future readers.  Blindly
> doing VMCLEAR but then conditionally executing VMXOFF is nonsensical.  I'll tack on
> a patch, and also add a comment to call out that CR4.VMXE can be _cleared_
> asynchronously by NMI, but can't be set after being checked.  I.e. explain that
> checking CR4.VMXE is a "best effort" sort of thing.

Yeah looks good.

Re: [PATCH v3 05/18] x86/reboot: Disable virtualization during reboot iff callback is registered

[Huang, Kai]

On Mon, 2023-05-22 at 10:51 -0700, Sean Christopherson wrote:
> On Mon, May 22, 2023, Kai Huang wrote:
> > On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> > > Attempt to disable virtualization during an emergency reboot if and only
> > > if there is a registered virt callback, i.e. iff a hypervisor (KVM) is
> > > active.  If there's no active hypervisor, then the CPU can't be operating
> > > with VMX or SVM enabled (barring an egregious bug).
> > > 
> > > Note, IRQs are disabled, which prevents KVM from coming along and enabling
> > > virtualization after the fact.
> > > 
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >  arch/x86/kernel/reboot.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> > > index 92b380e199a3..20f7bdabc52e 100644
> > > --- a/arch/x86/kernel/reboot.c
> > > +++ b/arch/x86/kernel/reboot.c
> > > @@ -22,7 +22,6 @@
> > >  #include <asm/reboot_fixups.h>
> > >  #include <asm/reboot.h>
> > >  #include <asm/pci_x86.h>
> > > -#include <asm/virtext.h>
> > >  #include <asm/cpu.h>
> > >  #include <asm/nmi.h>
> > >  #include <asm/smp.h>
> > > @@ -545,7 +544,7 @@ static void emergency_reboot_disable_virtualization(void)
> > >  	 * Do the NMI shootdown even if virtualization is off on _this_ CPU, as
> > >  	 * other CPUs may have virtualization enabled.
> > >  	 */
> > > -	if (cpu_has_vmx() || cpu_has_svm(NULL)) {
> > > +	if (rcu_access_pointer(cpu_emergency_virt_callback)) {
> > >  		/* Safely force _this_ CPU out of VMX/SVM operation. */
> > >  		cpu_emergency_disable_virtualization();
> > 
> > 
> > IIUC, for cpu_emergency_disable_virtualization() itself, looks it's OK to not
> > having the pointer check, since it internally will do rcu_dereference() inside
> > RCU critical section anyway.
> > 
> > But nmi_shootdown_cpus_on_restart() is called after
> > cpu_emergency_disable_virtualization(), and having the pointer check here can
> > avoid sending NMI to remote cpus if there's no active hypervisor.
> > 
> > Am I missing something?  If not, is it worth to call this out in changelog?
> 
> No, you're not missing anything.  I agree it's worth a line in the changelog.
> Dropping the "spurious" NMI should be a-ok, but explicitly calling out the side
> effect could be helpful for debug if something is silently relying on the NMI.

Yeah my thinking too.  Thanks.

Re: [PATCH v3 14/18] KVM: SVM: Check that the current CPU supports SVM in kvm_is_svm_supported()

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Check "this" CPU instead of the boot CPU when querying SVM support so that
> the per-CPU checks done during hardware enabling actually function as
> intended, i.e. will detect issues where SVM isn't support on all CPUs.
> 
> Disable migration for the use from svm_init() mostly so that the standard
> accessors for the per-CPU data can be used without getting yelled at by
> CONFIG_DEBUG_PREEMPT=y sanity checks.  Preventing the "disabled by BIOS"
> error message from reporting the wrong CPU is largely a bonus, as ensuring
> a stable CPU during module load is a non-goal for KVM.

"non-goal for KVM (at least for now)" ? :)

> 
> Link: https://lore.kernel.org/all/ZAdxNgv0M6P63odE@google.com
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kvm/svm/svm.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index b347173b4e29..cf5f3880751b 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -523,18 +523,20 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu)
>  		vcpu->arch.osvw.status |= 1;
>  }
>  
> -static bool kvm_is_svm_supported(void)
> +static bool __kvm_is_svm_supported(void)
>  {
> -	int cpu = raw_smp_processor_id();
> +	int cpu = smp_processor_id();
> +	struct cpuinfo_x86 *c = &cpu_data(cpu);
> +
>  	u64 vm_cr;
>  
> -	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
> -	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
> +	if (c->x86_vendor != X86_VENDOR_AMD &&
> +	    c->x86_vendor != X86_VENDOR_HYGON) {
>  		pr_err("CPU %d isn't AMD or Hygon\n", cpu);
>  		return false;
>  	}
>  
> -	if (!boot_cpu_has(X86_FEATURE_SVM)) {
> +	if (!cpu_has(c, X86_FEATURE_SVM)) {
>  		pr_err("SVM not supported by CPU %d\n", cpu);
>  		return false;
>  	}
> @@ -553,9 +555,20 @@ static bool kvm_is_svm_supported(void)
>  	return true;
>  }
>  
> +static bool kvm_is_svm_supported(void)
> +{
> +	bool supported;
> +
> +	migrate_disable();
> +	supported = __kvm_is_svm_supported();
> +	migrate_enable();
> +
> +	return supported;
> +}
> +
>  static int svm_check_processor_compat(void)
>  {
> -	if (!kvm_is_svm_supported())
> +	if (!__kvm_is_svm_supported())
>  		return -EIO;
>  
>  	return 0;
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 17/18] KVM: x86: Force kvm_rebooting=true during emergency reboot/crash

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Set kvm_rebooting when virtualization is disabled in an emergency so that
> KVM eats faults on virtualization instructions even if kvm_reboot() isn't
> reached.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 2 ++
>  arch/x86/kvm/vmx/vmx.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 2cc195d95d32..d00da133b14f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -611,6 +611,8 @@ static inline void kvm_cpu_svm_disable(void)
>  
>  static void svm_emergency_disable(void)
>  {
> +	kvm_rebooting = true;
> +
>  	kvm_cpu_svm_disable();
>  }
>  
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 008914396180..1dec932aff21 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -771,6 +771,8 @@ static void vmx_emergency_disable(void)
>  	int cpu = raw_smp_processor_id();
>  	struct loaded_vmcs *v;
>  
> +	kvm_rebooting = true;

Do we need a memory barrier here?

> +
>  	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
>  			    loaded_vmcss_on_cpu_link)
>  		vmcs_clear(v->vmcs);
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 16/18] x86/virt: KVM: Move "disable SVM" helper into KVM SVM

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Move cpu_svm_disable() into KVM proper now that all hardware
> virtualization management is routed through KVM.  Remove the now-empty
> virtext.h.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/virtext.h | 50 ----------------------------------
>  arch/x86/kvm/svm/svm.c         | 28 +++++++++++++++++--
>  2 files changed, 25 insertions(+), 53 deletions(-)
>  delete mode 100644 arch/x86/include/asm/virtext.h
> 
> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> deleted file mode 100644
> index 632575e257d8..000000000000
> --- a/arch/x86/include/asm/virtext.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/* CPU virtualization extensions handling
> - *
> - * This should carry the code for handling CPU virtualization extensions
> - * that needs to live in the kernel core.
> - *
> - * Author: Eduardo Habkost <ehabkost@redhat.com>
> - *
> - * Copyright (C) 2008, Red Hat Inc.
> - *
> - * Contains code from KVM, Copyright (C) 2006 Qumranet, Inc.
> - */
> -#ifndef _ASM_X86_VIRTEX_H
> -#define _ASM_X86_VIRTEX_H
> -
> -#include <asm/processor.h>
> -
> -#include <asm/vmx.h>
> -#include <asm/svm.h>
> -#include <asm/tlbflush.h>
> -
> -/*
> - * SVM functions:
> - */
> -/** Disable SVM on the current CPU
> - */
> -static inline void cpu_svm_disable(void)
> -{
> -	uint64_t efer;
> -
> -	wrmsrl(MSR_VM_HSAVE_PA, 0);
> -	rdmsrl(MSR_EFER, efer);
> -	if (efer & EFER_SVME) {
> -		/*
> -		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
> -		 * aren't blocked, e.g. if a fatal error occurred between CLGI
> -		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
> -		 * context between reading EFER and executing STGI.  In that
> -		 * case, GIF must already be set, otherwise the NMI would have
> -		 * been blocked, so just eat the fault.
> -		 */
> -		asm_volatile_goto("1: stgi\n\t"
> -				  _ASM_EXTABLE(1b, %l[fault])
> -				  ::: "memory" : fault);
> -fault:
> -		wrmsrl(MSR_EFER, efer & ~EFER_SVME);
> -	}
> -}
> -
> -#endif /* _ASM_X86_VIRTEX_H */
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index cf5f3880751b..2cc195d95d32 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -41,7 +41,6 @@
>  #include <asm/reboot.h>
>  #include <asm/fpu/api.h>
>  
> -#include <asm/virtext.h>
>  #include "trace.h"
>  
>  #include "svm.h"
> @@ -587,9 +586,32 @@ void __svm_write_tsc_multiplier(u64 multiplier)
>  	preempt_enable();
>  }
>  
> +static inline void kvm_cpu_svm_disable(void)
> +{
> +	uint64_t efer;
> +
> +	wrmsrl(MSR_VM_HSAVE_PA, 0);
> +	rdmsrl(MSR_EFER, efer);
> +	if (efer & EFER_SVME) {
> +		/*
> +		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
> +		 * aren't blocked, e.g. if a fatal error occurred between CLGI
> +		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
> +		 * context between reading EFER and executing STGI.  In that
> +		 * case, GIF must already be set, otherwise the NMI would have
> +		 * been blocked, so just eat the fault.
> +		 */
> +		asm_volatile_goto("1: stgi\n\t"
> +				  _ASM_EXTABLE(1b, %l[fault])
> +				  ::: "memory" : fault);
> +fault:
> +		wrmsrl(MSR_EFER, efer & ~EFER_SVME);
> +	}
> +}
> +
>  static void svm_emergency_disable(void)
>  {
> -	cpu_svm_disable();
> +	kvm_cpu_svm_disable();
>  }
>  
>  static void svm_hardware_disable(void)
> @@ -598,7 +620,7 @@ static void svm_hardware_disable(void)
>  	if (tsc_scaling)
>  		__svm_write_tsc_multiplier(SVM_TSC_RATIO_DEFAULT);
>  
> -	cpu_svm_disable();
> +	kvm_cpu_svm_disable();
>  
>  	amd_pmu_disable_virt();
>  }
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 15/18] KVM: VMX: Ensure CPU is stable when probing basic VMX support

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Disable migration when probing VMX support during module load to ensure
> the CPU is stable, mostly to match similar SVM logic, where allowing
> migration effective requires deliberately writing buggy code.  As a bonus,
> KVM won't report the wrong CPU to userspace if VMX is unsupported, but in
> practice that is a very, very minor bonus as the only way that reporting
> the wrong CPU would actually matter is if hardware is broken or if the
> system is misconfigured, i.e. if KVM gets migrated from a CPU that _does_
> support VMX to a CPU that does _not_ support VMX.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kvm/vmx/vmx.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e00dba166a9e..008914396180 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2740,9 +2740,9 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  	return 0;
>  }
>  
> -static bool kvm_is_vmx_supported(void)
> +static bool __kvm_is_vmx_supported(void)
>  {
> -	int cpu = raw_smp_processor_id();
> +	int cpu = smp_processor_id();
>  
>  	if (!(cpuid_ecx(1) & feature_bit(VMX))) {
>  		pr_err("VMX not supported by CPU %d\n", cpu);
> @@ -2758,13 +2758,24 @@ static bool kvm_is_vmx_supported(void)
>  	return true;
>  }
>  
> +static bool kvm_is_vmx_supported(void)
> +{
> +	bool supported;
> +
> +	migrate_disable();
> +	supported = __kvm_is_vmx_supported();
> +	migrate_enable();
> +
> +	return supported;
> +}
> +
>  static int vmx_check_processor_compat(void)
>  {
>  	int cpu = raw_smp_processor_id();
>  	struct vmcs_config vmcs_conf;
>  	struct vmx_capability vmx_cap;
>  
> -	if (!kvm_is_vmx_supported())
> +	if (!__kvm_is_vmx_supported())
>  		return -EIO;
>  
>  	if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0) {
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 09/18] x86/virt: KVM: Open code cpu_has_vmx() in KVM VMX

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Fold the raw CPUID check for VMX into kvm_is_vmx_supported(), its sole
> user.  Keep the check even though KVM also checks X86_FEATURE_VMX, as the
> intent is to provide a unique error message if VMX is unsupported by
> hardware, whereas X86_FEATURE_VMX may be clear due to firmware and/or
> kernel actions.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

To be honest I am not sure the benefit of open coding, but this is a simple
check, so:

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/virtext.h | 10 ----------
>  arch/x86/kvm/vmx/vmx.c         |  2 +-
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> index aaed66249ccf..b1171a5ad452 100644
> --- a/arch/x86/include/asm/virtext.h
> +++ b/arch/x86/include/asm/virtext.h
> @@ -22,14 +22,6 @@
>  /*
>   * VMX functions:
>   */
> -
> -static inline int cpu_has_vmx(void)
> -{
> -	unsigned long ecx = cpuid_ecx(1);
> -	return test_bit(5, &ecx); /* CPUID.1:ECX.VMX[bit 5] -> VT */
> -}
> -
> -
>  /**
>   * cpu_vmxoff() - Disable VMX on the current CPU
>   *
> @@ -61,8 +53,6 @@ static inline int cpu_vmx_enabled(void)
>  }
>  
>  /** Disable VMX if it is enabled on the current CPU

Nit: looks the kdoc style comment (double **) can be removed.

> - *
> - * You shouldn't call this if cpu_has_vmx() returns 0.
>   */
>  static inline void __cpu_emergency_vmxoff(void)
>  {
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 76cdb189f1b5..f44f93772b4f 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2721,7 +2721,7 @@ static bool kvm_is_vmx_supported(void)
>  {
>  	int cpu = raw_smp_processor_id();
>  
> -	if (!cpu_has_vmx()) {
> +	if (!(cpuid_ecx(1) & feature_bit(VMX))) {
>  		pr_err("VMX not supported by CPU %d\n", cpu);
>  		return false;
>  	}
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 13/18] x86/virt: KVM: Open code cpu_has_svm() into kvm_is_svm_supported()

[Huang, Kai]

On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> Fold the guts of cpu_has_svm() into kvm_is_svm_supported(), its sole
> remaining user.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

The same to the reply to VMX patch:

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/include/asm/virtext.h | 28 ----------------------------
>  arch/x86/kvm/svm/svm.c         | 11 ++++++++---
>  2 files changed, 8 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> index be50c414efe4..632575e257d8 100644
> --- a/arch/x86/include/asm/virtext.h
> +++ b/arch/x86/include/asm/virtext.h
> @@ -22,35 +22,7 @@
>  /*
>   * SVM functions:
>   */
> -
> -/** Check if the CPU has SVM support
> - *
> - * You can use the 'msg' arg to get a message describing the problem,
> - * if the function returns zero. Simply pass NULL if you are not interested
> - * on the messages; gcc should take care of not generating code for
> - * the messages on this case.
> - */
> -static inline int cpu_has_svm(const char **msg)
> -{
> -	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
> -	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
> -		if (msg)
> -			*msg = "not amd or hygon";
> -		return 0;
> -	}
> -
> -	if (!boot_cpu_has(X86_FEATURE_SVM)) {
> -		if (msg)
> -			*msg = "svm not available";
> -		return 0;
> -	}
> -	return 1;
> -}
> -
> -
>  /** Disable SVM on the current CPU
> - *
> - * You should call this only if cpu_has_svm() returned true.
>   */

Nit: "double **" can be removed in the comment.

>  static inline void cpu_svm_disable(void)
>  {
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 0f0d04900bf2..b347173b4e29 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -526,11 +526,16 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu)
>  static bool kvm_is_svm_supported(void)
>  {
>  	int cpu = raw_smp_processor_id();
> -	const char *msg;
>  	u64 vm_cr;
>  
> -	if (!cpu_has_svm(&msg)) {
> -		pr_err("SVM not supported by CPU %d, %s\n", cpu, msg);
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
> +	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) {
> +		pr_err("CPU %d isn't AMD or Hygon\n", cpu);
> +		return false;
> +	}
> +
> +	if (!boot_cpu_has(X86_FEATURE_SVM)) {
> +		pr_err("SVM not supported by CPU %d\n", cpu);
>  		return false;
>  	}
>  
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v3 17/18] KVM: x86: Force kvm_rebooting=true during emergency reboot/crash

[Huang, Kai]

On Tue, 2023-05-23 at 11:25 +1200, Kai Huang wrote:
> On Fri, 2023-05-12 at 16:50 -0700, Sean Christopherson wrote:
> > Set kvm_rebooting when virtualization is disabled in an emergency so that
> > KVM eats faults on virtualization instructions even if kvm_reboot() isn't
> > reached.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/svm/svm.c | 2 ++
> >  arch/x86/kvm/vmx/vmx.c | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 2cc195d95d32..d00da133b14f 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -611,6 +611,8 @@ static inline void kvm_cpu_svm_disable(void)
> >  
> >  static void svm_emergency_disable(void)
> >  {
> > +	kvm_rebooting = true;
> > +
> >  	kvm_cpu_svm_disable();
> >  }
> >  
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 008914396180..1dec932aff21 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -771,6 +771,8 @@ static void vmx_emergency_disable(void)
> >  	int cpu = raw_smp_processor_id();
> >  	struct loaded_vmcs *v;
> >  
> > +	kvm_rebooting = true;
> 
> Do we need a memory barrier here?
> 

Hmm.. Please ignore this.  I was confused between cache coherency vs memory
ordering.  Not enough coffee.

Reviewed-by: Kai Huang <kai.huang@intel.com>