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

Sean Christopherson
2020-03-10T17:10:20Z

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,
>