| Message ID | 20220311002528.2230172-19-dmatlack@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Extend Eager Page Splitting to the shadow MMU | expand |
On Fri, Mar 11, 2022 at 12:25:20AM +0000, David Matlack wrote: > Currently KVM only zaps collapsible 4KiB SPTEs in the shadow MMU (i.e. > in the rmap). This is fine for now KVM never creates intermediate huge > pages during dirty logging, i.e. a 1GiB page is never partially split to > a 2MiB page. > > However, this will stop being true once the shadow MMU participates in > eager page splitting, which can in fact leave behind partially split > huge pages. In preparation for that change, change the shadow MMU to > iterate over all necessary levels when zapping collapsible SPTEs. > > No functional change intended. > > Signed-off-by: David Matlack <dmatlack@google.com> > --- > arch/x86/kvm/mmu/mmu.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 89a7a8d7a632..2032be3edd71 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -6142,18 +6142,30 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm, > return need_tlb_flush; > } > > +static void kvm_rmap_zap_collapsible_sptes(struct kvm *kvm, > + const struct kvm_memory_slot *slot) > +{ > + bool flush; > + > + /* > + * Note, use KVM_MAX_HUGEPAGE_LEVEL - 1 since there's no need to zap > + * pages that are already mapped at the maximum possible level. > + */ > + flush = slot_handle_level(kvm, slot, kvm_mmu_zap_collapsible_spte, > + PG_LEVEL_4K, KVM_MAX_HUGEPAGE_LEVEL - 1, > + true); > + > + if (flush) > + kvm_arch_flush_remote_tlbs_memslot(kvm, slot); > + > +} Reviewed-by: Peter Xu <peterx@redhat.com> IMHO it looks cleaner to write it in the old way (drop the flush var). Maybe even unwrap the helper? Thanks,
On Wed, Mar 16, 2022 at 1:49 AM Peter Xu <peterx@redhat.com> wrote: > > On Fri, Mar 11, 2022 at 12:25:20AM +0000, David Matlack wrote: > > Currently KVM only zaps collapsible 4KiB SPTEs in the shadow MMU (i.e. > > in the rmap). This is fine for now KVM never creates intermediate huge > > pages during dirty logging, i.e. a 1GiB page is never partially split to > > a 2MiB page. > > > > However, this will stop being true once the shadow MMU participates in > > eager page splitting, which can in fact leave behind partially split > > huge pages. In preparation for that change, change the shadow MMU to > > iterate over all necessary levels when zapping collapsible SPTEs. > > > > No functional change intended. > > > > Signed-off-by: David Matlack <dmatlack@google.com> > > --- > > arch/x86/kvm/mmu/mmu.c | 26 +++++++++++++++++++------- > > 1 file changed, 19 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index 89a7a8d7a632..2032be3edd71 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -6142,18 +6142,30 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm, > > return need_tlb_flush; > > } > > > > +static void kvm_rmap_zap_collapsible_sptes(struct kvm *kvm, > > + const struct kvm_memory_slot *slot) > > +{ > > + bool flush; > > + > > + /* > > + * Note, use KVM_MAX_HUGEPAGE_LEVEL - 1 since there's no need to zap > > + * pages that are already mapped at the maximum possible level. > > + */ > > + flush = slot_handle_level(kvm, slot, kvm_mmu_zap_collapsible_spte, > > + PG_LEVEL_4K, KVM_MAX_HUGEPAGE_LEVEL - 1, > > + true); > > + > > + if (flush) > > + kvm_arch_flush_remote_tlbs_memslot(kvm, slot); > > + > > +} > > Reviewed-by: Peter Xu <peterx@redhat.com> > > IMHO it looks cleaner to write it in the old way (drop the flush var). > Maybe even unwrap the helper? Unwrapping the helper and dropping the flush var makes the if condition quite long and hard to read. But I think a compromise would to have kvm_rmap_zap_collapsible_sptes() return flush and leave the flush call in kvm_mmu_zap_collapsible_sptes(). > > Thanks, > > -- > Peter Xu >
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 89a7a8d7a632..2032be3edd71 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6142,18 +6142,30 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm, return need_tlb_flush; } +static void kvm_rmap_zap_collapsible_sptes(struct kvm *kvm, + const struct kvm_memory_slot *slot) +{ + bool flush; + + /* + * Note, use KVM_MAX_HUGEPAGE_LEVEL - 1 since there's no need to zap + * pages that are already mapped at the maximum possible level. + */ + flush = slot_handle_level(kvm, slot, kvm_mmu_zap_collapsible_spte, + PG_LEVEL_4K, KVM_MAX_HUGEPAGE_LEVEL - 1, + true); + + if (flush) + kvm_arch_flush_remote_tlbs_memslot(kvm, slot); + +} + void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm, const struct kvm_memory_slot *slot) { if (kvm_memslots_have_rmaps(kvm)) { write_lock(&kvm->mmu_lock); - /* - * 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. - */ - if (slot_handle_level_4k(kvm, slot, kvm_mmu_zap_collapsible_spte, true)) - kvm_arch_flush_remote_tlbs_memslot(kvm, slot); + kvm_rmap_zap_collapsible_sptes(kvm, slot); write_unlock(&kvm->mmu_lock); }
Currently KVM only zaps collapsible 4KiB SPTEs in the shadow MMU (i.e. in the rmap). This is fine for now KVM never creates intermediate huge pages during dirty logging, i.e. a 1GiB page is never partially split to a 2MiB page. However, this will stop being true once the shadow MMU participates in eager page splitting, which can in fact leave behind partially split huge pages. In preparation for that change, change the shadow MMU to iterate over all necessary levels when zapping collapsible SPTEs. No functional change intended. Signed-off-by: David Matlack <dmatlack@google.com> --- arch/x86/kvm/mmu/mmu.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)