RE: [PATCH v2] KVM: x86: enable dirty log gradually in small chunks

Zhoujian (jay)
2020-02-21T09:54:03Z

> -----Original Message-----
> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Friday, February 21, 2020 3:28 AM
> To: Zhoujian (jay) <jianjay.zhou@huawei.com>
> Cc: kvm@vger.kernel.org; pbonzini@redhat.com; wangxin (U)
> <wangxinxin.wang@huawei.com>; Huangweidong (C)
> <weidong.huang@huawei.com>; sean.j.christopherson@intel.com; Liujinsong
> (Paul) <liu.jinsong@huawei.com>
> Subject: Re: [PATCH v2] KVM: x86: enable dirty log gradually in small chunks
>
> On Thu, Feb 20, 2020 at 12:28:28PM +0800, Jay Zhou wrote:
> > @@ -5865,8 +5865,12 @@ void
> kvm_mmu_slot_remove_write_access(struct kvm *kvm,
> > bool flush;
> >
> > spin_lock(&kvm->mmu_lock);
> > - flush = slot_handle_all_level(kvm, memslot, slot_rmap_write_protect,
> > - false);
> > + if (kvm->manual_dirty_log_protect & KVM_DIRTY_LOG_INITIALLY_SET)
> > + flush = slot_handle_large_level(kvm, memslot,
> > + slot_rmap_write_protect, false);
> > + else
> > + flush = slot_handle_all_level(kvm, memslot,
> > + slot_rmap_write_protect, false);
>
> Another extra comment:
>
> I think we should still keep the old behavior for KVM_MEM_READONLY (in
> kvm_mmu_slot_apply_flags())) for this...

I also realized this issue after posting this patch, and I agree.

> Say, instead of doing this, maybe we
> want kvm_mmu_slot_remove_write_access() to take a new parameter to
> decide to which level we do the wr-protect.

How about using the "flags" field to distinguish:

if ((kvm->manual_dirty_log_protect & KVM_DIRTY_LOG_INITIALLY_SET)
&& (memslot->flags & KVM_MEM_LOG_DIRTY_PAGES))
flush = slot_handle_large_level(kvm, memslot,
slot_rmap_write_protect, false);
else
flush = slot_handle_all_level(kvm, memslot,
slot_rmap_write_protect, false);

Regards,
Jay Zhou

>
> Thanks,
>
> > spin_unlock(&kvm->mmu_lock);
> >
> > /*
>
> --
> Peter Xu