[PATCH v2] KVM: nVMX: expose VPID capability to L1

[PATCH v2] KVM: nVMX: expose VPID capability to L1

[Wanpeng Li]

Expose VPID capability to L1.

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v1 -> v2:
 * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT

 arch/x86/kvm/vmx.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 75f3ee0..c4ea890 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -442,7 +442,7 @@ struct nested_vmx {
 	u32 nested_vmx_true_entry_ctls_low;
 	u32 nested_vmx_misc_low;
 	u32 nested_vmx_misc_high;
-	u32 nested_vmx_ept_caps;
+	u64 nested_vmx_ept_vpid_caps;
 };
 
 #define POSTED_INTR_ON  0
@@ -2485,22 +2485,23 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		SECONDARY_EXEC_WBINVD_EXITING |
 		SECONDARY_EXEC_XSAVES;
 
-	if (enable_ept) {
+	if (enable_ept | enable_vpid) {
 		/* nested EPT: emulate EPT also to L1 */
 		vmx->nested.nested_vmx_secondary_ctls_high |=
 			SECONDARY_EXEC_ENABLE_EPT;
-		vmx->nested.nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT |
+		vmx->nested.nested_vmx_ept_vpid_caps = VMX_EPT_PAGE_WALK_4_BIT |
 			 VMX_EPTP_WB_BIT | VMX_EPT_2MB_PAGE_BIT |
 			 VMX_EPT_INVEPT_BIT;
-		vmx->nested.nested_vmx_ept_caps &= vmx_capability.ept;
+		vmx->nested.nested_vmx_ept_vpid_caps &= vmx_capability.ept;
 		/*
 		 * For nested guests, we don't do anything specific
 		 * for single context invalidation. Hence, only advertise
 		 * support for global context invalidation.
 		 */
-		vmx->nested.nested_vmx_ept_caps |= VMX_EPT_EXTENT_GLOBAL_BIT;
+		vmx->nested.nested_vmx_ept_vpid_caps |= VMX_EPT_EXTENT_GLOBAL_BIT;
+		vmx->nested.nested_vmx_ept_vpid_caps |= VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT << 32;
 	} else
-		vmx->nested.nested_vmx_ept_caps = 0;
+		vmx->nested.nested_vmx_ept_vpid_caps = 0;
 
 	if (enable_unrestricted_guest)
 		vmx->nested.nested_vmx_secondary_ctls_high |=
@@ -2616,8 +2617,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 			vmx->nested.nested_vmx_secondary_ctls_high);
 		break;
 	case MSR_IA32_VMX_EPT_VPID_CAP:
-		/* Currently, no nested vpid support */
-		*pdata = vmx->nested.nested_vmx_ept_caps;
+		*pdata = vmx->nested.nested_vmx_ept_vpid_caps;
 		break;
 	default:
 		return 1;
@@ -7142,7 +7142,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
 
 	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
 	      SECONDARY_EXEC_ENABLE_EPT) ||
-	    !(vmx->nested.nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) {
+	    !(vmx->nested.nested_vmx_ept_vpid_caps & VMX_EPT_INVEPT_BIT)) {
 		kvm_queue_exception(vcpu, UD_VECTOR);
 		return 1;
 	}
@@ -7158,7 +7158,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
 	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
 	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
 
