| Message ID | 20181101085739.56317-1-liran.alon@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: nVMX: Verify eVMCS revision id match supported eVMCS version on eVMCS VMPTRLD | expand |
Liran Alon <liran.alon@oracle.com> writes: > According to TLFS section 16.11.2 Enlightened VMCS, the first u32 > field of eVMCS should specify eVMCS VersionNumber. > > This version should be in the range of supported eVMCS versions exposed > to guest via CPUID.0x4000000A.EAX[0:15]. > The range which KVM expose to guest in this CPUID field should be the > same as the value returned in vmcs_version by nested_enable_evmcs(). > > According to the above, eVMCS VMPTRLD should verify that version specified > in given eVMCS is in the supported range. However, current code > mistakenly verfies this field against VMCS12_REVISION. > > One can also see that when KVM use eVMCS, it makes sure that > alloc_vmcs_cpu() sets allocated eVMCS revision_id to KVM_EVMCS_VERSION. > > Obvious fix should just change eVMCS VMPTRLD to verify first u32 field > of eVMCS is equal to KVM_EVMCS_VERSION. > However, it turns out that Microsoft Hyper-V fails to comply to their > own invented interface: When Hyper-V use eVMCS, it just sets first u32 > field of eVMCS to revision_id specified in MSR_IA32_VMX_BASIC (In our > case: VMCS12_REVISION). Instead of used eVMCS version number which is > one of the supported versions specified in CPUID.0x4000000A.EAX[0:15]. > To overcome Hyper-V bug, we accept either a supported eVMCS version > or VMCS12_REVISION as valid values for first u32 field of eVMCS. > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com> > Reviewed-by: Mark Kanda <mark.kanda@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> > --- > arch/x86/kvm/vmx.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 4555077d69ce..1d875cf16a79 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -9369,7 +9369,30 @@ static int nested_vmx_handle_enlightened_vmptrld(struct kvm_vcpu *vcpu, > > vmx->nested.hv_evmcs = kmap(vmx->nested.hv_evmcs_page); > > - if (vmx->nested.hv_evmcs->revision_id != VMCS12_REVISION) { > + /* > + * Currently, KVM only supports eVMCS version 1 > + * (== KVM_EVMCS_VERSION) and thus we expect guest to set this > + * value to first u32 field of eVMCS which should specify eVMCS > + * VersionNumber. > + * > + * Guest should be aware of supported eVMCS versions by host by > + * examining CPUID.0x4000000A.EAX[0:15]. Host userspace VMM is > + * expected to set this CPUID leaf according to the value > + * returned in vmcs_version from nested_enable_evmcs(). > + * > + * However, it turns out that Microsoft Hyper-V fails to comply > + * to their own invented interface: When Hyper-V use eVMCS, it > + * just sets first u32 field of eVMCS to revision_id specified > + * in MSR_IA32_VMX_BASIC. Instead of used eVMCS version number > + * which is one of the supported versions specified in > + * CPUID.0x4000000A.EAX[0:15]. > + * > + * To overcome Hyper-V bug, we accept here either a supported > + * eVMCS version or VMCS12 revision_id as valid values for first > + * u32 field of eVMCS. > + */ > + if ((vmx->nested.hv_evmcs->revision_id != KVM_EVMCS_VERSION) && > + (vmx->nested.hv_evmcs->revision_id != VMCS12_REVISION)) { > nested_release_evmcs(vcpu); > return 0; > } Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Thanks!
On 01/11/18 10:47, Vitaly Kuznetsov wrote:
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Queued, thanks.
Paolo
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 4555077d69ce..1d875cf16a79 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -9369,7 +9369,30 @@ static int nested_vmx_handle_enlightened_vmptrld(struct kvm_vcpu *vcpu, vmx->nested.hv_evmcs = kmap(vmx->nested.hv_evmcs_page); - if (vmx->nested.hv_evmcs->revision_id != VMCS12_REVISION) { + /* + * Currently, KVM only supports eVMCS version 1 + * (== KVM_EVMCS_VERSION) and thus we expect guest to set this + * value to first u32 field of eVMCS which should specify eVMCS + * VersionNumber. + * + * Guest should be aware of supported eVMCS versions by host by + * examining CPUID.0x4000000A.EAX[0:15]. Host userspace VMM is + * expected to set this CPUID leaf according to the value + * returned in vmcs_version from nested_enable_evmcs(). + * + * However, it turns out that Microsoft Hyper-V fails to comply + * to their own invented interface: When Hyper-V use eVMCS, it + * just sets first u32 field of eVMCS to revision_id specified + * in MSR_IA32_VMX_BASIC. Instead of used eVMCS version number + * which is one of the supported versions specified in + * CPUID.0x4000000A.EAX[0:15]. + * + * To overcome Hyper-V bug, we accept here either a supported + * eVMCS version or VMCS12 revision_id as valid values for first + * u32 field of eVMCS. + */ + if ((vmx->nested.hv_evmcs->revision_id != KVM_EVMCS_VERSION) && + (vmx->nested.hv_evmcs->revision_id != VMCS12_REVISION)) { nested_release_evmcs(vcpu); return 0; }