[v3,6/8] KVM-HV: Add flush_on_enter before guest enter

Message ID	20120731104859.16662.8738.stgit@abhimanyu.in.ibm.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <kvm-owner@vger.kernel.org> Gateway: Authorized Use Only! Violators will be prosecuted for <kvm@vger.kernel.org> from <nikunj@linux.vnet.ibm.com>; Tue, 31 Jul 2012 20:49:26 +1000 Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 31 Jul 2012 20:49:23 +1000 Subject: [PATCH v3 6/8] KVM-HV: Add flush_on_enter before guest enter To: peterz@infradead.org, mtosatti@redhat.com, avi@redhat.com From: "Nikunj A. Dadhania" <nikunj@linux.vnet.ibm.com> Cc: raghukt@linux.vnet.ibm.com, alex.shi@intel.com, mingo@elte.hu, kvm@vger.kernel.org, hpa@zytor.com Date: Tue, 31 Jul 2012 16:19:02 +0530 Message-ID: <20120731104859.16662.8738.stgit@abhimanyu.in.ibm.com> In-Reply-To: <20120731104312.16662.27889.stgit@abhimanyu.in.ibm.com> References: <20120731104312.16662.27889.stgit@abhimanyu.in.ibm.com> User-Agent: StGit/0.16-2-g0d85 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: kvm-owner@vger.kernel.org Precedence: bulk

Message ID

20120731104859.16662.8738.stgit@abhimanyu.in.ibm.com (mailing list archive)

State

New, archived

Headers

Subject: [PATCH v3 6/8] KVM-HV: Add flush_on_enter before guest enter
To: peterz@infradead.org, mtosatti@redhat.com, avi@redhat.com
From: "Nikunj A. Dadhania" <nikunj@linux.vnet.ibm.com>
Cc: raghukt@linux.vnet.ibm.com, alex.shi@intel.com, mingo@elte.hu,
	kvm@vger.kernel.org, hpa@zytor.com
Date: Tue, 31 Jul 2012 16:19:02 +0530
Message-ID: <20120731104859.16662.8738.stgit@abhimanyu.in.ibm.com>
In-Reply-To: <20120731104312.16662.27889.stgit@abhimanyu.in.ibm.com>
References: <20120731104312.16662.27889.stgit@abhimanyu.in.ibm.com>
User-Agent: StGit/0.16-2-g0d85
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Sender: kvm-owner@vger.kernel.org
Precedence: bulk

Commit Message

Nikunj A. Dadhania July 31, 2012, 10:49 a.m. UTC

From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>

PV-Flush guest would indicate to flush on enter, flush the TLB before
entering and exiting the guest.

Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
---
 arch/x86/kvm/x86.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marcelo Tosatti Aug. 2, 2012, 8:14 p.m. UTC | #1

On Tue, Jul 31, 2012 at 04:19:02PM +0530, Nikunj A. Dadhania wrote:
> From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> 
> PV-Flush guest would indicate to flush on enter, flush the TLB before
> entering and exiting the guest.
> 
> Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  arch/x86/kvm/x86.c |   23 +++++++----------------
>  1 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 580abcf..a67e971 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1557,20 +1557,9 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>  		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time));
>  }
>  
> -static void kvm_set_atomic(u64 *addr, u64 old, u64 new)
> -{
> -	int loop = 1000000;
> -	while (1) {
> -		if (cmpxchg(addr, old, new) == old)
> -			break;
> -		loop--;
> -		if (!loop) {
> -			pr_info("atomic cur: %lx old: %lx new: %lx\n",
> -				*addr, old, new);
> -			break;
> -		}
> -	}
> -}
> +#define VS_NOT_IN_GUEST      (0)
> +#define VS_IN_GUEST          (1 << KVM_VCPU_STATE_IN_GUEST_MODE)
> +#define VS_SHOULD_FLUSH      (1 << KVM_VCPU_STATE_SHOULD_FLUSH)
>  
>  static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
>  {
> @@ -1584,7 +1573,8 @@ static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
>  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
>  	kaddr += vcpu->arch.v_state.vs_offset;
>  	vs = kaddr;
> -	kvm_set_atomic(&vs->state, 0, 1 << KVM_VCPU_STATE_IN_GUEST_MODE);
> +	if (xchg(&vs->state, VS_IN_GUEST) == VS_SHOULD_FLUSH)
> +		kvm_x86_ops->tlb_flush(vcpu);
>  	kunmap_atomic(kaddr);
>  }
>  
> @@ -1600,7 +1590,8 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
>  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
>  	kaddr += vcpu->arch.v_state.vs_offset;
>  	vs = kaddr;
> -	kvm_set_atomic(&vs->state, 1 << KVM_VCPU_STATE_IN_GUEST_MODE, 0);
> +	if (xchg(&vs->state, VS_NOT_IN_GUEST) == VS_SHOULD_FLUSH)
> +		kvm_x86_ops->tlb_flush(vcpu);
>  	kunmap_atomic(kaddr);
>  }

