On 9/30/24 18:53, Sean Christopherson wrote:
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.
Showing the KVM option is useful anyway as a grouping for other options
(e.g. SW-protected VMs, Xen, etc.). I can play with reordering
everything and using "select" to group these options, but I doubt it
will be better/more user-friendly than the above minimal change.
And also...
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.
... documentation changes can wait for 6.13 anyway, unlike fixing
the build (even if in a rare config that would mostly be hit by
randconfig).
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.
Sure.
Paolo
