On Thu, Sep 19, 2019 at 10:22:42AM +1000, Paul Mackerras wrote: > On Wed, Sep 11, 2019 at 11:50:35AM -0700, Sean Christopherson wrote: > > Move the implementations of KVM_GET_DIRTY_LOG and KVM_CLEAR_DIRTY_LOG > > for CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT into common KVM code. > > The arch specific implemenations are extremely similar, differing > > only in whether the dirty log needs to be sync'd from hardware (x86) > > and how the TLBs are flushed. Add new arch hooks to handle sync > > and TLB flush; the sync will also be used for non-generic dirty log > > support in a future patch (s390). > > > > The ulterior motive for providing a common implementation is to > > eliminate the dependency between arch and common code with respect to > > the memslot referenced by the dirty log, i.e. to make it obvious in the > > code that the validity of the memslot is guaranteed, as a future patch > > will rework memslot handling such that id_to_memslot() can return NULL. > > I notice you add empty definitions of kvm_arch_sync_dirty_log() for > PPC, both Book E and Book 3S. Given that PPC doesn't select > CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT, why is this necessary? s390 has a non-empty kvm_arch_sync_dirty_log() but doesn't select CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT. Patch 11/13 moves s390's call of kvm_arch_sync_dirty_log() from s390's kvm_vm_ioctl_get_dirty_log() into the common (but not "generic") kvm_get_dirty_log() so that it's obvious that kvm_vm_ioctl_get_dirty_log() and kvm_get_dirty_log() are operating on the same memslot, i.e. aren't independently querying id_to_memslot(). I originally made kvm_arch_sync_dirty_log() opt-in with a __KVM_HAVE_ARCH macro, but the resulting #ifdeffery felt uglier than having PPC and ARM provide empty functions.
