diff mbox series

[1/4] KVM: nVMX: Flush TLB entries tagged by dest EPTP on L1<->L2 transitions

Message ID 20180906133228.118282-2-liran.alon@oracle.com (mailing list archive)
State New, archived
Headers show
Series : KVM: nVMX: Consider TLB are tagged with different EPTP if L1 uses EPT | expand

Commit Message

Liran Alon Sept. 6, 2018, 1:32 p.m. UTC 
If L1 and L2 share VPID (because L1 don't use VPID or we haven't allocated
a vpid02), we need to flush TLB on L1<->L2 transitions.

Before this patch, this TLB flushing was done by vmx_flush_tlb().
If L0 use EPT, this will translate into INVEPT(active_eptp);
However, if L1 use EPT, in L1->L2 VMEntry, active EPTP is EPTP01 but
TLB entries populated by L2 are tagged with EPTP02.
Therefore we should delay vmx_flush_tlb() until active_eptp is EPTP02.

To achieve this, instead of directly calling vmx_flush_tlb() we request
it to be called by KVM_REQ_TLB_FLUSH which is evaluated after
KVM_REQ_LOAD_CR3 which sets the active_eptp to EPTP02 as required.

Similarly, on L2->L1 VMExit, active EPTP is EPTP02 but TLB entries
populated by L1 are tagged with EPTP01 and therefore we should delay
vmx_flush_tlb() until active_eptp is EPTP01.

Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Nikita Leshchenko <nikita.leshchenko@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 arch/x86/kvm/vmx.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Oct. 1, 2018, 12:51 p.m. UTC | #1 
On 06/09/2018 15:32, Liran Alon wrote:
> If L1 and L2 share VPID (because L1 don't use VPID or we haven't allocated
> a vpid02), we need to flush TLB on L1<->L2 transitions.
> 
> Before this patch, this TLB flushing was done by vmx_flush_tlb().
> If L0 use EPT, this will translate into INVEPT(active_eptp);
> However, if L1 use EPT, in L1->L2 VMEntry, active EPTP is EPTP01 but
> TLB entries populated by L2 are tagged with EPTP02.
> Therefore we should delay vmx_flush_tlb() until active_eptp is EPTP02.
> 
> To achieve this, instead of directly calling vmx_flush_tlb() we request
> it to be called by KVM_REQ_TLB_FLUSH which is evaluated after
> KVM_REQ_LOAD_CR3 which sets the active_eptp to EPTP02 as required.
> 
> Similarly, on L2->L1 VMExit, active EPTP is EPTP02 but TLB entries
> populated by L1 are tagged with EPTP01 and therefore we should delay
> vmx_flush_tlb() until active_eptp is EPTP01.
> 
> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> Reviewed-by: Nikita Leshchenko <nikita.leshchenko@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  arch/x86/kvm/vmx.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index bed75118111c..7dcf5e96039d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -12245,7 +12245,15 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>  				__vmx_flush_tlb(vcpu, vmx->nested.vpid02, true);
>  			}
>  		} else {
> -			vmx_flush_tlb(vcpu, true);
> +			/*
> +			 * If L1 use EPT, then L0 needs to execute INVEPT on
> +			 * EPTP02 instead of EPTP01. Therefore, delay TLB
> +			 * flush until vmcs02->eptp is fully updated by
> +			 * KVM_REQ_LOAD_CR3. Note that this assumes
> +			 * KVM_REQ_TLB_FLUSH is evaluated after
> +			 * KVM_REQ_LOAD_CR3 in vcpu_enter_guest().
> +			 */
> +			kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>  		}
>  	}
>  
> @@ -13131,10 +13139,14 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
>  	 * Therefore, flush TLB only in case vmcs01 uses VPID and
>  	 * vmcs12 don't use VPID as in this case L1 & L2 TLB entries
>  	 * are both tagged with vmx->vpid.
> +	 *
> +	 * If vmcs12 uses EPT, we need to execute this flush on EPTP01
> +	 * and therefore we request the TLB flush to happen only after VMCS EPTP
> +	 * has been set by KVM_REQ_LOAD_CR3.
>  	 */
>  	if (enable_vpid &&
> -	    !(nested_cpu_has_vpid(vmcs12) && to_vmx(vcpu)->nested.vpid02)) {
> -		vmx_flush_tlb(vcpu, true);
> +		!(nested_cpu_has_vpid(vmcs12) && to_vmx(vcpu)->nested.vpid02)) {
> +		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>  	}
>  
>  	vmcs_write32(GUEST_SYSENTER_CS, vmcs12->host_ia32_sysenter_cs);
> 

