[PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[Avi Kivity]

Instead, drop large mappings, which were the reason we dropped shadow.

Signed-off-by: Avi Kivity <avi@redhat.com>
---

v2: maintain largepage stats

 arch/x86/kvm/mmu.c  |   12 ++++++++----
 virt/kvm/kvm_main.c |    7 +------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 43bd5e3..dc36088 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3444,14 +3444,18 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
 		if (!test_bit(slot, sp->slot_bitmap))
 			continue;
 
-		if (sp->role.level != PT_PAGE_TABLE_LEVEL)
-			continue;
-
 		pt = sp->spt;
-		for (i = 0; i < PT64_ENT_PER_PAGE; ++i)
+		for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
+			if (sp->role.level != PT_PAGE_TABLE_LEVEL
+			    && is_large_pte(pt[i])) {
+				drop_spte(kvm, &pt[i],
+					  shadow_trap_nonpresent_pte);
+				--kvm->stat.lpages;
+			}
 			/* avoid RMW */
 			if (is_writable_pte(pt[i]))
 				update_spte(&pt[i], pt[i] & ~PT_WRITABLE_MASK);
+		}
 	}
 	kvm_flush_remote_tlbs(kvm);
 }
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b1b6cbb..b3bfeb8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -586,7 +586,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 			    struct kvm_userspace_memory_region *mem,
 			    int user_alloc)
 {
-	int r, flush_shadow = 0;
+	int r;
 	gfn_t base_gfn;
 	unsigned long npages;
 	unsigned long i;
@@ -706,8 +706,6 @@ skip_lpage:
 		if (kvm_create_dirty_bitmap(&new) < 0)
 			goto out_free;
 		/* destroy any largepage mappings for dirty tracking */
-		if (old.npages)
-			flush_shadow = 1;
 	}
 #else  /* not defined CONFIG_S390 */
 	new.user_alloc = user_alloc;
@@ -778,9 +776,6 @@ skip_lpage:
 	kvm_free_physmem_slot(&old, &new);
 	kfree(old_memslots);
 
-	if (flush_shadow)
-		kvm_arch_flush_shadow(kvm);
-
 	return 0;
 
 out_free:
-- 
1.7.1

Re: [PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[Takuya Yoshikawa]

On Mon, 27 Dec 2010 12:08:45 +0200
Avi Kivity <avi@redhat.com> wrote:

> Instead, drop large mappings, which were the reason we dropped shadow.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> 
> v2: maintain largepage stats
> 
>  arch/x86/kvm/mmu.c  |   12 ++++++++----
>  virt/kvm/kvm_main.c |    7 +------
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 43bd5e3..dc36088 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3444,14 +3444,18 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
>  		if (!test_bit(slot, sp->slot_bitmap))
>  			continue;
>  
> -		if (sp->role.level != PT_PAGE_TABLE_LEVEL)
> -			continue;
> -
>  		pt = sp->spt;
> -		for (i = 0; i < PT64_ENT_PER_PAGE; ++i)
> +		for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
> +			if (sp->role.level != PT_PAGE_TABLE_LEVEL
> +			    && is_large_pte(pt[i])) {
> +				drop_spte(kvm, &pt[i],
> +					  shadow_trap_nonpresent_pte);
> +				--kvm->stat.lpages;
> +			}
>  			/* avoid RMW */
>  			if (is_writable_pte(pt[i]))
>  				update_spte(&pt[i], pt[i] & ~PT_WRITABLE_MASK);

I'm a bit confused here.
rmap_write_protect() does drop_spte() but update_spte() in
/* check for huge page mappings */ loop.

I may be able to learn something from your share code later on.

Thanks,
  Takuya




> +		}
>  	}
>  	kvm_flush_remote_tlbs(kvm);
>  }
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index b1b6cbb..b3bfeb8 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -586,7 +586,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
>  			    struct kvm_userspace_memory_region *mem,
>  			    int user_alloc)
>  {
> -	int r, flush_shadow = 0;
> +	int r;
>  	gfn_t base_gfn;
>  	unsigned long npages;
>  	unsigned long i;
> @@ -706,8 +706,6 @@ skip_lpage:
>  		if (kvm_create_dirty_bitmap(&new) < 0)
>  			goto out_free;
>  		/* destroy any largepage mappings for dirty tracking */
> -		if (old.npages)
> -			flush_shadow = 1;
>  	}
>  #else  /* not defined CONFIG_S390 */
>  	new.user_alloc = user_alloc;
> @@ -778,9 +776,6 @@ skip_lpage:
>  	kvm_free_physmem_slot(&old, &new);
>  	kfree(old_memslots);
>  
> -	if (flush_shadow)
> -		kvm_arch_flush_shadow(kvm);
> -
>  	return 0;
>  
>  out_free:
> -- 
> 1.7.1
> 
> --
> 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


