On 25/09/20 23:22, Ben Gardon wrote: > start_level, KVM_MAX_HUGEPAGE_LEVEL, false); > + if (kvm->arch.tdp_mmu_enabled) > + flush = kvm_tdp_mmu_wrprot_slot(kvm, memslot, false) || flush; > spin_unlock(&kvm->mmu_lock); > In fact you can just pass down the end-level KVM_MAX_HUGEPAGE_LEVEL or PGLEVEL_4K here to kvm_tdp_mmu_wrprot_slot and from there to wrprot_gfn_range. > > + /* > + * Take a reference on the root so that it cannot be freed if > + * this thread releases the MMU lock and yields in this loop. > + */ > + get_tdp_mmu_root(kvm, root); > + > + spte_set = wrprot_gfn_range(kvm, root, slot->base_gfn, > + slot->base_gfn + slot->npages, skip_4k) || > + spte_set; > + > + put_tdp_mmu_root(kvm, root); Generalyl using "|=" is the more common idiom in mmu.c. > +static bool clear_dirty_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > + gfn_t start, gfn_t end) > ... > + __handle_changed_spte(kvm, as_id, iter.gfn, iter.old_spte, > + new_spte, iter.level); > + handle_changed_spte_acc_track(iter.old_spte, new_spte, > + iter.level); Is it worth not calling handle_changed_spte? handle_changed_spte_dlog obviously will never fire but duplicating the code is a bit ugly. I guess this patch is the first one that really gives the "feeling" of what the data structures look like. The main difference with the shadow MMU is that you have the tdp_iter instead of the callback-based code of slot_handle_level_range, but otherwise it's not hard to follow one if you know the other. Reorganizing the code so that mmu.c is little more than a wrapper around the two will help as well in this respect. Paolo
