Re: [PATCH] x86: kvm: add back X86_LOCAL_APIC dependency

[Sean Christopherson <seanjc@google.com>]

On Tue, Nov 12, 2024, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Enabling KVM now causes a build failure on x86-32 if X86_LOCAL_APIC
> is disabled:
> 
> arch/x86/kvm/svm/svm.c: In function 'svm_emergency_disable_virtualization_cpu':
> arch/x86/kvm/svm/svm.c:597:9: error: 'kvm_rebooting' undeclared (first use in this function); did you mean 'kvm_irq_routing'?
>   597 |         kvm_rebooting = true;
>       |         ^~~~~~~~~~~~~
>       |         kvm_irq_routing
> arch/x86/kvm/svm/svm.c:597:9: note: each undeclared identifier is reported only once for each function it appears in
> make[6]: *** [scripts/Makefile.build:221: arch/x86/kvm/svm/svm.o] Error 1
> In file included from include/linux/rculist.h:11,
>                  from include/linux/hashtable.h:14,
>                  from arch/x86/kvm/svm/avic.c:18:
> arch/x86/kvm/svm/avic.c: In function 'avic_pi_update_irte':
> arch/x86/kvm/svm/avic.c:909:38: error: 'struct kvm' has no member named 'irq_routing'
>   909 |         irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
>       |                                      ^~
> include/linux/rcupdate.h:538:17: note: in definition of macro '__rcu_dereference_check'
>   538 |         typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> 
> Move the dependency to the same place as before.
> 
> Fixes: ea4290d77bda ("KVM: x86: leave kvm.ko out of the build if no vendor module is requested")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202410060426.e9Xsnkvi-lkp@intel.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Question: is there actually any point in keeping KVM support for 32-bit host
> processors? From what I can tell, the only 32-bit CPUs that support this are
> the rare Atom E6xx and Z5xx models and the even older Yonah/Sossaman "Core
> Duo", everything else is presumably better off just running a 64-bit kernel
> even for 32-bit guests?
> ---
>  arch/x86/kvm/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 1ed1e4f5d51c..849a03f3ba95 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -19,7 +19,6 @@ if VIRTUALIZATION
>  
>  config KVM_X86
>  	def_tristate KVM if KVM_INTEL || KVM_AMD
> -	depends on X86_LOCAL_APIC
>  	select KVM_COMMON
>  	select KVM_GENERIC_MMU_NOTIFIER
>  	select KVM_ELIDE_TLB_FLUSH_IF_YOUNG
> @@ -93,6 +92,7 @@ config KVM_SW_PROTECTED_VM
>  config KVM_INTEL
>  	tristate "KVM for Intel (and compatible) processors support"
>  	depends on KVM && IA32_FEAT_CTL
> +	depends on X86_LOCAL_APIC
>  	help
>  	  Provides support for KVM on processors equipped with Intel's VT
>  	  extensions, a.k.a. Virtual Machine Extensions (VMX).
> @@ -130,6 +130,7 @@ config X86_SGX_KVM
>  config KVM_AMD
>  	tristate "KVM for AMD processors support"
>  	depends on KVM && (CPU_SUP_AMD || CPU_SUP_HYGON)
> +	depends on X86_LOCAL_APIC

The dependency can and should go on "config KVM", not on the vendor modules.  The
net effect on the build is the same, but preventing the user from selecting KVM
will provide a slightly better experience.