-- 
Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

Re: [PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[Avi Kivity]

On 12/27/2010 12:35 PM, Takuya Yoshikawa wrote:
> On Mon, 27 Dec 2010 12:08:45 +0200
> Avi Kivity<avi@redhat.com>  wrote:
>
> >  Instead, drop large mappings, which were the reason we dropped shadow.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
> >  ---
> >
> >  v2: maintain largepage stats
> >
> >   arch/x86/kvm/mmu.c  |   12 ++++++++----
> >   virt/kvm/kvm_main.c |    7 +------
> >   2 files changed, 9 insertions(+), 10 deletions(-)
> >
> >  diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> >  index 43bd5e3..dc36088 100644
> >  --- a/arch/x86/kvm/mmu.c
> >  +++ b/arch/x86/kvm/mmu.c
> >  @@ -3444,14 +3444,18 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
> >   		if (!test_bit(slot, sp->slot_bitmap))
> >   			continue;
> >
> >  -		if (sp->role.level != PT_PAGE_TABLE_LEVEL)
> >  -			continue;
> >  -
> >   		pt = sp->spt;
> >  -		for (i = 0; i<  PT64_ENT_PER_PAGE; ++i)
> >  +		for (i = 0; i<  PT64_ENT_PER_PAGE; ++i) {
> >  +			if (sp->role.level != PT_PAGE_TABLE_LEVEL
> >  +			&&  is_large_pte(pt[i])) {
> >  +				drop_spte(kvm,&pt[i],
> >  +					  shadow_trap_nonpresent_pte);
> >  +				--kvm->stat.lpages;
> >  +			}
> >   			/* avoid RMW */
> >   			if (is_writable_pte(pt[i]))
> >   				update_spte(&pt[i], pt[i]&  ~PT_WRITABLE_MASK);
>
> I'm a bit confused here.
> rmap_write_protect() does drop_spte() but update_spte() in
> /* check for huge page mappings */ loop.
>

rmap_write_protect() protects a 4k page, so it uses update_spte() to 
drop the W bit, and drops large mappings that point to the same 4k 
page.  We can't drop the W bit from the large page, because that will 
write protect 511 unrelated pages.

> I may be able to learn something from your share code later on.
>

Unlikely - I was thinking of sharing the

>  +				drop_spte(kvm,&pt[i],
>  +					  shadow_trap_nonpresent_pte);
>  +				--kvm->stat.lpages;


bits.  Now I'm thinking of dropping kvm->stat, I think we have all the 
information it provides covered by tracepoints.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[Takuya Yoshikawa]

On Mon, 27 Dec 2010 12:43:59 +0200
Avi Kivity <avi@redhat.com> wrote:
> rmap_write_protect() protects a 4k page, so it uses update_spte() to 
> drop the W bit, and drops large mappings that point to the same 4k 
> page.  We can't drop the W bit from the large page, because that will 
> write protect 511 unrelated pages.

I see, I'll reread these from that sense.

Thank you for the explanation.

  Takuya

> 
> > I may be able to learn something from your share code later on.
> >
> 
> Unlikely - I was thinking of sharing the
> 
> >  +				drop_spte(kvm,&pt[i],
> >  +					  shadow_trap_nonpresent_pte);
> >  +				--kvm->stat.lpages;
> 
> 
> bits.  Now I'm thinking of dropping kvm->stat, I think we have all the 
> information it provides covered by tracepoints.
> 
> -- 

Re: [PATCH v2] KVM: MMU: Don't flush shadow when enabling dirty tracking

[Marcelo Tosatti]

On Mon, Dec 27, 2010 at 12:08:45PM +0200, Avi Kivity wrote:
> Instead, drop large mappings, which were the reason we dropped shadow.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> 
> v2: maintain largepage stats
> 
>  arch/x86/kvm/mmu.c  |   12 ++++++++----
>  virt/kvm/kvm_main.c |    7 +------
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 

Applied, thanks.