Messages in this thread | | | From | Jim Mattson <> | Date | Mon, 2 Mar 2020 13:59:10 -0800 | Subject | Re: [PATCH 2/6] KVM: x86: Fix CPUID range check for Centaur and Hypervisor ranges |
| |
On Mon, Mar 2, 2020 at 11:57 AM Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > Extend the mask in cpuid_function_in_range() for finding the "class" of > the function to 0xfffffff00. While there is no official definition of > what constitutes a class, e.g. arguably bits 31:16 should be the class > and bits 15:0 the functions within that class, the Hypervisor logic > effectively uses bits 31:8 as the class by virtue of checking for > different bases in increments of 0x100, e.g. KVM advertises its CPUID > functions starting at 0x40000100 when HyperV features are advertised at > the default base of 0x40000000.
This convention deserves explicit documentation outside of the commit message.
> Masking against 0x80000000 only handles basic and extended leafs, which > results in Centaur and Hypervisor range checks being performed against > the basic CPUID range, e.g. if CPUID.0x40000000.EAX=0x4000000A and there > is no entry for CPUID.0x40000006, then function 0x40000006 would be > incorrectly reported as out of bounds. > > The bad range check doesn't cause function problems for any known VMM > because out-of-range semantics only come into play if the exact entry > isn't found, and VMMs either support a very limited Hypervisor range, > e.g. the official KVM range is 0x40000000-0x40000001 (effectively no > room for undefined leafs) or explicitly defines gaps to be zero, e.g. > Qemu explicitly creates zeroed entries up to the Cenatur and Hypervisor > limits (the latter comes into play when providing HyperV features).
Does Centaur implement the bizarre Intel behavior for out-of-bound entries? It seems that if there are Centaur leaves defined, the CPUD semantics should be those specified by Centaur.
> The bad behavior can be visually confirmed by dumping CPUID output in > the guest when running Qemu with a stable TSC, as Qemu extends the limit > of range 0x40000000 to 0x40000010 to advertise VMware's cpuid_freq, > without defining zeroed entries for 0x40000002 - 0x4000000f. > > Fixes: 43561123ab37 ("kvm: x86: Improve emulation of CPUID leaves 0BH and 1FH") > Cc: Jim Mattson <jmattson@google.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/cpuid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 6be012937eba..c320126e0118 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -993,7 +993,7 @@ static bool cpuid_function_in_range(struct kvm_vcpu *vcpu, u32 function) > { > struct kvm_cpuid_entry2 *max; > > - max = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0); > + max = kvm_find_cpuid_entry(vcpu, function & 0xffffff00u, 0);
This assumes that CPUID.(function & 0xffffff00):EAX always contains the maximum input value for the 256-entry range sharing the high 24 bits. I don't believe that convention has ever been established or documented.
> return max && function <= max->eax; > } > > -- > 2.24.1 >
| |