From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 5/6] KVM: x86: Add new dirty logging kvm_x86_ops for PML Date: Tue, 3 Feb 2015 16:53:02 +0100 Message-ID: <20150203155302.GE19731@potion.redhat.com> References: <1422413668-3509-1-git-send-email-kai.huang@linux.intel.com> <1422413668-3509-6-git-send-email-kai.huang@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: pbonzini@redhat.com, gleb@kernel.org, linux@arm.linux.org.uk, kvm@vger.kernel.org To: Kai Huang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42520 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965920AbbBCPxP (ORCPT ); Tue, 3 Feb 2015 10:53:15 -0500 Content-Disposition: inline In-Reply-To: <1422413668-3509-6-git-send-email-kai.huang@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 2015-01-28 10:54+0800, Kai Huang: > This patch adds new kvm_x86_ops dirty logging hooks to enable/disable dirty > logging for particular memory slot, and to flush potentially logged dirty GPAs > before reporting slot->dirty_bitmap to userspace. > > kvm x86 common code calls these hooks when they are available so PML logic can > be hidden to VMX specific. Other ARCHs won't be impacted as these hooks are NULL > for them. > > Signed-off-by: Kai Huang > --- > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -802,6 +802,31 @@ struct kvm_x86_ops { > + > + /* > + * Arch-specific dirty logging hooks. These hooks are only supposed to > + * be valid if the specific arch has hardware-accelerated dirty logging > + * mechanism. Currently only for PML on VMX. > + * > + * - slot_enable_log_dirty: > + * called when enabling log dirty mode for the slot. (I guess that "log dirty mode" isn't the meaning that people will think after seeing 'log_dirty' ... I'd at least change 'log_dirty' to 'dirty_log' in these names.) > + * - slot_disable_log_dirty: > + * called when disabling log dirty mode for the slot. > + * also called when slot is created with log dirty disabled. > + * - flush_log_dirty: > + * called before reporting dirty_bitmap to userspace. > + * - enable_log_dirty_pt_masked: > + * called when reenabling log dirty for the GFNs in the mask after > + * corresponding bits are cleared in slot->dirty_bitmap. This name is very confusing ... I think we should hint that this is called after we learn that the page has been written to and would like to monitor it again. Using something like collected/refresh? (I'd have to do horrible things to come up with a good name, sorry.) > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3780,6 +3780,12 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > > mutex_lock(&kvm->slots_lock); > > + /* > + * Flush potentially hardware-cached dirty pages to dirty_bitmap. > + */ > + if (kvm_x86_ops->flush_log_dirty) > + kvm_x86_ops->flush_log_dirty(kvm); (Flushing would make more sense in kvm_get_dirty_log_protect().) > + > r = kvm_get_dirty_log_protect(kvm, log, &is_dirty); > > /* > @@ -7533,6 +7539,56 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, > return 0; > } > > +static void kvm_mmu_slot_apply_flags(struct kvm *kvm, > + struct kvm_memory_slot *new) > +{ > + /* Still write protect RO slot */ > + if (new->flags & KVM_MEM_READONLY) { > + kvm_mmu_slot_remove_write_access(kvm, new); We didn't write protect RO slots before, does this patch depend on it? > @@ -7562,16 +7618,15 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, > - if ((change != KVM_MR_DELETE) && (new->flags & KVM_MEM_LOG_DIRTY_PAGES)) > - kvm_mmu_slot_remove_write_access(kvm, new); > + if (change != KVM_MR_DELETE) > + kvm_mmu_slot_apply_flags(kvm, new);