Nevermind the early comment (the other comments on that message are
valid).

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marcelo Tosatti Aug. 2, 2012, 8:16 p.m. UTC | #2

On Thu, Aug 02, 2012 at 05:14:02PM -0300, Marcelo Tosatti wrote:
> On Tue, Jul 31, 2012 at 04:19:02PM +0530, Nikunj A. Dadhania wrote:
> > From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> > 
> > PV-Flush guest would indicate to flush on enter, flush the TLB before
> > entering and exiting the guest.
> > 
> > Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> > ---
> >  arch/x86/kvm/x86.c |   23 +++++++----------------
> >  1 files changed, 7 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 580abcf..a67e971 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1557,20 +1557,9 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> >  		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time));
> >  }
> >  
> > -static void kvm_set_atomic(u64 *addr, u64 old, u64 new)
> > -{
> > -	int loop = 1000000;
> > -	while (1) {
> > -		if (cmpxchg(addr, old, new) == old)
> > -			break;
> > -		loop--;
> > -		if (!loop) {
> > -			pr_info("atomic cur: %lx old: %lx new: %lx\n",
> > -				*addr, old, new);
> > -			break;
> > -		}
> > -	}
> > -}
> > +#define VS_NOT_IN_GUEST      (0)
> > +#define VS_IN_GUEST          (1 << KVM_VCPU_STATE_IN_GUEST_MODE)
> > +#define VS_SHOULD_FLUSH      (1 << KVM_VCPU_STATE_SHOULD_FLUSH)
> >  
> >  static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> >  {
> > @@ -1584,7 +1573,8 @@ static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> >  	kaddr += vcpu->arch.v_state.vs_offset;
> >  	vs = kaddr;
> > -	kvm_set_atomic(&vs->state, 0, 1 << KVM_VCPU_STATE_IN_GUEST_MODE);
> > +	if (xchg(&vs->state, VS_IN_GUEST) == VS_SHOULD_FLUSH)
> > +		kvm_x86_ops->tlb_flush(vcpu);
> >  	kunmap_atomic(kaddr);
> >  }
> >  
> > @@ -1600,7 +1590,8 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
> >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> >  	kaddr += vcpu->arch.v_state.vs_offset;
> >  	vs = kaddr;
> > -	kvm_set_atomic(&vs->state, 1 << KVM_VCPU_STATE_IN_GUEST_MODE, 0);
> > +	if (xchg(&vs->state, VS_NOT_IN_GUEST) == VS_SHOULD_FLUSH)
> > +		kvm_x86_ops->tlb_flush(vcpu);
> >  	kunmap_atomic(kaddr);
> >  }
> 
> Nevermind the early comment (the other comments on that message are
> valid).

Ah, so the pseucode mentions flush-on-exit because we can be clearing 
the flag on xchg. Setting KVM_REQ_TLB_FLUSH instead should be enough
(please verify).

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Nikunj A. Dadhania Aug. 3, 2012, 5:37 a.m. UTC | #3

On Thu, 2 Aug 2012 17:16:41 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Thu, Aug 02, 2012 at 05:14:02PM -0300, Marcelo Tosatti wrote:
> > On Tue, Jul 31, 2012 at 04:19:02PM +0530, Nikunj A. Dadhania wrote:
> > > From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> > >  
> > >  static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> > >  {
> > > @@ -1584,7 +1573,8 @@ static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> > >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> > >  	kaddr += vcpu->arch.v_state.vs_offset;
> > >  	vs = kaddr;
> > > -	kvm_set_atomic(&vs->state, 0, 1 << KVM_VCPU_STATE_IN_GUEST_MODE);
> > > +	if (xchg(&vs->state, VS_IN_GUEST) == VS_SHOULD_FLUSH)
> > > +		kvm_x86_ops->tlb_flush(vcpu);
> > >  	kunmap_atomic(kaddr);
> > >  }
> > >  
> > > @@ -1600,7 +1590,8 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
> > >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> > >  	kaddr += vcpu->arch.v_state.vs_offset;
> > >  	vs = kaddr;
> > > -	kvm_set_atomic(&vs->state, 1 << KVM_VCPU_STATE_IN_GUEST_MODE, 0);
> > > +	if (xchg(&vs->state, VS_NOT_IN_GUEST) == VS_SHOULD_FLUSH)
> > > +		kvm_x86_ops->tlb_flush(vcpu);
> > >  	kunmap_atomic(kaddr);
> > >  }
> > 
> > Nevermind the early comment (the other comments on that message are
> > valid).
I assume the above is related to kvm_set_atomic comment in the [3/8]

