Re: [PATCH 48/61] KVM: x86: Do host CPUID at load time to mask KVM cpu caps

[Sean Christopherson]

On Mon, Feb 24, 2020 at 11:46:09PM +0100, Vitaly Kuznetsov wrote:
> Sean Christopherson <sean.j.christopherson@xxxxxxxxx> writes:
> 
> > Mask kvm_cpu_caps based on host CPUID in preparation for overriding the
> > CPUID results during KVM_GET_SUPPORTED_CPUID instead of doing the
> > masking at runtime.
> >
> > Note, masking may or may not be necessary, e.g. the kernel rarely, if
> > ever, sets real CPUID bits that are not supported by hardware.  But, the
> > code is cheap and only runs once at load, so an abundance of caution is
> > warranted.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > ---
> >  arch/x86/kvm/cpuid.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index ab2a34337588..4416f2422321 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -272,8 +272,22 @@ static __always_inline void cpuid_entry_mask(struct kvm_cpuid_entry2 *entry,
> >  
> >  static __always_inline void kvm_cpu_cap_mask(enum cpuid_leafs leaf, u32 mask)
> >  {
> > +	const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32);
> > +	struct kvm_cpuid_entry2 entry;
> > +
> >  	reverse_cpuid_check(leaf);
> >  	kvm_cpu_caps[leaf] &= mask;
> > +
> > +#ifdef CONFIG_KVM_CPUID_AUDIT
> > +	/* Entry needs to be fully populated when auditing is enabled. */
> > +	entry.function = cpuid.function;
> > +	entry.index = cpuid.index;
> > +#endif
> > +
> > +	cpuid_count(cpuid.function, cpuid.index,
> > +		    &entry.eax, &entry.ebx, &entry.ecx, &entry.edx);
> > +
> > +	kvm_cpu_caps[leaf] &= *__cpuid_entry_get_reg(&entry, &cpuid);
> >  }
> >  
> >  void kvm_set_cpu_caps(void)
> 
> If we don't really believe that masking will actually mask anything,
> maybe we should move it under '#ifdef CONFIG_KVM_CPUID_AUDIT'? And/or 
> add a WARN_ON()?

I'm not opposed to trying that, but I'd definitely want to do it as a
separate patch, or maybe even let it stew separately in kvm/queue for a
few cycles.

> The patch itself looks good, so:
> Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> 
> -- 
> Vitaly
>