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

[Avi Kivity <avi@redhat.com>]

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