-	types = (vmx->nested.nested_vmx_ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6;
+	types = (vmx->nested.nested_vmx_ept_vpid_caps >> VMX_EPT_EXTENT_SHIFT) & 6;
 
 	if (!(types & (1UL << type))) {
 		nested_vmx_failValid(vcpu,
@@ -8763,7 +8763,7 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
 {
 	WARN_ON(mmu_is_nested(vcpu));
 	kvm_init_shadow_ept_mmu(vcpu,
-			to_vmx(vcpu)->nested.nested_vmx_ept_caps &
+			to_vmx(vcpu)->nested.nested_vmx_ept_vpid_caps &
 			VMX_EPT_EXECUTE_ONLY_BIT);
 	vcpu->arch.mmu.set_cr3           = vmx_set_cr3;
 	vcpu->arch.mmu.get_cr3           = nested_ept_get_cr3;
-- 
1.9.1

Re: [PATCH v2] KVM: nVMX: expose VPID capability to L1

[Paolo Bonzini]

On 29/09/2015 04:55, Wanpeng Li wrote:
> Expose VPID capability to L1.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> v1 -> v2:
>  * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT

Thanks.  I've checked more thoroughly your implementation against the
SDM now, and there are a few missing things between this patch and the
one that emulates INVVPID:

- you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR

- you were not checking against the supported types in the
implementation of the INVVPID instruction

- the memory operand must always be read even if it isn't needed (e.g.,
for type==global), similar to INVEPT

- for single-context invalidation you're not checking that VPID != 0,
though in practice that doesn't matter because we don't want to support
single-context invalidation

- you're always setting the MSR's bits to 1 even if !enable_vpid

At this point it's better if you resend the whole nested VPID
implementation, i.e. the following five patches:

KVM: VMX: adjust interface to allocate/free_vpid
KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid
KVM: nVMX: emulate the INVVPID instruction
KVM: nVMX: nested VPID emulation
KVM: nVMX: expose VPID capability to L1

with the above issues fixed.  Please also send kvm-unit-tests patches
that tests for the error cases.

Thanks,

Paolo

>  arch/x86/kvm/vmx.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 75f3ee0..c4ea890 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -442,7 +442,7 @@ struct nested_vmx {
>  	u32 nested_vmx_true_entry_ctls_low;
>  	u32 nested_vmx_misc_low;
>  	u32 nested_vmx_misc_high;
> -	u32 nested_vmx_ept_caps;
> +	u64 nested_vmx_ept_vpid_caps;
>  };
>  
>  #define POSTED_INTR_ON  0
> @@ -2485,22 +2485,23 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		SECONDARY_EXEC_WBINVD_EXITING |
>  		SECONDARY_EXEC_XSAVES;
>  
> -	if (enable_ept) {
> +	if (enable_ept | enable_vpid) {
>  		/* nested EPT: emulate EPT also to L1 */
>  		vmx->nested.nested_vmx_secondary_ctls_high |=
>  			SECONDARY_EXEC_ENABLE_EPT;
> -		vmx->nested.nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT |
> +		vmx->nested.nested_vmx_ept_vpid_caps = VMX_EPT_PAGE_WALK_4_BIT |
>  			 VMX_EPTP_WB_BIT | VMX_EPT_2MB_PAGE_BIT |
>  			 VMX_EPT_INVEPT_BIT;
> -		vmx->nested.nested_vmx_ept_caps &= vmx_capability.ept;
> +		vmx->nested.nested_vmx_ept_vpid_caps &= vmx_capability.ept;
>  		/*
>  		 * For nested guests, we don't do anything specific
>  		 * for single context invalidation. Hence, only advertise
>  		 * support for global context invalidation.
>  		 */
> -		vmx->nested.nested_vmx_ept_caps |= VMX_EPT_EXTENT_GLOBAL_BIT;
> +		vmx->nested.nested_vmx_ept_vpid_caps |= VMX_EPT_EXTENT_GLOBAL_BIT;
> +		vmx->nested.nested_vmx_ept_vpid_caps |= VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT << 32;
>  	} else
> -		vmx->nested.nested_vmx_ept_caps = 0;
> +		vmx->nested.nested_vmx_ept_vpid_caps = 0;
>  
>  	if (enable_unrestricted_guest)
>  		vmx->nested.nested_vmx_secondary_ctls_high |=
> @@ -2616,8 +2617,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
>  			vmx->nested.nested_vmx_secondary_ctls_high);
>  		break;
>  	case MSR_IA32_VMX_EPT_VPID_CAP:
> -		/* Currently, no nested vpid support */
> -		*pdata = vmx->nested.nested_vmx_ept_caps;
> +		*pdata = vmx->nested.nested_vmx_ept_vpid_caps;
>  		break;
>  	default:
>  		return 1;
> @@ -7142,7 +7142,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>  
>  	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
>  	      SECONDARY_EXEC_ENABLE_EPT) ||
> -	    !(vmx->nested.nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) {
> +	    !(vmx->nested.nested_vmx_ept_vpid_caps & VMX_EPT_INVEPT_BIT)) {
>  		kvm_queue_exception(vcpu, UD_VECTOR);
>  		return 1;
>  	}
> @@ -7158,7 +7158,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>  	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
>  	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
>  
> -	types = (vmx->nested.nested_vmx_ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6;
> +	types = (vmx->nested.nested_vmx_ept_vpid_caps >> VMX_EPT_EXTENT_SHIFT) & 6;
>  
>  	if (!(types & (1UL << type))) {
>  		nested_vmx_failValid(vcpu,
> @@ -8763,7 +8763,7 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
>  {
>  	WARN_ON(mmu_is_nested(vcpu));
>  	kvm_init_shadow_ept_mmu(vcpu,
> -			to_vmx(vcpu)->nested.nested_vmx_ept_caps &
> +			to_vmx(vcpu)->nested.nested_vmx_ept_vpid_caps &
>  			VMX_EPT_EXECUTE_ONLY_BIT);
>  	vcpu->arch.mmu.set_cr3           = vmx_set_cr3;
>  	vcpu->arch.mmu.get_cr3           = nested_ept_get_cr3;
> 

Re: [PATCH v2] KVM: nVMX: expose VPID capability to L1

[Wanpeng Li]

On 9/29/15 6:39 PM, Paolo Bonzini wrote:
> On 29/09/2015 04:55, Wanpeng Li wrote:
>> Expose VPID capability to L1.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> v1 -> v2:
>>   * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT
> Thanks.  I've checked more thoroughly your implementation against the
> SDM now, and there are a few missing things between this patch and the
> one that emulates INVVPID:
>
> - you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR
>
> - you were not checking against the supported types in the
> implementation of the INVVPID instruction
>
> - the memory operand must always be read even if it isn't needed (e.g.,
> for type==global), similar to INVEPT
>
> - for single-context invalidation you're not checking that VPID != 0,
> though in practice that doesn't matter because we don't want to support
> single-context invalidation
>
> - you're always setting the MSR's bits to 1 even if !enable_vpid
>
> At this point it's better if you resend the whole nested VPID
> implementation, i.e. the following five patches:
>
> KVM: VMX: adjust interface to allocate/free_vpid
> KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid
> KVM: nVMX: emulate the INVVPID instruction
> KVM: nVMX: nested VPID emulation
> KVM: nVMX: expose VPID capability to L1
>
> with the above issues fixed.  Please also send kvm-unit-tests patches
> that tests for the error cases.

Ok, I will do this after the vacation(until 10.7) in my country. :-)

Regards,
Wanpeng Li

Re: [PATCH v2] KVM: nVMX: expose VPID capability to L1

[Wanpeng Li]

On 9/29/15 6:39 PM, Paolo Bonzini wrote:
> On 29/09/2015 04:55, Wanpeng Li wrote:
>> Expose VPID capability to L1.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> v1 -> v2:
>>   * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT
> Thanks.  I've checked more thoroughly your implementation against the
> SDM now, and there are a few missing things between this patch and the
> one that emulates INVVPID:
>
> - you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR
>
> - you were not checking against the supported types in the
> implementation of the INVVPID instruction
>
> - the memory operand must always be read even if it isn't needed (e.g.,
> for type==global), similar to INVEPT
>
> - for single-context invalidation you're not checking that VPID != 0,
> though in practice that doesn't matter because we don't want to support
> single-context invalidation
>
> - you're always setting the MSR's bits to 1 even if !enable_vpid

I believe you mean always setting the MSR's bits to 1 when !enable_ept 
and enable_vpid.

>
> At this point it's better if you resend the whole nested VPID
> implementation, i.e. the following five patches:
>
> KVM: VMX: adjust interface to allocate/free_vpid
> KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid
> KVM: nVMX: emulate the INVVPID instruction
> KVM: nVMX: nested VPID emulation
> KVM: nVMX: expose VPID capability to L1
>
> with the above issues fixed.

I just send out v2.

> Please also send kvm-unit-tests patches
> that tests for the error cases.

I will do it later.

Regards,
Wanpeng Li