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

[Avi Kivity <avi@redhat.com>]

On 12/27/2010 06:06 AM, Takuya Yoshikawa wrote:
> 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>
> >  ---
> >   arch/x86/kvm/mmu.c  |   10 ++++++----
> >   virt/kvm/kvm_main.c |    7 +------
> >   2 files changed, 7 insertions(+), 10 deletions(-)
> >
> >  diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> >  index 43bd5e3..1bbe2c0 100644
> >  --- a/arch/x86/kvm/mmu.c
> >  +++ b/arch/x86/kvm/mmu.c
> >  @@ -3444,14 +3444,16 @@ 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);
> >   			/* avoid RMW */
> >   			if (is_writable_pte(pt[i]))
> >   				update_spte(&pt[i], pt[i]&  ~PT_WRITABLE_MASK);
> >  +		}
> >   	}
> >   	kvm_flush_remote_tlbs(kvm);
> >   }
>
> What is the difference from the similar part in rmap_write_protect()?
>
> === from rmap_write_protect() ===
>
> 	/* check for huge page mappings */
> 	for (i = PT_DIRECTORY_LEVEL;
> 	     i<  PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
> 		rmapp = gfn_to_rmap(kvm, gfn, i);
> 		spte = rmap_next(kvm, rmapp, NULL);
> 		while (spte) {
> 			BUG_ON(!spte);
> 			BUG_ON(!(*spte&  PT_PRESENT_MASK));
> 			BUG_ON((*spte&  (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK)) != (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK));
> 			pgprintk("rmap_write_protect(large): spte %p %llx %lld\n", spte, *spte, gfn);
> 			if (is_writable_pte(*spte)) {
> 				drop_spte(kvm, spte,
> 					  shadow_trap_nonpresent_pte);
> 				--kvm->stat.lpages;
> 				spte = NULL;
> 				write_protected = 1;
> 			}
> 			spte = rmap_next(kvm, rmapp, spte);
> 		}
> 	}
> ===

Shouldn't be any.  I forgot to rmap_remove() and to update kvm_stat, 
thanks for pointing it out.

I'll post an updated patch.  We'll want to share code between the two 
later on.

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