On Tue, Oct 19, 2021 at 9:22 AM David Matlack <dmatlack@xxxxxxxxxx> wrote: > > slot_handle_leaf is a misnomer because it only operates on 4K SPTEs > whereas "leaf" is used to describe any valid terminal SPTE (4K or > large page). Rename slot_handle_leaf to slot_handle_level_4k to > avoid confusion. > > Making this change makes it more obvious there is a benign discrepency > between the legacy MMU and the TDP MMU when it comes to dirty logging. > The legacy MMU only iterates through 4K SPTEs when zapping for > collapsing and when clearing D-bits. The TDP MMU, on the other hand, > iterates through SPTEs on all levels. > > The TDP MMU behavior of zapping SPTEs at all levels is technically > overkill for its current dirty logging implementation, which always > demotes to 4k SPTES, but both the TDP MMU and legacy MMU zap if and only > if the SPTE can be replaced by a larger page, i.e. will not spuriously > zap 2m (or larger) SPTEs. Opportunistically add comments to explain this > discrepency in the code. > > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx> Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx> > --- > v1: https://lore.kernel.org/kvm/20211011204418.162846-1-dmatlack@xxxxxxxxxx/ > - Clarified that the TDP MMU does not perform spurious zaps in commit > message [Sean, Ben] > - Use "legacy MMU" instead of "KVM" in comments to avoid comments > becoming stale in the future if the TDP MMU gets support for 2m dirty > logging [Sean] > > arch/x86/kvm/mmu/mmu.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 24a9f4c3f5e7..fa918289c9e0 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -5382,8 +5382,8 @@ slot_handle_level(struct kvm *kvm, const struct kvm_memory_slot *memslot, > } > > static __always_inline bool > -slot_handle_leaf(struct kvm *kvm, const struct kvm_memory_slot *memslot, > - slot_level_handler fn, bool flush_on_yield) > +slot_handle_level_4k(struct kvm *kvm, const struct kvm_memory_slot *memslot, > + slot_level_handler fn, bool flush_on_yield) > { > return slot_handle_level(kvm, memslot, fn, PG_LEVEL_4K, > PG_LEVEL_4K, flush_on_yield); > @@ -5772,7 +5772,12 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm, > > if (kvm_memslots_have_rmaps(kvm)) { > write_lock(&kvm->mmu_lock); > - flush = slot_handle_leaf(kvm, slot, kvm_mmu_zap_collapsible_spte, true); > + /* > + * Zap only 4k SPTEs since the legacy MMU only supports dirty > + * logging at a 4k granularity and never creates collapsible > + * 2m SPTEs during dirty logging. > + */ > + flush = slot_handle_level_4k(kvm, slot, kvm_mmu_zap_collapsible_spte, true); > if (flush) > kvm_arch_flush_remote_tlbs_memslot(kvm, slot); > write_unlock(&kvm->mmu_lock); > @@ -5809,8 +5814,11 @@ void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm, > > if (kvm_memslots_have_rmaps(kvm)) { > write_lock(&kvm->mmu_lock); > - flush = slot_handle_leaf(kvm, memslot, __rmap_clear_dirty, > - false); > + /* > + * Clear dirty bits only on 4k SPTEs since the legacy MMU only > + * support dirty logging at a 4k granularity. > + */ > + flush = slot_handle_level_4k(kvm, memslot, __rmap_clear_dirty, false); > write_unlock(&kvm->mmu_lock); > } > > -- > 2.33.0.1079.g6e70778dc9-goog >
