> -----Original Message----- > From: Peter Xu [mailto:peterx@xxxxxxxxxx] > Sent: Friday, February 21, 2020 3:28 AM > To: Zhoujian (jay) <jianjay.zhou@xxxxxxxxxx> > Cc: kvm@xxxxxxxxxxxxxxx; pbonzini@xxxxxxxxxx; wangxin (U) > <wangxinxin.wang@xxxxxxxxxx>; Huangweidong (C) > <weidong.huang@xxxxxxxxxx>; sean.j.christopherson@xxxxxxxxx; Liujinsong > (Paul) <liu.jinsong@xxxxxxxxxx> > 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