Re: [PATCH v2 18/49] KVM: x86: Account for max supported CPUID leaf when getting raw host CPUID

[Maxim Levitsky]

On Fri, 2024-05-17 at 10:38 -0700, Sean Christopherson wrote:
> Explicitly zero out the feature word in kvm_cpu_caps if the word's
> associated CPUID function is greater than the max leaf supported by the
> CPU.  For such unsupported functions, Intel CPUs return the output from
> the last supported leaf, not all zeros.
> 
> Practically speaking, this is likely a benign bug, as KVM uses the raw
> host CPUID to mask the kernel's computed capabilities, and the kernel does
> perform max leaf checks when populating boot_cpu_data.  The only way KVM's
> goof could be problematic is if the kernel force-set a feature in a leaf
> that is completely unsupported, _and_ the max supported leaf happened to
> return a value with '1' the same bit position.  Which is theoretically
> possible, but extremely unlikely.  And even if that did happen, it's
> entirely possible that KVM would still provide the correct functionality;
> the kernel did set the capability after all.
> 
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/kvm/cpuid.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index a51e48663f53..77625a5477b1 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -571,18 +571,37 @@ int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> +static __always_inline u32 raw_cpuid_get(struct cpuid_reg cpuid)
> +{
> +	struct kvm_cpuid_entry2 entry;
> +	u32 base;
> +
> +	/*
> +	 * KVM only supports features defined by Intel (0x0), AMD (0x80000000),
> +	 * and Centaur (0xc0000000).  WARN if a feature for new vendor base is
> +	 * defined, as this and other code would need to be updated.
> +	 */
> +	base = cpuid.function & 0xffff0000;
> +	if (WARN_ON_ONCE(base && base != 0x80000000 && base != 0xc0000000))
> +		return 0;
> +
> +	if (cpuid_eax(base) < cpuid.function)
> +		return 0;
> +
> +	cpuid_count(cpuid.function, cpuid.index,
> +		    &entry.eax, &entry.ebx, &entry.ecx, &entry.edx);
> +
> +	return *__cpuid_entry_get_reg(&entry, cpuid.reg);
> +}
> +
>  /* Mask kvm_cpu_caps for @leaf with the raw CPUID capabilities of this CPU. */
>  static __always_inline void __kvm_cpu_cap_mask(unsigned int leaf)
>  {
>  	const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32);
> -	struct kvm_cpuid_entry2 entry;
>  
>  	reverse_cpuid_check(leaf);
>  
> -	cpuid_count(cpuid.function, cpuid.index,
> -		    &entry.eax, &entry.ebx, &entry.ecx, &entry.edx);
> -
> -	kvm_cpu_caps[leaf] &= *__cpuid_entry_get_reg(&entry, cpuid.reg);
> +	kvm_cpu_caps[leaf] &= raw_cpuid_get(cpuid);
>  }
>  
>  static __always_inline

Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
	Maxim Levitsky