Fixes: 6e42782f516f05c8030f63308f2457681b1c9919

Queued, thanks.

Paolo
Liran Alon Oct. 7, 2018, 8:36 p.m. UTC | #2 
> On 1 Oct 2018, at 15:51, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 06/09/2018 15:32, Liran Alon wrote:
>> If L1 and L2 share VPID (because L1 don't use VPID or we haven't allocated
>> a vpid02), we need to flush TLB on L1<->L2 transitions.
>> 
>> Before this patch, this TLB flushing was done by vmx_flush_tlb().
>> If L0 use EPT, this will translate into INVEPT(active_eptp);
>> However, if L1 use EPT, in L1->L2 VMEntry, active EPTP is EPTP01 but
>> TLB entries populated by L2 are tagged with EPTP02.
>> Therefore we should delay vmx_flush_tlb() until active_eptp is EPTP02.
>> 
>> To achieve this, instead of directly calling vmx_flush_tlb() we request
>> it to be called by KVM_REQ_TLB_FLUSH which is evaluated after
>> KVM_REQ_LOAD_CR3 which sets the active_eptp to EPTP02 as required.
>> 
>> Similarly, on L2->L1 VMExit, active EPTP is EPTP02 but TLB entries
>> populated by L1 are tagged with EPTP01 and therefore we should delay
>> vmx_flush_tlb() until active_eptp is EPTP01.
>> 
> 
> Fixes: 6e42782f516f05c8030f63308f2457681b1c9919
> 
> Queued, thanks.
> 
> Paolo

I don’t see how this patch fixes 6e42782f516f05c8030f63308f2457681b1c9919.

-Liran
Paolo Bonzini Oct. 8, 2018, 1:17 p.m. UTC | #3 
On 07/10/2018 22:36, Liran Alon wrote:
>>>
>>> To achieve this, instead of directly calling vmx_flush_tlb() we request
>>> it to be called by KVM_REQ_TLB_FLUSH which is evaluated after
>>> KVM_REQ_LOAD_CR3 which sets the active_eptp to EPTP02 as required.
>>>
>>> Similarly, on L2->L1 VMExit, active EPTP is EPTP02 but TLB entries
>>> populated by L1 are tagged with EPTP01 and therefore we should delay
>>> vmx_flush_tlb() until active_eptp is EPTP01.
>>>
>> Fixes: 6e42782f516f05c8030f63308f2457681b1c9919
>>
>> Queued, thanks.
>>
>> Paolo
> I don’t see how this patch fixes 6e42782f516f05c8030f63308f2457681b1c9919.

Indeed, I mistakenly thought that the flush was after kvm_mmu_load of
the nested EPT context (so that patch would have put it afterwards), but
it is actually the TLB flush that comes first.

Paolo
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index bed75118111c..7dcf5e96039d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -12245,7 +12245,15 @@  static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 				__vmx_flush_tlb(vcpu, vmx->nested.vpid02, true);
 			}
 		} else {
-			vmx_flush_tlb(vcpu, true);
+			/*
+			 * If L1 use EPT, then L0 needs to execute INVEPT on
+			 * EPTP02 instead of EPTP01. Therefore, delay TLB
+			 * flush until vmcs02->eptp is fully updated by
+			 * KVM_REQ_LOAD_CR3. Note that this assumes
+			 * KVM_REQ_TLB_FLUSH is evaluated after
+			 * KVM_REQ_LOAD_CR3 in vcpu_enter_guest().
+			 */
+			kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 		}
 	}
 
@@ -13131,10 +13139,14 @@  static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	 * Therefore, flush TLB only in case vmcs01 uses VPID and
 	 * vmcs12 don't use VPID as in this case L1 & L2 TLB entries
 	 * are both tagged with vmx->vpid.
+	 *
+	 * If vmcs12 uses EPT, we need to execute this flush on EPTP01
+	 * and therefore we request the TLB flush to happen only after VMCS EPTP
+	 * has been set by KVM_REQ_LOAD_CR3.
 	 */
 	if (enable_vpid &&
-	    !(nested_cpu_has_vpid(vmcs12) && to_vmx(vcpu)->nested.vpid02)) {
-		vmx_flush_tlb(vcpu, true);
+		!(nested_cpu_has_vpid(vmcs12) && to_vmx(vcpu)->nested.vpid02)) {
+		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 	}
 
 	vmcs_write32(GUEST_SYSENTER_CS, vmcs12->host_ia32_sysenter_cs);