linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control
@ 2017-03-21  4:18 Wanpeng Li
  2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Wanpeng Li @ 2017-03-21  4:18 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

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

This can be reproduced by running kvm-unit-tests/vmx.flat on L0 w/ vpid disabled.

Test suite: VPID
Unhandled exception 6 #UD at ip 00000000004051a6
error_code=0000      rflags=00010047      cs=00000008
rax=0000000000000000 rcx=0000000000000001 rdx=0000000000000047 rbx=0000000000402f79
rbp=0000000000456240 rsi=0000000000000001 rdi=0000000000000000
r8=000000000000000a  r9=00000000000003f8 r10=0000000080010011 r11=0000000000000000
r12=0000000000000003 r13=0000000000000708 r14=0000000000000000 r15=0000000000000000
cr0=0000000080010031 cr2=0000000000000000 cr3=0000000007fff000 cr4=0000000000002020
cr8=0000000000000000
STACK: @4051a6 40523e 400f7f 402059 40028f

We should hide and forbid VPID in L1 if it is disabled on L0. However, nested VPID
enable bit is set unconditionally during setup nested vmx exec controls though VPID 
is not exposed through nested VMX capablity. This patch fixes it by don't set nested 
VPID enable bit if it is disabled on L0.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Fixes: 5c614b3583e (KVM: nVMX: nested VPID emulation)
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 98e82ee..8795a70 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2753,7 +2753,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		SECONDARY_EXEC_RDTSCP |
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
-		SECONDARY_EXEC_ENABLE_VPID |
 		SECONDARY_EXEC_APIC_REGISTER_VIRT |
 		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 		SECONDARY_EXEC_WBINVD_EXITING |
@@ -2781,10 +2780,12 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 	 * though it is treated as global context.  The alternative is
 	 * not failing the single-context invvpid, and it is worse.
 	 */
-	if (enable_vpid)
+	if (enable_vpid) {
+		vmx->nested.nested_vmx_secondary_ctls_high |=
+			SECONDARY_EXEC_ENABLE_VPID;
 		vmx->nested.nested_vmx_vpid_caps = VMX_VPID_INVVPID_BIT |
 			VMX_VPID_EXTENT_SUPPORTED_MASK;
-	else
+	} else
 		vmx->nested.nested_vmx_vpid_caps = 0;
 
 	if (enable_unrestricted_guest)
-- 
2.7.4

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

