[PATCH v3 1/2] KVM: nVMX: enhance allocate/free_vpid to handle shadow vpid

[PATCH v3 1/2] KVM: nVMX: enhance allocate/free_vpid to handle shadow vpid

[Wanpeng Li]

Enhance allocate/free_vid to handle shadow vpid.

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9ff6a3f..4956081 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4155,29 +4155,27 @@ static int alloc_identity_pagetable(struct kvm *kvm)
 	return r;
 }
 
-static void allocate_vpid(struct vcpu_vmx *vmx)
+static int allocate_vpid(void)
 {
-	int vpid;
+	int vpid = 0;
 
-	vmx->vpid = 0;
 	if (!enable_vpid)
-		return;
+		return 0;
 	spin_lock(&vmx_vpid_lock);
 	vpid = find_first_zero_bit(vmx_vpid_bitmap, VMX_NR_VPIDS);
-	if (vpid < VMX_NR_VPIDS) {
-		vmx->vpid = vpid;
+	if (vpid < VMX_NR_VPIDS)
 		__set_bit(vpid, vmx_vpid_bitmap);
-	}
 	spin_unlock(&vmx_vpid_lock);
+	return vpid;
 }
 
-static void free_vpid(struct vcpu_vmx *vmx)
+static void free_vpid(int vpid)
 {
 	if (!enable_vpid)
 		return;
 	spin_lock(&vmx_vpid_lock);
-	if (vmx->vpid != 0)
-		__clear_bit(vmx->vpid, vmx_vpid_bitmap);
+	if (vpid != 0)
+		__clear_bit(vpid, vmx_vpid_bitmap);
 	spin_unlock(&vmx_vpid_lock);
 }
 
@@ -8482,7 +8480,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
 
 	if (enable_pml)
 		vmx_disable_pml(vmx);
-	free_vpid(vmx);
+	free_vpid(vmx->vpid);
 	leave_guest_mode(vcpu);
 	vmx_load_vmcs01(vcpu);
 	free_nested(vmx);
@@ -8501,7 +8499,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 	if (!vmx)
 		return ERR_PTR(-ENOMEM);
 
-	allocate_vpid(vmx);
+	vmx->vpid = allocate_vpid();
 
 	err = kvm_vcpu_init(&vmx->vcpu, kvm, id);
 	if (err)
@@ -8577,7 +8575,7 @@ free_msrs:
 uninit_vcpu:
 	kvm_vcpu_uninit(&vmx->vcpu);
 free_vcpu:
-	free_vpid(vmx);
+	free_vpid(vmx->vpid);
 	kmem_cache_free(kvm_vcpu_cache, vmx);
 	return ERR_PTR(err);
 }
-- 
1.9.1

[PATCH v3 2/2] KVM: nVMX: nested VPID emulation

[Wanpeng Li]

VPID is used to tag address space and avoid a TLB flush. Currently L0 use 
the same VPID to run L1 and all its guests. KVM flushes VPID when switching 
between L1 and L2. 

This patch advertises VPID to the L1 hypervisor, then address space of L1 and 
L2 can be separately treated and avoid TLB flush when swithing between L1 and 
L2. For each nested vmentry, if vpid12 is changed, reuse shadow vpid w/ an 
invvpid.

Performance: 

run lmbench on L2 w/ 3.5 kernel.

Context switching - times in microseconds - smaller is better
-------------------------------------------------------------------------
Host                 OS  2p/0K 2p/16K 2p/64K 8p/16K 8p/64K 16p/16K 16p/64K
                         ctxsw  ctxsw  ctxsw ctxsw  ctxsw   ctxsw   ctxsw
--------- ------------- ------ ------ ------ ------ ------ ------- -------
kernel    Linux 3.5.0-1 1.2200 1.3700 1.4500 4.7800 2.3300 5.60000 2.88000  nested VPID 
kernel    Linux 3.5.0-1 1.2600 1.4300 1.5600   12.7   12.9 3.49000 7.46000  vanilla

