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

[Huang, Kai]

On Fri, 2023-03-10 at 13:42 -0800, 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).

IIUC, this patch is the final one that you want to achieve how the "disable
virtualization" callback should work in the non-KVM core kernel (the rest
patches are related to moving VMXOFF code to KVM as the core-kernel now just
calls the callback, etc).  

There are middle step patches (2-7) to eventually help to get to this point. 
But to be honest, personally, I am not sure whether those patches are necessary,
i.e. to me they actually cost more time to review since I have to think whether
such intermediate status is reasonable or not.  I am wondering whether we can
just merge those patches together as single one, so it's easy to see what is the
final goal to achieve?

Just my 2cents, of course.

> 
> Note, IRQs are disabled, which prevents KVM from coming along and enabling
> virtualization after the fact.
> 
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/kernel/reboot.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index cb268ec7ce85..dd7def3d4144 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>
> @@ -568,7 +567,6 @@ void cpu_emergency_disable_virtualization(void)
>  		callback();
>  	rcu_read_unlock();
>  }
> -#endif /* CONFIG_KVM_INTEL || CONFIG_KVM_AMD */
>  
>  static void emergency_reboot_disable_virtualization(void)
>  {
> @@ -585,7 +583,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();
>  
> @@ -593,6 +591,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.0.rc1.284.g88254d51c5-goog
>