* [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  4:18 [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Wanpeng Li
@ 2017-03-21  4:18 ` Wanpeng Li
  2017-03-21  8:50   ` David Hildenbrand
  2017-03-21 16:19   ` Paolo Bonzini
  2017-03-21  4:18 ` [PATCH v2 3/3] KVM: x86: correct async page present tracepoint Wanpeng Li
  2017-03-21 16:19 ` [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Paolo Bonzini
  2 siblings, 2 replies; 11+ messages in thread
From: Wanpeng Li @ 2017-03-21  4:18 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

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

This can be reproduced by running L2 on L1, and disable VPID on L0 if w/o 
commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 crash as below:

KVM: entry failed, hardware error 0x7
EAX=00000000 EBX=00000000 ECX=00000000 EDX=000306c3
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 0000ffff 00009300
CS =f000 ffff0000 0000ffff 00009b00
SS =0000 00000000 0000ffff 00009300
DS =0000 00000000 0000ffff 00009300
FS =0000 00000000 0000ffff 00009300
GS =0000 00000000 0000ffff 00009300
LDT=0000 00000000 0000ffff 00008200
TR =0000 00000000 0000ffff 00008b00
GDT=     00000000 0000ffff
IDT=     00000000 0000ffff
CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 
DR6=00000000ffff0ff0 DR7=0000000000000400
EFER=0000000000000000

Reference SDM 30.3 INVVPID:
 
Protected Mode Exceptions
#UD 
  - If not in VMX operation.
  - If the logical processor does not support VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=0).
  - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) but does 
    not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).

So we should check both VPID enable bit in vmx exec control and INVVPID support bit 
in vmx capability MSRs to enable VPID. This patch adds the guarantee to not enable VPID
if INVVPID is not exposed in vmx capability MSRs.

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 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 06d8080..b310214 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
 	return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
 }
 
+static inline bool cpu_has_vmx_invvpid(void)
+{
+	return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
+}
+
 static inline bool cpu_has_vmx_ept(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
@@ -6519,8 +6524,10 @@ static __init int hardware_setup(void)
 	if (boot_cpu_has(X86_FEATURE_NX))
 		kvm_enable_efer_bits(EFER_NX);
 
-	if (!cpu_has_vmx_vpid())
+	if (!cpu_has_vmx_vpid() ||
+		!(cpu_has_vmx_invvpid()))
 		enable_vpid = 0;
+
 	if (!cpu_has_vmx_shadow_vmcs())
 		enable_shadow_vmcs = 0;
 	if (enable_shadow_vmcs)
-- 
2.7.4

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

* [PATCH v2 3/3] KVM: x86: correct async page present tracepoint
  2017-03-21  4:18 [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Wanpeng Li
  2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
@ 2017-03-21  4:18 ` Wanpeng Li
  2017-03-21  8:57   ` David Hildenbrand
  2017-03-21 16:19 ` [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Paolo Bonzini
  2 siblings, 1 reply; 11+ messages in thread
From: Wanpeng Li @ 2017-03-21  4:18 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

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

After async pf setup successfully, there is a broadcast wakeup w/ special 
token 0xffffffff which tells vCPU that it should wake up all processes 
waiting for APFs though there is no real process waiting at the moment.

The async page present tracepoint print prematurely and fails to catch the 
special token setup. This patch fixes it by moving the async page present 
tracepoint after the special token setup.

Before patch:

qemu-system-x86-8499  [006] ...1  5973.473292: kvm_async_pf_ready: token 0x0 gva 0x0

After patch:

qemu-system-x86-8499  [006] ...1  5973.473292: kvm_async_pf_ready: token 0xffffffff gva 0x0

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/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1faf620..e27eb7f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8566,11 +8566,11 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 {
 	struct x86_exception fault;
 
-	trace_kvm_async_pf_ready(work->arch.token, work->gva);
 	if (work->wakeup_all)
 		work->arch.token = ~0; /* broadcast wakeup */
 	else
 		kvm_del_async_pf_gfn(vcpu, work->arch.gfn);
+	trace_kvm_async_pf_ready(work->arch.token, work->gva);
 
 	if ((vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED) &&
 	    !apf_put_user(vcpu, KVM_PV_REASON_PAGE_READY)) {
-- 
2.7.4

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
@ 2017-03-21  8:50   ` David Hildenbrand
  2017-03-21  8:58     ` Wanpeng Li
  2017-03-21 16:19   ` Paolo Bonzini
  1 sibling, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2017-03-21  8:50 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

On 21.03.2017 05:18, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> This can be reproduced by running L2 on L1, and disable VPID on L0 if w/o 
> commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 crash as below:
> 
> KVM: entry failed, hardware error 0x7
> EAX=00000000 EBX=00000000 ECX=00000000 EDX=000306c3
> ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
> EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 00000000 0000ffff 00009300
> CS =f000 ffff0000 0000ffff 00009b00
> SS =0000 00000000 0000ffff 00009300
> DS =0000 00000000 0000ffff 00009300
> FS =0000 00000000 0000ffff 00009300
> GS =0000 00000000 0000ffff 00009300
> LDT=0000 00000000 0000ffff 00008200
> TR =0000 00000000 0000ffff 00008b00
> GDT=     00000000 0000ffff
> IDT=     00000000 0000ffff
> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 
> DR6=00000000ffff0ff0 DR7=0000000000000400
> EFER=0000000000000000
> 
> Reference SDM 30.3 INVVPID:
>  
> Protected Mode Exceptions
> #UD 
>   - If not in VMX operation.
>   - If the logical processor does not support VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=0).
>   - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) but does 
>     not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).
> 
> So we should check both VPID enable bit in vmx exec control and INVVPID support bit 
> in vmx capability MSRs to enable VPID. This patch adds the guarantee to not enable VPID
> if INVVPID is not exposed in vmx capability MSRs.
> 

Makes sense to me. Wonder how many systems are out there that have VPID
but not INVVPID? Or will this never happen on real hardware?

> 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 | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 06d8080..b310214 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>  	return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>  }
>  
> +static inline bool cpu_has_vmx_invvpid(void)
> +{
> +	return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
> +}
> +
>  static inline bool cpu_has_vmx_ept(void)
>  {
>  	return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -6519,8 +6524,10 @@ static __init int hardware_setup(void)
>  	if (boot_cpu_has(X86_FEATURE_NX))
>  		kvm_enable_efer_bits(EFER_NX);
>  
> -	if (!cpu_has_vmx_vpid())
> +	if (!cpu_has_vmx_vpid() ||
> +		!(cpu_has_vmx_invvpid()))

This indentation looks weird. Can't this be fit into one line?

>  		enable_vpid = 0;
> +

unrelated change

>  	if (!cpu_has_vmx_shadow_vmcs())
>  		enable_shadow_vmcs = 0;
>  	if (enable_shadow_vmcs)
> 


-- 

Thanks,

David

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

* Re: [PATCH v2 3/3] KVM: x86: correct async page present tracepoint
  2017-03-21  4:18 ` [PATCH v2 3/3] KVM: x86: correct async page present tracepoint Wanpeng Li
@ 2017-03-21  8:57   ` David Hildenbrand
  0 siblings, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2017-03-21  8:57 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li

On 21.03.2017 05:18, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> After async pf setup successfully, there is a broadcast wakeup w/ special 
> token 0xffffffff which tells vCPU that it should wake up all processes 
> waiting for APFs though there is no real process waiting at the moment.
> 
> The async page present tracepoint print prematurely and fails to catch the 
> special token setup. This patch fixes it by moving the async page present 
> tracepoint after the special token setup.
> 
> Before patch:
> 
> qemu-system-x86-8499  [006] ...1  5973.473292: kvm_async_pf_ready: token 0x0 gva 0x0
> 
> After patch:
> 
> qemu-system-x86-8499  [006] ...1  5973.473292: kvm_async_pf_ready: token 0xffffffff gva 0x0
> 

Wonder if there is a reason why this is traced before any work is done.
Maybe we should keep that order (by breaking up the if-else).

> 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/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1faf620..e27eb7f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8566,11 +8566,11 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>  {
>  	struct x86_exception fault;
>  
> -	trace_kvm_async_pf_ready(work->arch.token, work->gva);
>  	if (work->wakeup_all)
>  		work->arch.token = ~0; /* broadcast wakeup */
>  	else
>  		kvm_del_async_pf_gfn(vcpu, work->arch.gfn);
> +	trace_kvm_async_pf_ready(work->arch.token, work->gva);
>  
>  	if ((vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED) &&
>  	    !apf_put_user(vcpu, KVM_PV_REASON_PAGE_READY)) {
> 


-- 

Thanks,

David

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  8:50   ` David Hildenbrand
@ 2017-03-21  8:58     ` Wanpeng Li
  2017-03-21  9:01       ` David Hildenbrand
  2017-03-21  9:15       ` David Hildenbrand
  0 siblings, 2 replies; 11+ messages in thread
From: Wanpeng Li @ 2017-03-21  8:58 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li

2017-03-21 16:50 GMT+08:00 David Hildenbrand <david@redhat.com>:
> On 21.03.2017 05:18, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> This can be reproduced by running L2 on L1, and disable VPID on L0 if w/o
>> commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 crash as below:
>>
>> KVM: entry failed, hardware error 0x7
>> EAX=00000000 EBX=00000000 ECX=00000000 EDX=000306c3
>> ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
>> EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES =0000 00000000 0000ffff 00009300
>> CS =f000 ffff0000 0000ffff 00009b00
>> SS =0000 00000000 0000ffff 00009300
>> DS =0000 00000000 0000ffff 00009300
>> FS =0000 00000000 0000ffff 00009300
>> GS =0000 00000000 0000ffff 00009300
>> LDT=0000 00000000 0000ffff 00008200
>> TR =0000 00000000 0000ffff 00008b00
>> GDT=     00000000 0000ffff
>> IDT=     00000000 0000ffff
>> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
>> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
>> DR6=00000000ffff0ff0 DR7=0000000000000400
>> EFER=0000000000000000
>>
>> Reference SDM 30.3 INVVPID:
>>
>> Protected Mode Exceptions
>> #UD
>>   - If not in VMX operation.
>>   - If the logical processor does not support VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=0).
>>   - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) but does
>>     not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).
>>
>> So we should check both VPID enable bit in vmx exec control and INVVPID support bit
>> in vmx capability MSRs to enable VPID. This patch adds the guarantee to not enable VPID
>> if INVVPID is not exposed in vmx capability MSRs.
>>
>
> Makes sense to me. Wonder how many systems are out there that have VPID
> but not INVVPID? Or will this never happen on real hardware?

At least this will not happen on the real hardware on my hands.

>
>> 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 | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 06d8080..b310214 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>>       return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>>  }
>>
>> +static inline bool cpu_has_vmx_invvpid(void)
>> +{
>> +     return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
>> +}
>> +
>>  static inline bool cpu_has_vmx_ept(void)
>>  {
>>       return vmcs_config.cpu_based_2nd_exec_ctrl &
>> @@ -6519,8 +6524,10 @@ static __init int hardware_setup(void)
>>       if (boot_cpu_has(X86_FEATURE_NX))
>>               kvm_enable_efer_bits(EFER_NX);
>>
>> -     if (!cpu_has_vmx_vpid())
>> +     if (!cpu_has_vmx_vpid() ||
>> +             !(cpu_has_vmx_invvpid()))
>
> This indentation looks weird. Can't this be fit into one line?

The same as cpu_has_vmx_ept_4levels().

>
>>               enable_vpid = 0;
>> +
>
> unrelated change

To make the vpid codes more clear. Please refer to other callees in
hardware_setup().

>
>>       if (!cpu_has_vmx_shadow_vmcs())
>>               enable_shadow_vmcs = 0;
>>       if (enable_shadow_vmcs)
>>
>
>
> --
>
> Thanks,
>
> David

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  8:58     ` Wanpeng Li
@ 2017-03-21  9:01       ` David Hildenbrand
  2017-03-21  9:05         ` Wanpeng Li
  2017-03-21  9:15       ` David Hildenbrand
  1 sibling, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2017-03-21  9:01 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li


> The same as cpu_has_vmx_ept_4levels().
> 
>>
>>>               enable_vpid = 0;
>>> +
>>
>> unrelated change
> 
> To make the vpid codes more clear. Please refer to other callees in
> hardware_setup().

I was talking about the added empty line. This is unrelated to the patch
you're posting.

-- 

Thanks,

David

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  9:01       ` David Hildenbrand
@ 2017-03-21  9:05         ` Wanpeng Li
  0 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2017-03-21  9:05 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li

2017-03-21 17:01 GMT+08:00 David Hildenbrand <david@redhat.com>:
>
>> The same as cpu_has_vmx_ept_4levels().
>>
>>>
>>>>               enable_vpid = 0;
>>>> +
>>>
>>> unrelated change
>>
>> To make the vpid codes more clear. Please refer to other callees in
>> hardware_setup().
>
> I was talking about the added empty line. This is unrelated to the patch
> you're posting.

Yeah, I was talking about the same thing. Just a *small* cleanup and
it is suitable for the patch.

Regards,
Wanpeng Li

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  8:58     ` Wanpeng Li
  2017-03-21  9:01       ` David Hildenbrand
@ 2017-03-21  9:15       ` David Hildenbrand
  1 sibling, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2017-03-21  9:15 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li


>>> 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 | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 06d8080..b310214 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>>>       return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>>>  }
>>>
>>> +static inline bool cpu_has_vmx_invvpid(void)
>>> +{
>>> +     return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
>>> +}
>>> +
>>>  static inline bool cpu_has_vmx_ept(void)
>>>  {
>>>       return vmcs_config.cpu_based_2nd_exec_ctrl &
>>> @@ -6519,8 +6524,10 @@ static __init int hardware_setup(void)
>>>       if (boot_cpu_has(X86_FEATURE_NX))
>>>               kvm_enable_efer_bits(EFER_NX);
>>>
>>> -     if (!cpu_has_vmx_vpid())
>>> +     if (!cpu_has_vmx_vpid() ||
>>> +             !(cpu_has_vmx_invvpid()))
>>
>> This indentation looks weird. Can't this be fit into one line?
> 
> The same as cpu_has_vmx_ept_4levels().

I only know the general rules:

1. make things fit into one line unless it really harms readability
2. when splitting conditions over multiple lines, make them start at the
same level.

And I said, this indentation looks weird, because 1 and 2 are not met.

Anyhow, the general patch is fine in my opinion.

-- 

Thanks,

David

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

* Re: [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability
  2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
  2017-03-21  8:50   ` David Hildenbrand
@ 2017-03-21 16:19   ` Paolo Bonzini
  1 sibling, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2017-03-21 16:19 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm; +Cc: Radim Krčmář, Wanpeng Li



On 21/03/2017 05:18, Wanpeng Li wrote:
> -	if (!cpu_has_vmx_vpid())
> +	if (!cpu_has_vmx_vpid() ||
> +		!(cpu_has_vmx_invvpid()))

Too many parentheses and a useless line break.

Paolo

>  		enable_vpid = 0;

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

* Re: [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control
  2017-03-21  4:18 [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Wanpeng Li
  2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
  2017-03-21  4:18 ` [PATCH v2 3/3] KVM: x86: correct async page present tracepoint Wanpeng Li
@ 2017-03-21 16:19 ` Paolo Bonzini
  2 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2017-03-21 16:19 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm; +Cc: Radim Krčmář, Wanpeng Li

On 21/03/2017 05:18, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> This can be reproduced by running kvm-unit-tests/vmx.flat on L0 w/ vpid disabled.
> 
> Test suite: VPID
> Unhandled exception 6 #UD at ip 00000000004051a6
> error_code=0000      rflags=00010047      cs=00000008
> rax=0000000000000000 rcx=0000000000000001 rdx=0000000000000047 rbx=0000000000402f79
> rbp=0000000000456240 rsi=0000000000000001 rdi=0000000000000000
> r8=000000000000000a  r9=00000000000003f8 r10=0000000080010011 r11=0000000000000000
> r12=0000000000000003 r13=0000000000000708 r14=0000000000000000 r15=0000000000000000
> cr0=0000000080010031 cr2=0000000000000000 cr3=0000000007fff000 cr4=0000000000002020
> cr8=0000000000000000
> STACK: @4051a6 40523e 400f7f 402059 40028f
> 
> We should hide and forbid VPID in L1 if it is disabled on L0. However, nested VPID
> enable bit is set unconditionally during setup nested vmx exec controls though VPID 
> is not exposed through nested VMX capablity. This patch fixes it by don't set nested 
> VPID enable bit if it is disabled on L0.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Fixes: 5c614b3583e (KVM: nVMX: nested VPID emulation)
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 98e82ee..8795a70 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2753,7 +2753,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		SECONDARY_EXEC_RDTSCP |
>  		SECONDARY_EXEC_DESC |
>  		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
> -		SECONDARY_EXEC_ENABLE_VPID |
>  		SECONDARY_EXEC_APIC_REGISTER_VIRT |
>  		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  		SECONDARY_EXEC_WBINVD_EXITING |
> @@ -2781,10 +2780,12 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  	 * though it is treated as global context.  The alternative is
>  	 * not failing the single-context invvpid, and it is worse.
>  	 */
> -	if (enable_vpid)
> +	if (enable_vpid) {
> +		vmx->nested.nested_vmx_secondary_ctls_high |=
> +			SECONDARY_EXEC_ENABLE_VPID;
>  		vmx->nested.nested_vmx_vpid_caps = VMX_VPID_INVVPID_BIT |
>  			VMX_VPID_EXTENT_SUPPORTED_MASK;
> -	else
> +	} else
>  		vmx->nested.nested_vmx_vpid_caps = 0;
>  
>  	if (enable_unrestricted_guest)
> 

Applied patch 1 and 3 to kvm/master (well, locally for now).

Paolo

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

end of thread, other threads:[~2017-03-21 16:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-21  4:18 [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Wanpeng Li
2017-03-21  4:18 ` [PATCH v2 2/3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability Wanpeng Li
2017-03-21  8:50   ` David Hildenbrand
2017-03-21  8:58     ` Wanpeng Li
2017-03-21  9:01       ` David Hildenbrand
2017-03-21  9:05         ` Wanpeng Li
2017-03-21  9:15       ` David Hildenbrand
2017-03-21 16:19   ` Paolo Bonzini
2017-03-21  4:18 ` [PATCH v2 3/3] KVM: x86: correct async page present tracepoint Wanpeng Li
2017-03-21  8:57   ` David Hildenbrand
2017-03-21 16:19 ` [PATCH v2 1/3] KVM: nVMX: Fix nested VPID vmx exec control Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).