Suggested-by: Wincy Van <fanwenyi0529@gmail.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4956081..2fd5b5e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -424,6 +424,9 @@ struct nested_vmx {
 	/* to migrate it to L2 if VM_ENTRY_LOAD_DEBUG_CONTROLS is off */
 	u64 vmcs01_debugctl;
 
+	u16 vpid02;
+	u16 last_vpid;
+
 	u32 nested_vmx_procbased_ctls_low;
 	u32 nested_vmx_procbased_ctls_high;
 	u32 nested_vmx_true_procbased_ctls_low;
@@ -1155,6 +1158,11 @@ static inline bool nested_cpu_has_virt_x2apic_mode(struct vmcs12 *vmcs12)
 	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
 }
 
+static inline bool nested_cpu_has_vpid(struct vmcs12 *vmcs12)
+{
+	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_VPID);
+}
+
 static inline bool nested_cpu_has_apic_reg_virt(struct vmcs12 *vmcs12)
 {
 	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_APIC_REGISTER_VIRT);
@@ -2469,6 +2477,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 		SECONDARY_EXEC_RDTSCP |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
+		SECONDARY_EXEC_ENABLE_VPID |
 		SECONDARY_EXEC_APIC_REGISTER_VIRT |
 		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 		SECONDARY_EXEC_WBINVD_EXITING |
@@ -6662,6 +6671,7 @@ static void free_nested(struct vcpu_vmx *vmx)
 		return;
 
 	vmx->nested.vmxon = false;
+	free_vpid(vmx->nested.vpid02);
 	nested_release_vmcs12(vmx);
 	if (enable_shadow_vmcs)
 		free_vmcs(vmx->nested.current_shadow_vmcs);
@@ -8547,8 +8557,10 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 			goto free_vmcs;
 	}
 
-	if (nested)
+	if (nested) {
 		nested_vmx_setup_ctls_msrs(vmx);
+		vmx->nested.vpid02 = allocate_vpid();
+	}
 
 	vmx->nested.posted_intr_nv = -1;
 	vmx->nested.current_vmptr = -1ull;
@@ -8569,6 +8581,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 	return &vmx->vcpu;
 
 free_vmcs:
+	free_vpid(vmx->nested.vpid02);
 	free_loaded_vmcs(vmx->loaded_vmcs);
 free_msrs:
 	kfree(vmx->guest_msrs);
