Re: [RFC PATCH 10/27] KVM: x86: Use vendor-specific overlay flags instead of F_CPUID_DEFAULT

[Binbin Wu <binbin.wu@linux.intel.com>]

Below are responses to issues reported by sashiko. [1]
[1] https://sashiko.dev/#/patchset/20260417073610.3246316-1-binbin.wu%40linux.intel.com?part=10

Issue #1:
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 2c4e64aa14c47..71959f4918e78 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
[ ... ]
> @@ -983,7 +983,7 @@ void kvm_initialize_cpu_caps(void)
>  		/* MPX_MAWAU */
>  		F(RDPID, F_CPUID_DEFAULT | F_CPUID_TDX),
>  		/* KEY_LOCKER */
> -		F(BUS_LOCK_DETECT, F_CPUID_DEFAULT | F_CPUID_TDX),
> +		F(BUS_LOCK_DETECT, F_CPUID_VMX | F_CPUID_TDX),
>  		F(CLDEMOTE, F_CPUID_DEFAULT | F_CPUID_TDX),
>  		/* Reserved */
[ ... ]
> @@ -1022,7 +1022,7 @@ void kvm_initialize_cpu_caps(void)
>  		/* HYBRID_CPU */
>  		F(TSXLDTRK, F_CPUID_DEFAULT | F_CPUID_TDX),
>  		/* Reserved, PCONFIG, ARCH_LBR */
> -		F(IBT, F_CPUID_DEFAULT | F_CPUID_TDX),
> +		F(IBT, F_CPUID_VMX | F_CPUID_TDX),
>  		/* Reserved */
[ ... ]
> @@ -1075,7 +1075,7 @@ void kvm_initialize_cpu_caps(void)
>  
>  	kvm_cpu_cap_init(CPUID_7_1_ECX,
>  		/* MSR_IMM is reserved in TDX spec */
> -		SCATTERED_F(MSR_IMM, F_CPUID_DEFAULT),
> +		SCATTERED_F(MSR_IMM, F_CPUID_VMX),
>  	);
"Since IBT, BUS_LOCK_DETECT, and MSR_IMM are now initialized with
F_CPUID_VMX, they are never placed in the CPUID_OL_SVM overlay. 
Looking at the changes in arch/x86/kvm/svm/svm.c:"
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 2b4a17536580b..a21c500e1a91c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
[ ... ]
> -	kvm_cpu_cap_clear(X86_FEATURE_IBT, F_CPUID_DEFAULT);
> +	kvm_cpu_cap_clear(X86_FEATURE_IBT, F_CPUID_SVM);
[ ... ]
> -	kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT, F_CPUID_DEFAULT);
> -	kvm_cpu_cap_clear(X86_FEATURE_MSR_IMM, F_CPUID_DEFAULT);
> +	kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT, F_CPUID_SVM);
> +	kvm_cpu_cap_clear(X86_FEATURE_MSR_IMM, F_CPUID_SVM);
"Doesn't this make these kvm_cpu_cap_clear() calls redundant dead code?
They are now attempting to clear bits from the SVM overlay that were never
set in the first place.
Additionally, if SVM support is added for these architectural features in
the future, wouldn't developers naturally remove these clear calls from
svm_set_cpu_caps(), only to find the features still disabled because of the
F_CPUID_VMX mask in cpuid.c?
Keeping F_CPUID_DEFAULT in cpuid.c and relying on the explicit clear calls
in svm_set_cpu_caps() seems like it would avoid this trap."

This is addressed in the next patch (i.e. 11/27).

Issue #2:
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1049,11 +1049,11 @@ void kvm_initialize_cpu_caps(void)
>  	if (boot_cpu_has(X86_FEATURE_AMD_IBPB_RET) &&
>  	    boot_cpu_has(X86_FEATURE_AMD_IBPB) &&
>  	    boot_cpu_has(X86_FEATURE_AMD_IBRS))
> -		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL, F_CPUID_SVM);
>  	if (boot_cpu_has(X86_FEATURE_STIBP))
> -		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP, F_CPUID_VMX);
>  	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
> -		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD, F_CPUID_SVM);
[ ... ]
> @@ -1217,26 +1217,26 @@ void kvm_initialize_cpu_caps(void)
>  	 * record that in cpufeatures so use them.
>  	 */
>  	if (boot_cpu_has(X86_FEATURE_IBPB)) {
> -		kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB, F_CPUID_SVM);
>  		if (boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
>  		    !boot_cpu_has_bug(X86_BUG_EIBRS_PBRSB))
> -			kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB_RET, F_CPUID_DEFAULT);
> +			kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB_RET, F_CPUID_SVM);
>  	}
>  	if (boot_cpu_has(X86_FEATURE_IBRS))
> -		kvm_cpu_cap_set(X86_FEATURE_AMD_IBRS, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_AMD_IBRS, F_CPUID_SVM);
>  	if (boot_cpu_has(X86_FEATURE_STIBP))
> -		kvm_cpu_cap_set(X86_FEATURE_AMD_STIBP, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_AMD_STIBP, F_CPUID_SVM);
>  	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
> -		kvm_cpu_cap_set(X86_FEATURE_AMD_SSBD, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_AMD_SSBD, F_CPUID_SVM);
>  	if (!boot_cpu_has_bug(X86_BUG_SPEC_STORE_BYPASS))
> -		kvm_cpu_cap_set(X86_FEATURE_AMD_SSB_NO, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_AMD_SSB_NO, F_CPUID_SVM);
>  	/*
>  	 * The preference is to use SPEC CTRL MSR instead of the
>  	 * VIRT_SPEC MSR.
>  	 */
>  	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) &&
>  	    !boot_cpu_has(X86_FEATURE_AMD_SSBD))
> -		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD, F_CPUID_DEFAULT);
> +		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD, F_CPUID_SVM);

"Does restricting these cross-vendor mitigation features to specific vendor
overlays break cross-vendor live migration?
KVM previously synthesized equivalent cross-vendor hardware mitigation
features (like X86_FEATURE_AMD_IBPB) into both overlays using
F_CPUID_DEFAULT. This allowed an AMD guest model running on an Intel host
to see the AMD-specific CPUID bits, as KVM_GET_SUPPORTED_CPUID returns the
active overlay for the physical host.
By changing these to F_CPUID_SVM, the AMD mitigation features will be
completely missing from the VMX overlay. Will this cause userspace to
silently drop these security mitigations or refuse to launch cross-vendor
VMs, leaving guests vulnerable to speculative execution attacks?"

This is false positive since these features are AMD only features.
And it seems the AI misunderstand the reason of using F_CPUID_DEFAULT in
previous patches. Also, launching cross-vendor VMs is not supported by
KVM.