> 
> Ah, so the pseucode mentions flush-on-exit because we can be clearing 
> the flag on xchg. Setting KVM_REQ_TLB_FLUSH instead should be enough
> (please verify).
> 
Yes, that will work while exiting. 

In the vm_enter case, we need to do a kvm_x86_ops->tlb_flush(vcpu), as
we have already passed the phase of checking the request.

Nikunj

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marcelo Tosatti Aug. 3, 2012, 5:31 p.m. UTC | #4

On Fri, Aug 03, 2012 at 11:07:13AM +0530, Nikunj A Dadhania wrote:
> On Thu, 2 Aug 2012 17:16:41 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Thu, Aug 02, 2012 at 05:14:02PM -0300, Marcelo Tosatti wrote:
> > > On Tue, Jul 31, 2012 at 04:19:02PM +0530, Nikunj A. Dadhania wrote:
> > > > From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> > > >  
> > > >  static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> > > >  {
> > > > @@ -1584,7 +1573,8 @@ static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
> > > >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> > > >  	kaddr += vcpu->arch.v_state.vs_offset;
> > > >  	vs = kaddr;
> > > > -	kvm_set_atomic(&vs->state, 0, 1 << KVM_VCPU_STATE_IN_GUEST_MODE);
> > > > +	if (xchg(&vs->state, VS_IN_GUEST) == VS_SHOULD_FLUSH)
> > > > +		kvm_x86_ops->tlb_flush(vcpu);
> > > >  	kunmap_atomic(kaddr);
> > > >  }
> > > >  
> > > > @@ -1600,7 +1590,8 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
> > > >  	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
> > > >  	kaddr += vcpu->arch.v_state.vs_offset;
> > > >  	vs = kaddr;
> > > > -	kvm_set_atomic(&vs->state, 1 << KVM_VCPU_STATE_IN_GUEST_MODE, 0);
> > > > +	if (xchg(&vs->state, VS_NOT_IN_GUEST) == VS_SHOULD_FLUSH)
> > > > +		kvm_x86_ops->tlb_flush(vcpu);
> > > >  	kunmap_atomic(kaddr);
> > > >  }
> > > 
> > > Nevermind the early comment (the other comments on that message are
> > > valid).
> I assume the above is related to kvm_set_atomic comment in the [3/8]

Yes.

> > Ah, so the pseucode mentions flush-on-exit because we can be clearing 
> > the flag on xchg. Setting KVM_REQ_TLB_FLUSH instead should be enough
> > (please verify).
> > 
> Yes, that will work while exiting. 
> 
> In the vm_enter case, we need to do a kvm_x86_ops->tlb_flush(vcpu), as
> we have already passed the phase of checking the request.

Yes.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 580abcf..a67e971 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1557,20 +1557,9 @@  static void record_steal_time(struct kvm_vcpu *vcpu)
 		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time));
 }
 
-static void kvm_set_atomic(u64 *addr, u64 old, u64 new)
-{
-	int loop = 1000000;
-	while (1) {
-		if (cmpxchg(addr, old, new) == old)
-			break;
-		loop--;
-		if (!loop) {
-			pr_info("atomic cur: %lx old: %lx new: %lx\n",
-				*addr, old, new);
-			break;
-		}
-	}
-}
+#define VS_NOT_IN_GUEST      (0)
+#define VS_IN_GUEST          (1 << KVM_VCPU_STATE_IN_GUEST_MODE)
+#define VS_SHOULD_FLUSH      (1 << KVM_VCPU_STATE_SHOULD_FLUSH)
 
 static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
 {
@@ -1584,7 +1573,8 @@  static void kvm_set_vcpu_state(struct kvm_vcpu *vcpu)
 	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
 	kaddr += vcpu->arch.v_state.vs_offset;
 	vs = kaddr;
-	kvm_set_atomic(&vs->state, 0, 1 << KVM_VCPU_STATE_IN_GUEST_MODE);
+	if (xchg(&vs->state, VS_IN_GUEST) == VS_SHOULD_FLUSH)
+		kvm_x86_ops->tlb_flush(vcpu);
 	kunmap_atomic(kaddr);
 }
 
@@ -1600,7 +1590,8 @@  static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
 	kaddr = kmap_atomic(vcpu->arch.v_state.vs_page);
 	kaddr += vcpu->arch.v_state.vs_offset;
 	vs = kaddr;
-	kvm_set_atomic(&vs->state, 1 << KVM_VCPU_STATE_IN_GUEST_MODE, 0);
+	if (xchg(&vs->state, VS_NOT_IN_GUEST) == VS_SHOULD_FLUSH)
+		kvm_x86_ops->tlb_flush(vcpu);
 	kunmap_atomic(kaddr);
 }

[v3,6/8] KVM-HV: Add flush_on_enter before guest enter

Commit Message

Comments

Patch