@@ -9444,12 +9457,24 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 
 	if (enable_vpid) {
 		/*
-		 * Trivially support vpid by letting L2s share their parent
-		 * L1's vpid. TODO: move to a more elaborate solution, giving
-		 * each L2 its own vpid and exposing the vpid feature to L1.
+		 * There is no direct mapping between vpid02 and vpid12, the
+		 * vpid02 is per-vCPU for L0 and reused while the value of
+		 * vpid12 is changed w/ one invvpid during nested vmentry.
+		 * The vpid12 is allocated by L1 for L2, so it will not
+		 * influence global bitmap(for vpid01 and vpid02 allocation)
+		 * even if spawn a lot of nested vCPUs.
 		 */
-		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
-		vmx_flush_tlb(vcpu);
+		if (nested_cpu_has_vpid(vmcs12)) {
+			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->nested.vpid02);
+			if (vmcs12->virtual_processor_id != vmx->nested.last_vpid) {
+				vmx->nested.last_vpid = vmcs12->virtual_processor_id;
+				vmx_flush_tlb(vcpu);
+			}
+		} else {
+			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
+			vmx_flush_tlb(vcpu);
+		}
+
 	}
 
 	if (nested_cpu_has_ept(vmcs12)) {
-- 
1.9.1

Re: [PATCH v3 1/2] KVM: nVMX: enhance allocate/free_vpid to handle shadow vpid

[Jan Kiszka]

On 2015-09-16 05:51, Wanpeng Li wrote:
> Enhance allocate/free_vid to handle shadow vpid.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 9ff6a3f..4956081 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -4155,29 +4155,27 @@ static int alloc_identity_pagetable(struct kvm *kvm)
>  	return r;
>  }
>  
> -static void allocate_vpid(struct vcpu_vmx *vmx)
> +static int allocate_vpid(void)
>  {
> -	int vpid;
> +	int vpid = 0;

Initialization is not pointless with the current code.

>  
> -	vmx->vpid = 0;
>  	if (!enable_vpid)
> -		return;
> +		return 0;
>  	spin_lock(&vmx_vpid_lock);
>  	vpid = find_first_zero_bit(vmx_vpid_bitmap, VMX_NR_VPIDS);
> -	if (vpid < VMX_NR_VPIDS) {
> -		vmx->vpid = vpid;
> +	if (vpid < VMX_NR_VPIDS)
>  		__set_bit(vpid, vmx_vpid_bitmap);
> -	}
>  	spin_unlock(&vmx_vpid_lock);
> +	return vpid;

You should return 0 also if vpid == VMX_NR_VPIDS.

>  }
>  
> -static void free_vpid(struct vcpu_vmx *vmx)
> +static void free_vpid(int vpid)
>  {
>  	if (!enable_vpid)

You could already test for vpid == 0 here...

>  		return;
>  	spin_lock(&vmx_vpid_lock);
> -	if (vmx->vpid != 0)
> -		__clear_bit(vmx->vpid, vmx_vpid_bitmap);
> +	if (vpid != 0)

...then you could skip this.

> +		__clear_bit(vpid, vmx_vpid_bitmap);
>  	spin_unlock(&vmx_vpid_lock);
>  }
>  
> @@ -8482,7 +8480,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
>  
>  	if (enable_pml)
>  		vmx_disable_pml(vmx);
> -	free_vpid(vmx);
> +	free_vpid(vmx->vpid);
>  	leave_guest_mode(vcpu);
>  	vmx_load_vmcs01(vcpu);
>  	free_nested(vmx);
> @@ -8501,7 +8499,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>  	if (!vmx)
>  		return ERR_PTR(-ENOMEM);
>  
> -	allocate_vpid(vmx);
> +	vmx->vpid = allocate_vpid();
>  
>  	err = kvm_vcpu_init(&vmx->vcpu, kvm, id);
>  	if (err)
> @@ -8577,7 +8575,7 @@ free_msrs:
>  uninit_vcpu:
>  	kvm_vcpu_uninit(&vmx->vcpu);
>  free_vcpu:
> -	free_vpid(vmx);
> +	free_vpid(vmx->vpid);
>  	kmem_cache_free(kvm_vcpu_cache, vmx);
>  	return ERR_PTR(err);
>  }
> 

Yes, this is what I had in mind.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v3 2/2] KVM: nVMX: nested VPID emulation

[Jan Kiszka]

