| Message ID | 1480472050-58023-4-git-send-email-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-11-29 18:14-0800, David Matlack: > KVM emulates MSR_IA32_VMX_CR{0,4}_FIXED1 with the value -1ULL, meaning > all CR0 and CR4 bits are allowed to be 1 during VMX operation. > > This does not match real hardware, which disallows the high 32 bits of > CR0 to be 1, and disallows reserved bits of CR4 to be 1 (including bits > which are defined in the SDM but missing according to CPUID). A guest > can induce a VM-entry failure by setting these bits in GUEST_CR0 and > GUEST_CR4, despite MSR_IA32_VMX_CR{0,4}_FIXED1 indicating they are > valid. > > Since KVM has allowed all bits to be 1 in CR0 and CR4, the existing > checks on these registers do not verify must-be-0 bits. Fix these checks > to identify must-be-0 bits according to MSR_IA32_VMX_CR{0,4}_FIXED1. > > This patch should introduce no change in behavior in KVM, since these > MSRs are still -1ULL. > > Signed-off-by: David Matlack <dmatlack@google.com> > --- > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > @@ -4104,6 +4110,40 @@ static void ept_save_pdptrs(struct kvm_vcpu *vcpu) > +static bool nested_guest_cr0_valid(struct kvm_vcpu *vcpu, unsigned long val) > +{ > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; > + struct vmcs12 *vmcs12 = get_vmcs12(vcpu); > + > + if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & > + SECONDARY_EXEC_UNRESTRICTED_GUEST && > + nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) > + fixed0 &= ~(X86_CR0_PE | X86_CR0_PG); These bits also seem to be guaranteed in fixed1 ... complicated dependencies. There is another exception, SDM 26.3.1.1 (Checks on Guest Control Registers, Debug Registers, and MSRs): Bit 29 (corresponding to CR0.NW) and bit 30 (CD) are never checked because the values of these bits are not changed by VM entry; see Section 26.3.2.1. And another check: If bit 31 in the CR0 field (corresponding to PG) is 1, bit 0 in that field (PE) must also be 1. > + > + return fixed_bits_valid(val, fixed0, fixed1); > +} > + > +static bool nested_host_cr0_valid(struct kvm_vcpu *vcpu, unsigned long val) > +{ > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; > + > + return fixed_bits_valid(val, fixed0, fixed1); > +} > + > +static bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) > +{ > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed0; > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed1; > + > + return fixed_bits_valid(val, fixed0, fixed1); > +} > + > +/* No difference in the restrictions on guest and host CR4 in VMX operation. */ > +#define nested_guest_cr4_valid nested_cr4_valid > +#define nested_host_cr4_valid nested_cr4_valid We should use cr0 and cr4 checks also in handle_vmon(). I've applied this series to kvm/queue for early testing. Please send replacement patch or patch(es) on top of this series. Thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Radim Krčmář" <rkrcmar@redhat.com> > To: "David Matlack" <dmatlack@google.com> > Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, jmattson@google.com, pbonzini@redhat.com > Sent: Wednesday, November 30, 2016 10:52:35 PM > Subject: Re: [PATCH v3 3/5] KVM: nVMX: fix checks on CR{0,4} during virtual VMX operation > > 2016-11-29 18:14-0800, David Matlack: > > KVM emulates MSR_IA32_VMX_CR{0,4}_FIXED1 with the value -1ULL, meaning > > all CR0 and CR4 bits are allowed to be 1 during VMX operation. > > > > This does not match real hardware, which disallows the high 32 bits of > > CR0 to be 1, and disallows reserved bits of CR4 to be 1 (including bits > > which are defined in the SDM but missing according to CPUID). A guest > > can induce a VM-entry failure by setting these bits in GUEST_CR0 and > > GUEST_CR4, despite MSR_IA32_VMX_CR{0,4}_FIXED1 indicating they are > > valid. > > > > Since KVM has allowed all bits to be 1 in CR0 and CR4, the existing > > checks on these registers do not verify must-be-0 bits. Fix these checks > > to identify must-be-0 bits according to MSR_IA32_VMX_CR{0,4}_FIXED1. > > > > This patch should introduce no change in behavior in KVM, since these > > MSRs are still -1ULL. > > > > Signed-off-by: David Matlack <dmatlack@google.com> > > --- > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > @@ -4104,6 +4110,40 @@ static void ept_save_pdptrs(struct kvm_vcpu *vcpu) > > +static bool nested_guest_cr0_valid(struct kvm_vcpu *vcpu, unsigned long > > val) > > +{ > > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; > > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; > > + struct vmcs12 *vmcs12 = get_vmcs12(vcpu); > > + > > + if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & > > + SECONDARY_EXEC_UNRESTRICTED_GUEST && > > + nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) > > + fixed0 &= ~(X86_CR0_PE | X86_CR0_PG); > > These bits also seem to be guaranteed in fixed1 ... complicated > dependencies. Bits that are set in fixed0 must be set in fixed1 too. Since patch 4 always sets CR0_FIXED1 to all-ones (matching bare metal), this is okay. > There is another exception, SDM 26.3.1.1 (Checks on Guest Control > Registers, Debug Registers, and MSRs): > > Bit 29 (corresponding to CR0.NW) and bit 30 (CD) are never checked > because the values of these bits are not changed by VM entry; see > Section 26.3.2.1. Same here, we never check them anyway. > And another check: > > If bit 31 in the CR0 field (corresponding to PG) is 1, bit 0 in that > field (PE) must also be 1. This should not be a problem, a failed vmentry is reflected into L1 anyway. We only need to check insofar as we could have a more restrictive check than what the processor does. Paolo > > + > > + return fixed_bits_valid(val, fixed0, fixed1); > > +} > > + > > +static bool nested_host_cr0_valid(struct kvm_vcpu *vcpu, unsigned long > > val) > > +{ > > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; > > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; > > + > > + return fixed_bits_valid(val, fixed0, fixed1); > > +} > > + > > +static bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) > > +{ > > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed0; > > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed1; > > + > > + return fixed_bits_valid(val, fixed0, fixed1); > > +} > > + > > +/* No difference in the restrictions on guest and host CR4 in VMX > > operation. */ > > +#define nested_guest_cr4_valid nested_cr4_valid > > +#define nested_host_cr4_valid nested_cr4_valid > > We should use cr0 and cr4 checks also in handle_vmon(). > > I've applied this series to kvm/queue for early testing. > Please send replacement patch or patch(es) on top of this series. > > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 30, 2016 at 2:33 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > ----- Original Message ----- >> From: "Radim Krčmář" <rkrcmar@redhat.com> >> To: "David Matlack" <dmatlack@google.com> >> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, jmattson@google.com, pbonzini@redhat.com >> Sent: Wednesday, November 30, 2016 10:52:35 PM >> Subject: Re: [PATCH v3 3/5] KVM: nVMX: fix checks on CR{0,4} during virtual VMX operation >> >> 2016-11-29 18:14-0800, David Matlack: >> > KVM emulates MSR_IA32_VMX_CR{0,4}_FIXED1 with the value -1ULL, meaning >> > all CR0 and CR4 bits are allowed to be 1 during VMX operation. >> > >> > This does not match real hardware, which disallows the high 32 bits of >> > CR0 to be 1, and disallows reserved bits of CR4 to be 1 (including bits >> > which are defined in the SDM but missing according to CPUID). A guest >> > can induce a VM-entry failure by setting these bits in GUEST_CR0 and >> > GUEST_CR4, despite MSR_IA32_VMX_CR{0,4}_FIXED1 indicating they are >> > valid. >> > >> > Since KVM has allowed all bits to be 1 in CR0 and CR4, the existing >> > checks on these registers do not verify must-be-0 bits. Fix these checks >> > to identify must-be-0 bits according to MSR_IA32_VMX_CR{0,4}_FIXED1. >> > >> > This patch should introduce no change in behavior in KVM, since these >> > MSRs are still -1ULL. >> > >> > Signed-off-by: David Matlack <dmatlack@google.com> >> > --- >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> > @@ -4104,6 +4110,40 @@ static void ept_save_pdptrs(struct kvm_vcpu *vcpu) >> > +static bool nested_guest_cr0_valid(struct kvm_vcpu *vcpu, unsigned long >> > val) >> > +{ >> > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; >> > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; >> > + struct vmcs12 *vmcs12 = get_vmcs12(vcpu); >> > + >> > + if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & >> > + SECONDARY_EXEC_UNRESTRICTED_GUEST && >> > + nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) >> > + fixed0 &= ~(X86_CR0_PE | X86_CR0_PG); >> >> These bits also seem to be guaranteed in fixed1 ... complicated >> dependencies. > > Bits that are set in fixed0 must be set in fixed1 too. Since patch 4 > always sets CR0_FIXED1 to all-ones (matching bare metal), this is okay. > >> There is another exception, SDM 26.3.1.1 (Checks on Guest Control >> Registers, Debug Registers, and MSRs): >> >> Bit 29 (corresponding to CR0.NW) and bit 30 (CD) are never checked >> because the values of these bits are not changed by VM entry; see >> Section 26.3.2.1. > > Same here, we never check them anyway. > >> And another check: >> >> If bit 31 in the CR0 field (corresponding to PG) is 1, bit 0 in that >> field (PE) must also be 1. > > This should not be a problem, a failed vmentry is reflected into L1 > anyway. We only need to check insofar as we could have a more restrictive > check than what the processor does. I had the same thought when I was first writing this patch, Radim. Maybe we should add a comment here. E.g. /* * CR0.PG && !CR0.PE is also invalid but caught by the CPU * during VM-entry to L2. */ > > Paolo > >> > + >> > + return fixed_bits_valid(val, fixed0, fixed1); >> > +} >> > + >> > +static bool nested_host_cr0_valid(struct kvm_vcpu *vcpu, unsigned long >> > val) >> > +{ >> > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; >> > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; >> > + >> > + return fixed_bits_valid(val, fixed0, fixed1); >> > +} >> > + >> > +static bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) >> > +{ >> > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed0; >> > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed1; >> > + >> > + return fixed_bits_valid(val, fixed0, fixed1); >> > +} >> > + >> > +/* No difference in the restrictions on guest and host CR4 in VMX >> > operation. */ >> > +#define nested_guest_cr4_valid nested_cr4_valid >> > +#define nested_host_cr4_valid nested_cr4_valid >> >> We should use cr0 and cr4 checks also in handle_vmon(). >> >> I've applied this series to kvm/queue for early testing. >> Please send replacement patch or patch(es) on top of this series. >> >> Thanks. >> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-11-30 15:53-0800, David Matlack: > On Wed, Nov 30, 2016 at 2:33 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> ----- Original Message ----- >>> From: "Radim Krčmář" <rkrcmar@redhat.com> >>> To: "David Matlack" <dmatlack@google.com> >>> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, jmattson@google.com, pbonzini@redhat.com >>> Sent: Wednesday, November 30, 2016 10:52:35 PM >>> Subject: Re: [PATCH v3 3/5] KVM: nVMX: fix checks on CR{0,4} during virtual VMX operation >>> >>> 2016-11-29 18:14-0800, David Matlack: >>> > KVM emulates MSR_IA32_VMX_CR{0,4}_FIXED1 with the value -1ULL, meaning >>> > all CR0 and CR4 bits are allowed to be 1 during VMX operation. >>> > >>> > This does not match real hardware, which disallows the high 32 bits of >>> > CR0 to be 1, and disallows reserved bits of CR4 to be 1 (including bits >>> > which are defined in the SDM but missing according to CPUID). A guest >>> > can induce a VM-entry failure by setting these bits in GUEST_CR0 and >>> > GUEST_CR4, despite MSR_IA32_VMX_CR{0,4}_FIXED1 indicating they are >>> > valid. >>> > >>> > Since KVM has allowed all bits to be 1 in CR0 and CR4, the existing >>> > checks on these registers do not verify must-be-0 bits. Fix these checks >>> > to identify must-be-0 bits according to MSR_IA32_VMX_CR{0,4}_FIXED1. >>> > >>> > This patch should introduce no change in behavior in KVM, since these >>> > MSRs are still -1ULL. >>> > >>> > Signed-off-by: David Matlack <dmatlack@google.com> >>> > --- >>> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> > @@ -4104,6 +4110,40 @@ static void ept_save_pdptrs(struct kvm_vcpu *vcpu) >>> > +static bool nested_guest_cr0_valid(struct kvm_vcpu *vcpu, unsigned long >>> > val) >>> > +{ >>> > + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; >>> > + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; >>> > + struct vmcs12 *vmcs12 = get_vmcs12(vcpu); >>> > + >>> > + if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & >>> > + SECONDARY_EXEC_UNRESTRICTED_GUEST && >>> > + nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) >>> > + fixed0 &= ~(X86_CR0_PE | X86_CR0_PG); >>> >>> These bits also seem to be guaranteed in fixed1 ... complicated >>> dependencies. >> >> Bits that are set in fixed0 must be set in fixed1 too. Since patch 4 >> always sets CR0_FIXED1 to all-ones (matching bare metal), this is okay. >> >>> There is another exception, SDM 26.3.1.1 (Checks on Guest Control >>> Registers, Debug Registers, and MSRs): >>> >>> Bit 29 (corresponding to CR0.NW) and bit 30 (CD) are never checked >>> because the values of these bits are not changed by VM entry; see >>> Section 26.3.2.1. >> >> Same here, we never check them anyway. True. >>> And another check: >>> >>> If bit 31 in the CR0 field (corresponding to PG) is 1, bit 0 in that >>> field (PE) must also be 1. >> >> This should not be a problem, a failed vmentry is reflected into L1 >> anyway. We only need to check insofar as we could have a more restrictive >> check than what the processor does. > > I had the same thought when I was first writing this patch, Radim. > Maybe we should add a comment here. E.g. > > /* > * CR0.PG && !CR0.PE is also invalid but caught by the CPU > * during VM-entry to L2. > */ Ah, no need, thanks. I expected that we want to kill L1 when VM entry failure happens, because it could have been L0's bug too ... we also pass failure while checking host state, which is definitely L0 bug and shouldn't ever get to L1. I'd like to assume that KVM is (going to be) sound, so both cases are reasonable (just hindering development). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 01a2b9e..b414513 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2864,12 +2864,18 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) vmx->nested.nested_vmx_vmcs_enum = 0x2e; } +/* + * if fixed0[i] == 1: val[i] must be 1 + * if fixed1[i] == 0: val[i] must be 0 + */ +static inline bool fixed_bits_valid(u64 val, u64 fixed0, u64 fixed1) +{ + return ((val & fixed1) | fixed0) == val; +} + static inline bool vmx_control_verify(u32 control, u32 low, u32 high) { - /* - * Bits 0 in high must be 0, and bits 1 in low must be 1. - */ - return ((control & high) | low) == control; + return fixed_bits_valid(control, low, high); } static inline u64 vmx_control_msr(u32 low, u32 high) @@ -4104,6 +4110,40 @@ static void ept_save_pdptrs(struct kvm_vcpu *vcpu) (unsigned long *)&vcpu->arch.regs_dirty); } +static bool nested_guest_cr0_valid(struct kvm_vcpu *vcpu, unsigned long val) +{ + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; + struct vmcs12 *vmcs12 = get_vmcs12(vcpu); + + if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & + SECONDARY_EXEC_UNRESTRICTED_GUEST && + nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) + fixed0 &= ~(X86_CR0_PE | X86_CR0_PG); + + return fixed_bits_valid(val, fixed0, fixed1); +} + +static bool nested_host_cr0_valid(struct kvm_vcpu *vcpu, unsigned long val) +{ + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed0; + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr0_fixed1; + + return fixed_bits_valid(val, fixed0, fixed1); +} + +static bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) +{ + u64 fixed0 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed0; + u64 fixed1 = to_vmx(vcpu)->nested.nested_vmx_cr4_fixed1; + + return fixed_bits_valid(val, fixed0, fixed1); +} + +/* No difference in the restrictions on guest and host CR4 in VMX operation. */ +#define nested_guest_cr4_valid nested_cr4_valid +#define nested_host_cr4_valid nested_cr4_valid + static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4); static void ept_update_paging_mode_cr0(unsigned long *hw_cr0, @@ -4232,8 +4272,8 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) if (!nested_vmx_allowed(vcpu)) return 1; } - if (to_vmx(vcpu)->nested.vmxon && - ((cr4 & VMXON_CR4_ALWAYSON) != VMXON_CR4_ALWAYSON)) + + if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4)) return 1; vcpu->arch.cr4 = cr4; @@ -5852,18 +5892,6 @@ vmx_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall) hypercall[2] = 0xc1; } -static bool nested_cr0_valid(struct kvm_vcpu *vcpu, unsigned long val) -{ - unsigned long always_on = VMXON_CR0_ALWAYSON; - struct vmcs12 *vmcs12 = get_vmcs12(vcpu); - - if (to_vmx(vcpu)->nested.nested_vmx_secondary_ctls_high & - SECONDARY_EXEC_UNRESTRICTED_GUEST && - nested_cpu_has2(vmcs12, SECONDARY_EXEC_UNRESTRICTED_GUEST)) - always_on &= ~(X86_CR0_PE | X86_CR0_PG); - return (val & always_on) == always_on; -} - /* called to set cr0 as appropriate for a mov-to-cr0 exit. */ static int handle_set_cr0(struct kvm_vcpu *vcpu, unsigned long val) { @@ -5882,7 +5910,7 @@ static int handle_set_cr0(struct kvm_vcpu *vcpu, unsigned long val) val = (val & ~vmcs12->cr0_guest_host_mask) | (vmcs12->guest_cr0 & vmcs12->cr0_guest_host_mask); - if (!nested_cr0_valid(vcpu, val)) + if (!nested_guest_cr0_valid(vcpu, val)) return 1; if (kvm_set_cr0(vcpu, val)) @@ -5891,8 +5919,9 @@ static int handle_set_cr0(struct kvm_vcpu *vcpu, unsigned long val) return 0; } else { if (to_vmx(vcpu)->nested.vmxon && - ((val & VMXON_CR0_ALWAYSON) != VMXON_CR0_ALWAYSON)) + !nested_host_cr0_valid(vcpu, val)) return 1; + return kvm_set_cr0(vcpu, val); } } @@ -10438,15 +10467,15 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) return 1; } - if (((vmcs12->host_cr0 & VMXON_CR0_ALWAYSON) != VMXON_CR0_ALWAYSON) || - ((vmcs12->host_cr4 & VMXON_CR4_ALWAYSON) != VMXON_CR4_ALWAYSON)) { + if (!nested_host_cr0_valid(vcpu, vmcs12->host_cr0) || + !nested_host_cr4_valid(vcpu, vmcs12->host_cr4)) { nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); return 1; } - if (!nested_cr0_valid(vcpu, vmcs12->guest_cr0) || - ((vmcs12->guest_cr4 & VMXON_CR4_ALWAYSON) != VMXON_CR4_ALWAYSON)) { + if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) || + !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) { nested_vmx_entry_failure(vcpu, vmcs12, EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT); return 1;
KVM emulates MSR_IA32_VMX_CR{0,4}_FIXED1 with the value -1ULL, meaning all CR0 and CR4 bits are allowed to be 1 during VMX operation. This does not match real hardware, which disallows the high 32 bits of CR0 to be 1, and disallows reserved bits of CR4 to be 1 (including bits which are defined in the SDM but missing according to CPUID). A guest can induce a VM-entry failure by setting these bits in GUEST_CR0 and GUEST_CR4, despite MSR_IA32_VMX_CR{0,4}_FIXED1 indicating they are valid. Since KVM has allowed all bits to be 1 in CR0 and CR4, the existing checks on these registers do not verify must-be-0 bits. Fix these checks to identify must-be-0 bits according to MSR_IA32_VMX_CR{0,4}_FIXED1. This patch should introduce no change in behavior in KVM, since these MSRs are still -1ULL. Signed-off-by: David Matlack <dmatlack@google.com> --- arch/x86/kvm/vmx.c | 77 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 24 deletions(-)