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

[Avi Kivity <avi@redhat.com>]

On 12/27/2010 11:16 AM, Avi Kivity wrote:
> 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.
>

Actually, drop_spte() calls rmap_remove(), so all that's broken is 
kvm->stat.lpages accounting.

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