diff mbox

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

Message ID 1508290980-3525-2-git-send-email-wanpeng.li@hotmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wanpeng Li Oct. 18, 2017, 1:43 a.m. UTC 
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(-)

Comments

Jim Mattson Oct. 18, 2017, 3:39 p.m. UTC | #1 
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
>
Paolo Bonzini Oct. 18, 2017, 3:40 p.m. UTC | #2 
On 18/10/2017 17:39, Jim Mattson wrote:
> 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?

Indeed.

Paolo

> 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
>>
diff mbox

Patch

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