Re: [GIT PULL] KVM/x86 changes for Linux 6.12

[Sean Christopherson <seanjc@google.com>]

On Mon, Sep 30, 2024, Paolo Bonzini wrote:
> On Sun, Sep 29, 2024 at 7:36 PM Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > The culprit is commit 590b09b1d88e ("KVM: x86: Register "emergency
> > disable" callbacks when virt is enabled"), and the reason seems to be
> > this:
> > 
> >   #if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(CONFIG_KVM_AMD)
> >   void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback);
> >   ...
> > 
> > ie if you have a config with KVM enabled, but neither KVM_INTEL nor
> > KVM_AMD set, you don't get that callback thing.
> > 
> > The fix may be something like the attached.
> 
> Yeah, there was an attempt in commit 6d55a94222db ("x86/reboot:
> Unconditionally define cpu_emergency_virt_cb typedef") but that only
> covers the headers and the !CONFIG_KVM case; not the !CONFIG_KVM_INTEL
> && !CONFIG_KVM_AMD one that you stumbled upon.
> 
> Your fix is not wrong, but there's no point in compiling kvm.ko if
> nobody is using it.
> 
> This is what I'll test more and submit:
> 
> ------------------ 8< ------------------
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] KVM: x86: leave kvm.ko out of the build if no vendor module is requested
> kvm.ko is nothing but library code shared by kvm-intel.ko and kvm-amd.ko.
> It provides no functionality on its own and it is unnecessary unless one
> of the vendor-specific module is compiled.  In particular, /dev/kvm is
> not created until one of kvm-intel.ko or kvm-amd.ko is loaded.
> Use CONFIG_KVM to decide if it is built-in or a module, but use the
> vendor-specific modules for the actual decision on whether to build it.
> This also fixes a build failure when CONFIG_KVM_INTEL and CONFIG_KVM_AMD
> are both disabled.  The cpu_emergency_register_virt_callback() function
> is called from kvm.ko, but it is only defined if at least one of
> CONFIG_KVM_INTEL and CONFIG_KVM_AMD is provided.
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 4287a8071a3a..aee054a91031 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -17,8 +17,8 @@ menuconfig VIRTUALIZATION
>  if VIRTUALIZATION
> -config KVM
> -	tristate "Kernel-based Virtual Machine (KVM) support"
> +config KVM_X86_COMMON
> +	def_tristate KVM if KVM_INTEL || KVM_AMD
>  	depends on HIGH_RES_TIMERS
>  	depends on X86_LOCAL_APIC
>  	select KVM_COMMON
> @@ -46,6 +47,9 @@ config KVM
>  	select KVM_GENERIC_HARDWARE_ENABLING
>  	select KVM_GENERIC_PRE_FAULT_MEMORY
>  	select KVM_WERROR if WERROR
> +
> +config KVM
> +	tristate "Kernel-based Virtual Machine (KVM) support"

I like the idea, but allowing users to select KVM=m|y but not building any parts
of KVM seems like it will lead to confusion.  What if we hide KVM entirely, and
autoselect m/y/n based on the vendor modules?  AFAICT, this behaves as expected.

Not having documentation for kvm.ko is unfortunate, but explaining how kvm.ko
interacts with kvm-{amd,intel}.ko probably belongs in Documentation/virt/kvm/?
anyways.

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 730c2f34d347..4350b83b63d8 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -18,7 +18,7 @@ menuconfig VIRTUALIZATION
 if VIRTUALIZATION
 
 config KVM
-       tristate "Kernel-based Virtual Machine (KVM) support"
+       def_tristate m if KVM_INTEL=m || KVM_AMD=m
        depends on X86_LOCAL_APIC
        select KVM_COMMON
        select KVM_GENERIC_MMU_NOTIFIER
@@ -45,19 +45,6 @@ config KVM
        select KVM_GENERIC_HARDWARE_ENABLING
        select KVM_GENERIC_PRE_FAULT_MEMORY
        select KVM_WERROR if WERROR
-       help
-         Support hosting fully virtualized guest machines using hardware
-         virtualization extensions.  You will need a fairly recent
-         processor equipped with virtualization extensions. You will also
-         need to select one or more of the processor modules below.
-
-         This module provides access to the hardware capabilities through
-         a character device node named /dev/kvm.
-
-         To compile this as a module, choose M here: the module
-         will be called kvm.
-
-         If unsure, say N.
 
 config KVM_WERROR
        bool "Compile KVM with -Werror"
@@ -88,7 +75,8 @@ config KVM_SW_PROTECTED_VM
 
 config KVM_INTEL
        tristate "KVM for Intel (and compatible) processors support"
-       depends on KVM && IA32_FEAT_CTL
+       depends on IA32_FEAT_CTL
+       select KVM if KVM_INTEL=y
        help
          Provides support for KVM on processors equipped with Intel's VT
          extensions, a.k.a. Virtual Machine Extensions (VMX).
@@ -125,7 +113,8 @@ config X86_SGX_KVM
 
 config KVM_AMD
        tristate "KVM for AMD processors support"
-       depends on KVM && (CPU_SUP_AMD || CPU_SUP_HYGON)
+       depends on CPU_SUP_AMD || CPU_SUP_HYGON
+       select KVM if KVM_AMD=y
        help
          Provides support for KVM on AMD processors equipped with the AMD-V
          (SVM) extensions.


>  	help
>  	  Support hosting fully virtualized guest machines using hardware
>  	  virtualization extensions.  You will need a fairly recent
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 5494669a055a..4304c89d6b64 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -32,7 +32,7 @@ kvm-intel-y		+= vmx/vmx_onhyperv.o vmx/hyperv_evmcs.o
>  kvm-amd-y		+= svm/svm_onhyperv.o
>  endif
> -obj-$(CONFIG_KVM)	+= kvm.o
> +obj-$(CONFIG_KVM_X86_COMMON) += kvm.o
>  obj-$(CONFIG_KVM_INTEL)	+= kvm-intel.o
>  obj-$(CONFIG_KVM_AMD)	+= kvm-amd.o
> ------------------ 8< ------------------
> 
> On top of this, the CONFIG_KVM #ifdefs could be changed to either
> CONFIG_KVM_X86_COMMON or (most of them) to CONFIG_KVM_INTEL; I started
> cleaning up the Kconfigs a few months ago and it's time to finish it
> off for 6.13.

If you haven't already, can you also kill off KVM_COMMON?  The only usage is in
scripts/gdb/linux/constants.py.in, to print Intel's posted interrupt IRQs in
scripts/gdb/linux/interrupts.py, which is quite ridiculous.