| Message ID | 20230311002258.852397-12-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups | expand |
On Fri, Mar 10, 2023 at 04:22:42PM -0800, Sean Christopherson wrote: ... > -static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm, > - struct kvm_memory_slot *slot, > - struct kvm_page_track_notifier_node *node) > -{ > - kvm_mmu_zap_all_fast(kvm); > -} > - > int kvm_mmu_init_vm(struct kvm *kvm) > { > struct kvm_page_track_notifier_node *node = &kvm->arch.mmu_sp_tracker; > @@ -6110,7 +6103,6 @@ int kvm_mmu_init_vm(struct kvm *kvm) > } > > node->track_write = kvm_mmu_pte_write; > - node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot; > kvm_page_track_register_notifier(kvm, node); > > kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index f706621c35b8..29dd6c97d145 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -12662,6 +12662,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm) > void kvm_arch_flush_shadow_memslot(struct kvm *kvm, > struct kvm_memory_slot *slot) > { > + kvm_mmu_zap_all_fast(kvm); Could we still call kvm_mmu_invalidate_zap_pages_in_memslot() here? As I know, for TDX, its version of kvm_mmu_invalidate_zap_pages_in_memslot() is like static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm, struct kvm_memory_slot *slot, struct kvm_page_track_notifier_node *node) { if (kvm_gfn_shared_mask(kvm)) kvm_mmu_zap_memslot(kvm, slot); else kvm_mmu_zap_all_fast(kvm); } Maybe this kind of judegment is better to be confined in mmu.c? Thanks Yan > + > kvm_page_track_flush_slot(kvm, slot); > } >
On Wed, Mar 15, 2023, Yan Zhao wrote: > On Fri, Mar 10, 2023 at 04:22:42PM -0800, Sean Christopherson wrote: > ... > > -static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm, > > - struct kvm_memory_slot *slot, > > - struct kvm_page_track_notifier_node *node) > > -{ > > - kvm_mmu_zap_all_fast(kvm); > > -} > > - > > int kvm_mmu_init_vm(struct kvm *kvm) > > { > > struct kvm_page_track_notifier_node *node = &kvm->arch.mmu_sp_tracker; > > @@ -6110,7 +6103,6 @@ int kvm_mmu_init_vm(struct kvm *kvm) > > } > > > > node->track_write = kvm_mmu_pte_write; > > - node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot; > > kvm_page_track_register_notifier(kvm, node); > > > > kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache; > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index f706621c35b8..29dd6c97d145 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -12662,6 +12662,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm) > > void kvm_arch_flush_shadow_memslot(struct kvm *kvm, > > struct kvm_memory_slot *slot) > > { > > + kvm_mmu_zap_all_fast(kvm); > Could we still call kvm_mmu_invalidate_zap_pages_in_memslot() here? > As I know, for TDX, its version of > kvm_mmu_invalidate_zap_pages_in_memslot() is like > > static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm, > struct kvm_memory_slot *slot, > struct kvm_page_track_notifier_node *node) > { > if (kvm_gfn_shared_mask(kvm)) > kvm_mmu_zap_memslot(kvm, slot); > else > kvm_mmu_zap_all_fast(kvm); > } > > Maybe this kind of judegment is better to be confined in mmu.c? Hmm, yeah, I agree. The only reason I exposed kvm_mmu_zap_all_fast() is because kvm_mmu_zap_all() is already exposed for kvm_arch_flush_shadow_all() and it felt weird/wrong to split those. But that's the only usage of kvm_mmu_zap_all(), so a better approach to maintain consistency would be to move kvm_arch_flush_shadow_{all,memslot}() into mmu.c. I'll do that in the next version.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 808c292ad3f4..17281d6825c9 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1844,6 +1844,7 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm, void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm, const struct kvm_memory_slot *memslot); void kvm_mmu_zap_all(struct kvm *kvm); +void kvm_mmu_zap_all_fast(struct kvm *kvm); void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen); void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned long kvm_nr_mmu_pages); diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 4685c80e441b..409dabec69df 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6030,7 +6030,7 @@ static void kvm_zap_obsolete_pages(struct kvm *kvm) * not use any resource of the being-deleted slot or all slots * after calling the function. */ -static void kvm_mmu_zap_all_fast(struct kvm *kvm) +void kvm_mmu_zap_all_fast(struct kvm *kvm) { lockdep_assert_held(&kvm->slots_lock); @@ -6086,13 +6086,6 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm) return unlikely(!list_empty_careful(&kvm->arch.zapped_obsolete_pages)); } -static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm, - struct kvm_memory_slot *slot, - struct kvm_page_track_notifier_node *node) -{ - kvm_mmu_zap_all_fast(kvm); -} - int kvm_mmu_init_vm(struct kvm *kvm) { struct kvm_page_track_notifier_node *node = &kvm->arch.mmu_sp_tracker; @@ -6110,7 +6103,6 @@ int kvm_mmu_init_vm(struct kvm *kvm) } node->track_write = kvm_mmu_pte_write; - node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot; kvm_page_track_register_notifier(kvm, node); kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index f706621c35b8..29dd6c97d145 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12662,6 +12662,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm) void kvm_arch_flush_shadow_memslot(struct kvm *kvm, struct kvm_memory_slot *slot) { + kvm_mmu_zap_all_fast(kvm); + kvm_page_track_flush_slot(kvm, slot); }
Call kvm_mmu_zap_all_fast() directly when flushing a memslot instead of bouncing through the page-track mechanism. KVM (unfortunately) needs to zap and flush all page tables on memslot DELETE/MOVE irrespective of whether KVM is shadowing guest page tables. This will allow changing KVM to register a page-track notifier on the first shadow root allocation, and will also allow deleting the misguided kvm_page_track_flush_slot() hook itself once KVM-GT also moves to a different method for reacting to memslot changes. No functional change intended. Cc: Yan Zhao <yan.y.zhao@intel.com> Link: https://lore.kernel.org/r/20221110014821.1548347-2-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu/mmu.c | 10 +--------- arch/x86/kvm/x86.c | 2 ++ 3 files changed, 4 insertions(+), 9 deletions(-)