On Thu, Mar 25, 2021 at 11:49 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Thu, Mar 25, 2021, Haiwei Li wrote: > > On Tue, Mar 23, 2021 at 10:37 AM <lihaiwei.kernel@xxxxxxxxx> wrote: > > > > > > From: Haiwei Li <lihaiwei@xxxxxxxxxxx> > > > > > > According to IA-32 SDM Vol.3D "A.1 BASIC VMX INFORMATION", two inspections > > > are missing. > > > * Bit 31 is always 0. Earlier versions of this manual specified that the > > > VMCS revision identifier was a 32-bit field in bits 31:0 of this MSR. For > > > all processors produced prior to this change, bit 31 of this MSR was read > > > as 0. > > > * The values of bits 47:45 and bits 63:57 are reserved and are read as 0. > > > > > > Signed-off-by: Haiwei Li <lihaiwei@xxxxxxxxxxx> > > > --- > > > arch/x86/kvm/vmx/vmx.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > > index 32cf828..0d6d13c 100644 > > > --- a/arch/x86/kvm/vmx/vmx.c > > > +++ b/arch/x86/kvm/vmx/vmx.c > > > @@ -2577,6 +2577,20 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf, > > > > > > rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high); > > > > > > + /* > > > + * IA-32 SDM Vol 3D: Bit 31 is always 0. > > > + * For all earlier processors, bit 31 of this MSR was read as 0. > > > + */ > > > + if (vmx_msr_low & (1u<<31)) > > > + return -EIO; > > > > Drop this code as Jim said. > > > > > + > > > + /* > > > + * IA-32 SDM Vol 3D: bits 47:45 and bits 63:57 are reserved and are read > > > + * as 0. > > > + */ > > > + if (vmx_msr_high & 0xfe00e000) > > > + return -EIO; > > > > Is this ok? Can we pick up the part? :) > > No. "Reserved and are read as 0" does not guarantee the bits will always be > reserved. There are very few bits used for feature enumeration in x86 that are > guaranteed to be '0' for all eternity. > > The whole point of reserving bits in registers is so that the CPU vendor, Intel > in this case, can introduce new features and enumerate them to software without > colliding with existing features or breaking software. E.g. if Intel adds a new > feature and uses any of these bits to enumerate the feature, this check would > prevent KVM from loading on CPUs that support the feature. Got it, only explicit restrictions should be checked. Thanks. -- Haiwei Li