On 2015-09-16 05:51, Wanpeng Li wrote:
> VPID is used to tag address space and avoid a TLB flush. Currently L0 use 
> the same VPID to run L1 and all its guests. KVM flushes VPID when switching 
> between L1 and L2. 
> 
> This patch advertises VPID to the L1 hypervisor, then address space of L1 and 
> L2 can be separately treated and avoid TLB flush when swithing between L1 and 
> L2. For each nested vmentry, if vpid12 is changed, reuse shadow vpid w/ an 
> invvpid.
> 
> Performance: 
> 
> run lmbench on L2 w/ 3.5 kernel.
> 
> Context switching - times in microseconds - smaller is better
> -------------------------------------------------------------------------
> Host                 OS  2p/0K 2p/16K 2p/64K 8p/16K 8p/64K 16p/16K 16p/64K
>                          ctxsw  ctxsw  ctxsw ctxsw  ctxsw   ctxsw   ctxsw
> --------- ------------- ------ ------ ------ ------ ------ ------- -------
> kernel    Linux 3.5.0-1 1.2200 1.3700 1.4500 4.7800 2.3300 5.60000 2.88000  nested VPID 
> kernel    Linux 3.5.0-1 1.2600 1.4300 1.5600   12.7   12.9 3.49000 7.46000  vanilla
> 
> Suggested-by: Wincy Van <fanwenyi0529@gmail.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 37 +++++++++++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4956081..2fd5b5e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -424,6 +424,9 @@ struct nested_vmx {
>  	/* to migrate it to L2 if VM_ENTRY_LOAD_DEBUG_CONTROLS is off */
>  	u64 vmcs01_debugctl;
>  
> +	u16 vpid02;
> +	u16 last_vpid;
> +
>  	u32 nested_vmx_procbased_ctls_low;
>  	u32 nested_vmx_procbased_ctls_high;
>  	u32 nested_vmx_true_procbased_ctls_low;
> @@ -1155,6 +1158,11 @@ static inline bool nested_cpu_has_virt_x2apic_mode(struct vmcs12 *vmcs12)
>  	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
>  }
>  
> +static inline bool nested_cpu_has_vpid(struct vmcs12 *vmcs12)
> +{
> +	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_VPID);
> +}
> +
>  static inline bool nested_cpu_has_apic_reg_virt(struct vmcs12 *vmcs12)
>  {
>  	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_APIC_REGISTER_VIRT);
> @@ -2469,6 +2477,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>  		SECONDARY_EXEC_RDTSCP |
>  		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
> +		SECONDARY_EXEC_ENABLE_VPID |
>  		SECONDARY_EXEC_APIC_REGISTER_VIRT |
>  		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  		SECONDARY_EXEC_WBINVD_EXITING |
> @@ -6662,6 +6671,7 @@ static void free_nested(struct vcpu_vmx *vmx)
>  		return;
>  
>  	vmx->nested.vmxon = false;
> +	free_vpid(vmx->nested.vpid02);
>  	nested_release_vmcs12(vmx);
>  	if (enable_shadow_vmcs)
>  		free_vmcs(vmx->nested.current_shadow_vmcs);
> @@ -8547,8 +8557,10 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>  			goto free_vmcs;
>  	}
>  
> -	if (nested)
> +	if (nested) {
>  		nested_vmx_setup_ctls_msrs(vmx);
> +		vmx->nested.vpid02 = allocate_vpid();
> +	}
>  
>  	vmx->nested.posted_intr_nv = -1;
>  	vmx->nested.current_vmptr = -1ull;
> @@ -8569,6 +8581,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>  	return &vmx->vcpu;
>  
>  free_vmcs:
> +	free_vpid(vmx->nested.vpid02);
>  	free_loaded_vmcs(vmx->loaded_vmcs);
>  free_msrs:
>  	kfree(vmx->guest_msrs);
> @@ -9444,12 +9457,24 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  
>  	if (enable_vpid) {
>  		/*
> -		 * Trivially support vpid by letting L2s share their parent
> -		 * L1's vpid. TODO: move to a more elaborate solution, giving
> -		 * each L2 its own vpid and exposing the vpid feature to L1.
> +		 * There is no direct mapping between vpid02 and vpid12, the
> +		 * vpid02 is per-vCPU for L0 and reused while the value of
> +		 * vpid12 is changed w/ one invvpid during nested vmentry.
> +		 * The vpid12 is allocated by L1 for L2, so it will not
> +		 * influence global bitmap(for vpid01 and vpid02 allocation)
> +		 * even if spawn a lot of nested vCPUs.
>  		 */
> -		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
> -		vmx_flush_tlb(vcpu);
> +		if (nested_cpu_has_vpid(vmcs12)) {
> +			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->nested.vpid02);
> +			if (vmcs12->virtual_processor_id != vmx->nested.last_vpid) {
> +				vmx->nested.last_vpid = vmcs12->virtual_processor_id;
> +				vmx_flush_tlb(vcpu);
> +			}
> +		} else {
> +			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
> +			vmx_flush_tlb(vcpu);
> +		}
> +
>  	}
>  
>  	if (nested_cpu_has_ept(vmcs12)) {
> 

Looks good to me.

Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v3 1/2] KVM: nVMX: enhance allocate/free_vpid to handle shadow vpid

