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

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

[Wanpeng Li]

From: Wanpeng Li <wanpeng.li@hotmail.com>

According to the Intel SDM, volume 3, section 28.3.2: Creating and
Using Cached Translation Information, "The following items describe the 
creation of mappings while EPT is not in use":
 - Linear mappings may be created. They are derived from the paging 
   structures referenced (directly or indirectly) by the current value 
   of CR3 and are associated with the current VPID and the current PCID.

The VPID is 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>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3644540..800d08c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3681,15 +3681,19 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 				SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
 				SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
 
+	rdmsr(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
+		vmx_capability.ept = 0;
+	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_VPID))
+		vmx_capability.vpid = 0;
 
 	min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
 #ifdef CONFIG_X86_64
-- 
2.7.4

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

[Jim Mattson]

According to the SDM,

The IA32_VMX_EPT_VPID_CAP MSR exists only on processors that support
the 1-setting of the “activate secondary
controls” VM-execution control (only if bit 63 of the
IA32_VMX_PROCBASED_CTLS MSR is 1) and that support
either the 1-setting of the “enable EPT” VM-execution control (only if
bit 33 of the IA32_VMX_PROCBASED_CTLS2
MSR is 1) or the 1-setting of the “enable VPID” VM-execution control
(only if bit 37 of the
IA32_VMX_PROCBASED_CTLS2 MSR is 1).

Therefore, it seems dangerous to hoist the
rdmsr(MSR_IA32_VMX_EPT_VPID_CAP, ...) outside of the conditional,
unless you change it to rdmsr_safe.

I don't think Intel has ever shipped a CPU with VPID but without EPT,
so the motivation for this change is unclear to me.

On Sat, Oct 14, 2017 at 7:31 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> According to the Intel SDM, volume 3, section 28.3.2: Creating and
> Using Cached Translation Information, "The following items describe the
> creation of mappings while EPT is not in use":
>  - Linear mappings may be created. They are derived from the paging
>    structures referenced (directly or indirectly) by the current value
>    of CR3 and are associated with the current VPID and the current PCID.
>
> The VPID is 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>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 3644540..800d08c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3681,15 +3681,19 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>                                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>                                 SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>
> +       rdmsr(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
> +               vmx_capability.ept = 0;
> +       if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_VPID))
> +               vmx_capability.vpid = 0;
>
>         min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
>  #ifdef CONFIG_X86_64
> --
> 2.7.4
>

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

[Wanpeng Li]

Hi Jim,
2017-10-17 3:53 GMT+08:00 Jim Mattson <jmattson@google.com>:
> According to the SDM,
>
> The IA32_VMX_EPT_VPID_CAP MSR exists only on processors that support
> the 1-setting of the “activate secondary
> controls” VM-execution control (only if bit 63 of the
> IA32_VMX_PROCBASED_CTLS MSR is 1) and that support
> either the 1-setting of the “enable EPT” VM-execution control (only if
> bit 33 of the IA32_VMX_PROCBASED_CTLS2
> MSR is 1) or the 1-setting of the “enable VPID” VM-execution control
> (only if bit 37 of the
> IA32_VMX_PROCBASED_CTLS2 MSR is 1).
>
> Therefore, it seems dangerous to hoist the
> rdmsr(MSR_IA32_VMX_EPT_VPID_CAP, ...) outside of the conditional,
> unless you change it to rdmsr_safe.
>
> I don't think Intel has ever shipped a CPU with VPID but without EPT,
> so the motivation for this change is unclear to me.

My fault, I need to add more explanation to both patch 1/2, 2/2,
actually both patches are try to handle issues observed on L1 when EPT
is not exposed to L1.

For patch 1/2:
EPT switching is emulated for nested, however, I still can use vmxcap
tool to observe "EPTP Switching   yes" even if EPT is not exposed to
L1.
For patch 2/2:
EPT is not exposed to L1, the VPID capability is exposed and can be
observed by vmxcap tool:
  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. This is
what patch 2/2 tries to fix.

Regards,
Wanpeng Li

>
> On Sat, Oct 14, 2017 at 7:31 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> According to the Intel SDM, volume 3, section 28.3.2: Creating and
>> Using Cached Translation Information, "The following items describe the
>> creation of mappings while EPT is not in use":
>>  - Linear mappings may be created. They are derived from the paging
>>    structures referenced (directly or indirectly) by the current value
>>    of CR3 and are associated with the current VPID and the current PCID.
>>
>> The VPID is 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>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 3644540..800d08c 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3681,15 +3681,19 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>>                                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>>                                 SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>>
>> +       rdmsr(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
>> +               vmx_capability.ept = 0;
>> +       if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_VPID))
>> +               vmx_capability.vpid = 0;
>>
>>         min = VM_EXIT_SAVE_DEBUG_CONTROLS | VM_EXIT_ACK_INTR_ON_EXIT;
>>  #ifdef CONFIG_X86_64
>> --
>> 2.7.4
>>