On Mon, Sep 30, 2024, Paolo Bonzini wrote:
> On Sun, Sep 29, 2024 at 7:36 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>
> 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.
