Re: [PATCH v2 4/7] KVM: x86: Fix CPUID range checks for Hypervisor and Centaur classes

[Sean Christopherson]

On Fri, Mar 06, 2020 at 10:03:37AM +0100, Paolo Bonzini wrote:
> On 05/03/20 22:51, Sean Christopherson wrote:
> >> Ah. So cross-vendor CPUID specifications are not supported?
> > Cross-vendor CPUID is sort of allowed?  E.g. this plays nice with creating
> > a Centaur CPU on an Intel platform.  My interpretation of GET_SUPPORTED...
> > is that KVM won't prevent enumerating what you want in CPUID, but it only
> > promises to correctly support select leafs.
> 
> But in practice does this change anything?  If the vendor is not Centaur 
> it's unlikely that there is a 0xc0000000 leaf.  The 0x80000000 bound is
> certainly not going to be at 0xc0000000 or beyond, and likewise to 0xc0000000
> bound is not going to be at 0xd0000000 or beyond.  So I'm not sure if
> anything is lost from this simplification:

Probably not?  But in the unlikely scenario that Intel wants to add a CPUID
leaf above 0xc0000000, I don't want to have to explain that it might cause
problems for KVM guests because I added code to emulate (alleged) Centaur
behavior for virtual Intel CPUs.

> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index ed5e0bda672c..f43a8875c126 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -963,8 +963,7 @@ static bool cpuid_function_in_range(struct kvm_vcpu *vcpu, u32 function)
>  
>  	if (function >= 0x40000000 && function <= 0x4fffffff)
>  		class = kvm_find_cpuid_entry(vcpu, function & 0xffffff00, 0);
> -	else if (function >= 0xc0000000 && function <= 0xcfffffff &&
> -		 is_guest_vendor_centaur(basic->ebx, basic->ecx, basic->edx))
> +	else if (function >= 0xc0000000)
>  		class = kvm_find_cpuid_entry(vcpu, 0xc0000000, 0);
>  	else
>  		class = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0);
> diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
> index 12ddfa493bae..3cb50eda606d 100644
> --- a/arch/x86/kvm/kvm_emulate.h
> +++ b/arch/x86/kvm/kvm_emulate.h
> @@ -424,13 +424,6 @@ static inline bool is_guest_vendor_hygon(u32 ebx, u32 ecx, u32 edx)
>  	       edx == X86EMUL_CPUID_VENDOR_HygonGenuine_edx;
>  }
>  
> -static inline bool is_guest_vendor_centaur(u32 ebx, u32 ecx, u32 edx)
> -{
> -	return ebx == X86EMUL_CPUID_VENDOR_CentaurHauls_ebx &&
> -	       ecx == X86EMUL_CPUID_VENDOR_CentaurHauls_ecx &&
> -	       edx == X86EMUL_CPUID_VENDOR_CentaurHauls_edx;
> -}
> -
>  enum x86_intercept_stage {
>  	X86_ICTP_NONE = 0,   /* Allow zero-init to not match anything */
>  	X86_ICPT_PRE_EXCEPT,
>