Re: [XEN][PATCH 1/2] x86: hvm: vmx: fix runtime vmx presence check for !CONFIG_INTEL_VMX case

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.09.2025 12:32, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> 
> Since commit b99227347230 ("x86: Fix AMD_SVM and INTEL_VMX dependency") the
> HVM Intel VT-x support can be gracefully disabled, but it still keeps VMX
> code partially built-in, because HVM code uses mix of:
> 
>  - "cpu_has_vmx" macro, which doesn't account for CONFIG_INTEL_VMX cfg
>  - "using_vmx()" function, which accounts for CONFIG_INTEL_VMX cfg
> 
> for runtime VMX availability checking. As result compiler DCE can't remove
> all, unreachable VMX code.
> 
> Fix it by sticking to "cpu_has_vmx" macro usage only which is updated to
> account CONFIG_INTEL_VMX cfg.
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> ---
> Hi
> 
> It could be good to have it in 4.21, so vmx/svm disabling
> option will be in complete state within 4.21 version.

Imo this isn't release critical and has come too late. It's of course
Oleksii's call in the end.

> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -136,7 +136,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>  #define cpu_has_sse3            boot_cpu_has(X86_FEATURE_SSE3)
>  #define cpu_has_pclmulqdq       boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>  #define cpu_has_monitor         boot_cpu_has(X86_FEATURE_MONITOR)
> -#define cpu_has_vmx             boot_cpu_has(X86_FEATURE_VMX)
> +#define cpu_has_vmx             (IS_ENABLED(CONFIG_INTEL_VMX) && \
> +                                 boot_cpu_has(X86_FEATURE_VMX))

I'm pretty sure using_vmx() was introduced precisely to avoid the use of
IS_ENABLED() here. What is completely missing from the description is a
discussion of the effect of this change on pre-existing uses of the
macro. ISTR there being at least one instance which would break with
that change. And no, I'm not looking forward to digging that out again,
when I already did at the time the using_vmx() was suggested and then
implemented. (I can't exclude it was the SVM counterpart; we want to
keep both in sync in any event, imo.)

Jan