diff mbox

KVM: Discard unnecessary kvm_mmu_flush_tlb() in kvm_mmu_load()

Message ID 1247130042-32290-1-git-send-email-sheng@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sheng Yang July 9, 2009, 9 a.m. UTC
set_cr3() should already cover the TLB flushing.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Sheng Yang July 13, 2009, 11:42 a.m. UTC | #1
On Thursday 09 July 2009 17:00:42 Sheng Yang wrote:
> set_cr3() should already cover the TLB flushing.
>
Comments?
Avi Kivity July 13, 2009, 12:23 p.m. UTC | #2
On 07/09/2009 12:00 PM, Sheng Yang wrote:
> set_cr3() should already cover the TLB flushing.
>
> Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> ---
>   arch/x86/kvm/mmu.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 231d880..501c11e 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2349,8 +2349,8 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
>   	spin_unlock(&vcpu->kvm->mmu_lock);
>   	if (r)
>   		goto out;
> +	/* set_cr3() should ensure TLB has been flushed */
>   	kvm_x86_ops->set_cr3(vcpu, vcpu->arch.mmu.root_hpa);
> -	kvm_mmu_flush_tlb(vcpu);
>   out:
>   	return r;
>   }
>    

Maybe we should drop the flushes in vmx/svm instead?
Sheng Yang July 13, 2009, 1:29 p.m. UTC | #3
On Monday 13 July 2009 20:23:58 Avi Kivity wrote:
> On 07/09/2009 12:00 PM, Sheng Yang wrote:
> > set_cr3() should already cover the TLB flushing.
> >
> > Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> > ---
> >   arch/x86/kvm/mmu.c |    2 +-
> >   1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 231d880..501c11e 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -2349,8 +2349,8 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
> >   	spin_unlock(&vcpu->kvm->mmu_lock);
> >   	if (r)
> >   		goto out;
> > +	/* set_cr3() should ensure TLB has been flushed */
> >   	kvm_x86_ops->set_cr3(vcpu, vcpu->arch.mmu.root_hpa);
> > -	kvm_mmu_flush_tlb(vcpu);
> >   out:
> >   	return r;
> >   }
>
> Maybe we should drop the flushes in vmx/svm instead?

That's what I unsure about... mmu_load() with flushes sounds reasonable, 
set_cr3() with flushes sounds a little more reasonable to me because I think 
set_cr3() may shouldn't depends on caller to flush, which seems more direct.
Marcelo Tosatti July 13, 2009, 1:51 p.m. UTC | #4
On Mon, Jul 13, 2009 at 07:42:56PM +0800, Sheng Yang wrote:
> On Thursday 09 July 2009 17:00:42 Sheng Yang wrote:
> > set_cr3() should already cover the TLB flushing.
> >
> Comments?

Looks good.

> 
> -- 
> regards
> Yang, Sheng
> 
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  arch/x86/kvm/mmu.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 231d880..501c11e 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -2349,8 +2349,8 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
> >  	spin_unlock(&vcpu->kvm->mmu_lock);
> >  	if (r)
> >  		goto out;
> > +	/* set_cr3() should ensure TLB has been flushed */
> >  	kvm_x86_ops->set_cr3(vcpu, vcpu->arch.mmu.root_hpa);
> > -	kvm_mmu_flush_tlb(vcpu);
> >  out:
> >  	return r;
> >  }
> 
--
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 July 15, 2009, 7:04 p.m. UTC | #5
On Thu, Jul 09, 2009 at 05:00:42PM +0800, Sheng Yang wrote:
> set_cr3() should already cover the TLB flushing.
> 
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/kvm/mmu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 231d880..501c11e 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2349,8 +2349,8 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
>  	spin_unlock(&vcpu->kvm->mmu_lock);
>  	if (r)
>  		goto out;
> +	/* set_cr3() should ensure TLB has been flushed */
>  	kvm_x86_ops->set_cr3(vcpu, vcpu->arch.mmu.root_hpa);
> -	kvm_mmu_flush_tlb(vcpu);
>  out:
>  	return r;
>  }
> -- 
> 1.5.4.5

Applied, thanks.

--
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 mbox

Patch

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 231d880..501c11e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2349,8 +2349,8 @@  int kvm_mmu_load(struct kvm_vcpu *vcpu)
 	spin_unlock(&vcpu->kvm->mmu_lock);
 	if (r)
 		goto out;
+	/* set_cr3() should ensure TLB has been flushed */
 	kvm_x86_ops->set_cr3(vcpu, vcpu->arch.mmu.root_hpa);
-	kvm_mmu_flush_tlb(vcpu);
 out:
 	return r;
 }