mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising
@ 2017-10-18  1:42 Wanpeng Li
  2017-10-18  1:43 ` [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection Wanpeng Li
  2017-10-18  9:31 ` [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising David Hildenbrand
  0 siblings, 2 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-18  1:42 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Jim Mattson

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

I can use vmxcap tool to observe "EPTP Switching   yes" even if EPT is not 
exposed to L1.

EPT switching is advertised unconditionally since it is emulated, however, 
it can be treated as an extended feature for EPT and it should not be 
advertised if EPT itself is not exposed. 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:
 * it is legal for the IA32_VMX_VMFUNC MSR to report all-zero
v1 -> v2:
 * don't advertise "EPT VM Functions" in secondary processor-based VM-execution 
   controls if we don't actually support any VM Functions.

 arch/x86/kvm/vmx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c460b0b..3644540 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2842,8 +2842,9 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		 * Advertise EPTP switching unconditionally
 		 * since we emulate it
 		 */
-		vmx->nested.nested_vmx_vmfunc_controls =
-			VMX_VMFUNC_EPTP_SWITCHING;
+		if (enable_ept)
+			vmx->nested.nested_vmx_vmfunc_controls =
+				VMX_VMFUNC_EPTP_SWITCHING;
 	}
 
 	/*
-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection
  2017-10-18  1:42 [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising Wanpeng Li
@ 2017-10-18  1:43 ` Wanpeng Li
  2017-10-18 15:39   ` Jim Mattson
  2017-10-18  9:31 ` [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising David Hildenbrand
  1 sibling, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2017-10-18  1:43 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Jim Mattson

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising
  2017-10-18  1:42 [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising Wanpeng Li
  2017-10-18  1:43 ` [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection Wanpeng Li
@ 2017-10-18  9:31 ` David Hildenbrand
  2017-10-18  9:36   ` Wanpeng Li
  1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2017-10-18  9:31 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Jim Mattson

On 18.10.2017 03:42, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> I can use vmxcap tool to observe "EPTP Switching   yes" even if EPT is not 
> exposed to L1.
> 
> EPT switching is advertised unconditionally since it is emulated, however, 
> it can be treated as an extended feature for EPT and it should not be 
> advertised if EPT itself is not exposed. 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:
>  * it is legal for the IA32_VMX_VMFUNC MSR to report all-zero
> v1 -> v2:
>  * don't advertise "EPT VM Functions" in secondary processor-based VM-execution 
>    controls if we don't actually support any VM Functions.
> 
>  arch/x86/kvm/vmx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c460b0b..3644540 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2842,8 +2842,9 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		 * Advertise EPTP switching unconditionally
>  		 * since we emulate it
>  		 */
> -		vmx->nested.nested_vmx_vmfunc_controls =
> -			VMX_VMFUNC_EPTP_SWITCHING;
> +		if (enable_ept)
> +			vmx->nested.nested_vmx_vmfunc_controls =
> +				VMX_VMFUNC_EPTP_SWITCHING;
>  	}
>  
>  	/*
> 

I think we talked about how that cannot happen. But you observe it, so
it seems to happen :)

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising
  2017-10-18  9:31 ` [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising David Hildenbrand
@ 2017-10-18  9:36   ` Wanpeng Li
  0 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-10-18  9:36 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li, Jim Mattson

2017-10-18 17:31 GMT+08:00 David Hildenbrand <david@redhat.com>:
> On 18.10.2017 03:42, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> I can use vmxcap tool to observe "EPTP Switching   yes" even if EPT is not
>> exposed to L1.
>>
>> EPT switching is advertised unconditionally since it is emulated, however,
>> it can be treated as an extended feature for EPT and it should not be
>> advertised if EPT itself is not exposed. 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:
>>  * it is legal for the IA32_VMX_VMFUNC MSR to report all-zero
>> v1 -> v2:
>>  * don't advertise "EPT VM Functions" in secondary processor-based VM-execution
>>    controls if we don't actually support any VM Functions.
>>
>>  arch/x86/kvm/vmx.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c460b0b..3644540 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2842,8 +2842,9 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>>                * Advertise EPTP switching unconditionally
>>                * since we emulate it
>>                */
>> -             vmx->nested.nested_vmx_vmfunc_controls =
>> -                     VMX_VMFUNC_EPTP_SWITCHING;
>> +             if (enable_ept)
>> +                     vmx->nested.nested_vmx_vmfunc_controls =
>> +                             VMX_VMFUNC_EPTP_SWITCHING;
>>       }
>>
>>       /*
>>
>
> I think we talked about how that cannot happen. But you observe it, so
> it seems to happen :)

It is very easy to reproduce if you run L1 w/ ept=0.

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection
  2017-10-18  1:43 ` [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection Wanpeng Li
@ 2017-10-18 15:39   ` Jim Mattson
  2017-10-18 15:40     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Jim Mattson @ 2017-10-18 15:39 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: LKML, kvm list, Paolo Bonzini, Radim Krčmář, Wanpeng Li

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection
  2017-10-18 15:39   ` Jim Mattson
@ 2017-10-18 15:40     ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2017-10-18 15:40 UTC (permalink / raw)
  To: Jim Mattson, Wanpeng Li
  Cc: LKML, kvm list, Radim Krčmář, Wanpeng Li

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-10-18 15:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18  1:42 [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising Wanpeng Li
2017-10-18  1:43 ` [PATCH v3 2/2] KVM: VMX: Fix VPID capability detection Wanpeng Li
2017-10-18 15:39   ` Jim Mattson
2017-10-18 15:40     ` Paolo Bonzini
2017-10-18  9:31 ` [PATCH v3 1/2] KVM: nVMX: Fix EPT switching advertising David Hildenbrand
2017-10-18  9:36   ` Wanpeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®