Re: [PATCH 1/4] x86/reboot: Unconditionally define cpu_emergency_virt_cb typedef

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 22/05/2024 8:02 am, Sean Christopherson wrote:
On Wed, May 15, 2024, Kai Huang wrote:
How about we just make all emergency virtualization disable code
unconditional but not guided by CONFIG_KVM_INTEL || CONFIG_KVM_AMD, i.e.,
revert commit

    261cd5ed934e ("x86/reboot: Expose VMCS crash hooks if and only if
KVM_{INTEL,AMD} is enabled")

It makes sense anyway from the perspective that it allows the out-of-tree
kernel module hypervisor to use this mechanism w/o needing to have the
kernel built with KVM enabled in Kconfig.  Otherwise, strictly speaking,
IIUC, the kernel won't be able to support out-of-tree module hypervisor as
there's no other way the module can intercept emergency reboot.

Practically speaking, no one is running an out-of-tree hypervisor without either
(a) KVM being enabled in the .config, or (b) non-trivial changes to the kernel.

Just for curiosity: why b) is required to support out-of-tree hypervisor when KVM is disabled in Kconfig? I am probably missing something.


Exposing/exporting select APIs and symbols if and only if KVM is enabled is a
a well-established pattern, and there are concrete benefits to doing so.  E.g.
it allows minimizing the kernel footprint for use cases that don't want/need KVM.

This approach avoids the weirdness of the unconditional define for only
cpu_emergency_virt_cb.

I genuinely don't understand why you find it weird to unconditionally define
cpu_emergency_virt_cb.  There are myriad examples throughout the kernel where a
typedef, struct, enum, etc. is declared/defined even though support for its sole
end consumer is disabled.  E.g. include/linux/mm_types.h declares "struct mem_cgroup"
for pretty much the exact same reason, even though the structure is only fully
defined if CONFIG_MEMCG=y.

The only oddity here is that the API that the #ifdef that guards the usage happens
to be right below the typedef, but it shouldn't take that much brain power to
figure out why a typedef exists outside of an #ifdef.

OK.  No more arguments.  :-)

Thanks for this series anyway.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux