Re: [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection

[Jim Mattson <jmattson@google.com>]

As long as we're taking the "better safe than sorry" approach, why
don't we go ahead and clear vmx_capability.ept/vmx_capability.vpid
along with the warnings?

On Tue, Oct 17, 2017 at 6:43 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> In my setup, EPT is not exposed to L1, the VPID capability is exposed and
> can be observed by vmxcap tool in L1:
> INVVPID supported                        yes
> Individual-address INVVPID               yes
> Single-context INVVPID                   yes
> All-context INVVPID                      yes
> Single-context-retaining-globals INVVPID yes
>
> However, the module parameter of VPID observed in L1 is always N, the
> cpu_has_vmx_invvpid() check in L1 KVM fails since vmx_capability.vpid
> is 0 and it is not read from MSR due to EPT is not exposed.
>
> The VPID can be used to tag linear mappings when EPT is not enabled. However,
> current logic just detects VPID capability if EPT is enabled, this patch
> fixes it.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Jim Mattson <jmattson@google.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> v2 -> v3:
>  * add pr_warn_once
> v1 -> v2:
>  * rdmsr_safe instead of rdmsr
>  * add more explanation to patch description
>
>  arch/x86/kvm/vmx.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 3644540..c14e981 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3681,15 +3681,22 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>                                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>                                 SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>
> +       rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP,
> +               &vmx_capability.ept, &vmx_capability.vpid);
> +
>         if (_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_EPT) {
>                 /* CR3 accesses and invlpg don't need to cause VM Exits when EPT
>                    enabled */
>                 _cpu_based_exec_control &= ~(CPU_BASED_CR3_LOAD_EXITING |
>                                              CPU_BASED_CR3_STORE_EXITING |
>                                              CPU_BASED_INVLPG_EXITING);
> -               rdmsr(MSR_IA32_VMX_EPT_VPID_CAP,
> -                     vmx_capability.ept, vmx_capability.vpid);
> -       }
> +       } else if (vmx_capability.ept)
> +               pr_warn_once("EPT CAP should not exist if not support "
> +                               "1-setting enable EPT VM-execution control\n");
> +       if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_VPID) &&
> +               vmx_capability.vpid)
> +               pr_warn_once("VPID CAP should not exist if not support "
> +                               "1-setting enable VPID VM-execution control\n");
>
>         min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
>  #ifdef CONFIG_X86_64
> --
> 2.7.4
>