RE: [PATCH 04/14] KVM: VMX: Extend VMX controls macro shenanigans

[Dong, Eddie]

> -----Original Message-----
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Sent: Monday, June 27, 2022 6:38 PM
> To: Dong, Eddie <eddie.dong@xxxxxxxxx>
> Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx; Paolo
> Bonzini <pbonzini@xxxxxxxxxx>; Anirudh Rayabharam
> <anrayabh@xxxxxxxxxxxxxxxxxxx>; Wanpeng Li <wanpengli@xxxxxxxxxxx>;
> Jim Mattson <jmattson@xxxxxxxxxx>; Maxim Levitsky
> <mlevitsk@xxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 04/14] KVM: VMX: Extend VMX controls macro
> shenanigans
> 
> On Mon, Jun 27, 2022, Dong, Eddie wrote:
> > >  static inline void lname##_controls_clearbit(struct vcpu_vmx *vmx, u##bits
> > > val)	\
> > >  {
> > > 	\
> > > +	BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname |
> > > KVM_OPT_VMX_##uname)));	\
> > >  	lname##_controls_set(vmx, lname##_controls_get(vmx) & ~val);
> > > 	\
> > >  }
> >
> > With this, will it be safer if we present L1 CTRL MSRs with the bits
> > KVM really uses? Do I miss something?
> 
> KVM will still allow L1 to use features/controls that KVM itself doesn't use, but
> exposing features/controls that KVM doesn't use will require a more explicit
> "override" of sorts, e.g. to prevent advertising features that are supported in
> hardware, known to KVM, but disabled for whatever reason, e.g. a CPU bug,
> eVMCS incompatibility, module param, etc...
Mmm, that is fine too.
But, do we consider the potential need of migration for a L1 VMM ? Normally the VM can be configured to be as hardware neutral for better compatibility, or exposing as close to hardware feature as possible for performance.
For nested features, I thought we didn't support migration if L1 VMM yet, so exposing hardware capability by default is fine at moment. We may revisit one day in future if we need to support migration.  This MACRO do help anyway 😊

> 
> The intent of this BUILD_BUG_ON() is to detect KVM usage of bits that aren't
> enabled by default, i.e. to lower the probability that a control gets used by KVM
> but isn't exposed to L1 because it's a dynamically enabled control.