[Wanpeng Li]

On 9/16/15 2:42 PM, Jan Kiszka wrote:
> On 2015-09-16 05:51, Wanpeng Li wrote:
>> Enhance allocate/free_vid to handle shadow vpid.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>   arch/x86/kvm/vmx.c | 24 +++++++++++-------------
>>   1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 9ff6a3f..4956081 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -4155,29 +4155,27 @@ static int alloc_identity_pagetable(struct kvm *kvm)
>>   	return r;
>>   }
>>   
>> -static void allocate_vpid(struct vcpu_vmx *vmx)
>> +static int allocate_vpid(void)
>>   {
>> -	int vpid;
>> +	int vpid = 0;
> Initialization is not pointless with the current code.
>
>>   
>> -	vmx->vpid = 0;
>>   	if (!enable_vpid)
>> -		return;
>> +		return 0;
>>   	spin_lock(&vmx_vpid_lock);
>>   	vpid = find_first_zero_bit(vmx_vpid_bitmap, VMX_NR_VPIDS);
>> -	if (vpid < VMX_NR_VPIDS) {
>> -		vmx->vpid = vpid;
>> +	if (vpid < VMX_NR_VPIDS)
>>   		__set_bit(vpid, vmx_vpid_bitmap);
>> -	}
>>   	spin_unlock(&vmx_vpid_lock);
>> +	return vpid;
> You should return 0 also if vpid == VMX_NR_VPIDS.

Agreed.

>
>>   }
>>   
>> -static void free_vpid(struct vcpu_vmx *vmx)
>> +static void free_vpid(int vpid)
>>   {
>>   	if (!enable_vpid)
> You could already test for vpid == 0 here...
>
>>   		return;
>>   	spin_lock(&vmx_vpid_lock);
>> -	if (vmx->vpid != 0)
>> -		__clear_bit(vmx->vpid, vmx_vpid_bitmap);
>> +	if (vpid != 0)
> ...then you could skip this.

Agreed.

>
>> +		__clear_bit(vpid, vmx_vpid_bitmap);
>>   	spin_unlock(&vmx_vpid_lock);
>>   }
>>   
>> @@ -8482,7 +8480,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
>>   
>>   	if (enable_pml)
>>   		vmx_disable_pml(vmx);
>> -	free_vpid(vmx);
>> +	free_vpid(vmx->vpid);
>>   	leave_guest_mode(vcpu);
>>   	vmx_load_vmcs01(vcpu);
>>   	free_nested(vmx);
>> @@ -8501,7 +8499,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>>   	if (!vmx)
>>   		return ERR_PTR(-ENOMEM);
>>   
>> -	allocate_vpid(vmx);
>> +	vmx->vpid = allocate_vpid();
>>   
>>   	err = kvm_vcpu_init(&vmx->vcpu, kvm, id);
>>   	if (err)
>> @@ -8577,7 +8575,7 @@ free_msrs:
>>   uninit_vcpu:
>>   	kvm_vcpu_uninit(&vmx->vcpu);
>>   free_vcpu:
>> -	free_vpid(vmx);
>> +	free_vpid(vmx->vpid);
>>   	kmem_cache_free(kvm_vcpu_cache, vmx);
>>   	return ERR_PTR(err);
>>   }
>>
> Yes, this is what I had in mind.

Thanks for your review. :-)

Regards,
Wanpeng Li