On Wed, Sep 26, 2018 at 02:30:02PM +1000, David Gibson wrote: > On Wed, Sep 26, 2018 at 02:08:37PM +1000, David Gibson wrote: > > > static void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte, > > > - unsigned long gpa, unsigned int shift) > > > + unsigned long gpa, unsigned int shift, > > > + struct kvm_memory_slot *memslot) > > > > > > { > > > - unsigned long page_size = 1ul << shift; > > > unsigned long old; > > > > > > old = kvmppc_radix_update_pte(kvm, pte, ~0UL, 0, gpa, shift); > > > kvmppc_radix_tlbie_page(kvm, gpa, shift); > > > if (old & _PAGE_DIRTY) { > > > unsigned long gfn = gpa >> PAGE_SHIFT; > > > - struct kvm_memory_slot *memslot; > > > + unsigned long page_size = PAGE_SIZE; > > > > > > - memslot = gfn_to_memslot(kvm, gfn); > > > - if (memslot && memslot->dirty_bitmap) > > > + if (shift) > > > + page_size = 1ul << shift; > > > + if (!memslot) > > > + memslot = gfn_to_memslot(kvm, gfn); > > ..it might be nicer to avoid the explicit test on memslot by maping a > __kvmppc_unmap_pte() which must have memslot passed in, and a wrapper > which computes it from the gfn. Then we would be looking up the memslot even when we aren't going to use it. In a subsequent patch this function gets used on shadow page tables and in that case we never need the memslot. Paul.