Liran Alon <liran.alon@xxxxxxxxxx> 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@xxxxxxxxxx>
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@xxxxxxxxxx>
> Reviewed-by: Mark Kanda <mark.kanda@xxxxxxxxxx>
> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Thanks!
--
Vitaly
