| Message ID | 20200925212302.3979661-8-bgardon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce the TDP MMU | expand |
On 25/09/20 23:22, Ben Gardon wrote: > +/* > + * If the MMU lock is contended or this thread needs to yield, flushes > + * the TLBs, releases, the MMU lock, yields, reacquires the MMU lock, > + * restarts the tdp_iter's walk from the root, and returns true. > + * If no yield is needed, returns false. > + */ The comment is not really necessary. :) Paolo > +static bool tdp_mmu_iter_cond_resched(struct kvm *kvm, struct tdp_iter *iter) > +{ > + if (need_resched() || spin_needbreak(&kvm->mmu_lock)) { > + kvm_flush_remote_tlbs(kvm); > + cond_resched_lock(&kvm->mmu_lock); > + tdp_iter_refresh_walk(iter); > + return true; > + } else { > + return false; > + }
On Fri, Sep 25, 2020 at 02:22:47PM -0700, Ben Gardon wrote: > Add functions to zap SPTEs to the TDP MMU. These are needed to tear down > TDP MMU roots properly and implement other MMU functions which require > tearing down mappings. Future patches will add functions to populate the > page tables, but as for this patch there will not be any work for these > functions to do. > > Tested by running kvm-unit-tests and KVM selftests on an Intel Haswell > machine. This series introduced no new failures. > > This series can be viewed in Gerrit at: > https://linux-review.googlesource.com/c/virt/kvm/kvm/+/2538 > > Signed-off-by: Ben Gardon <bgardon@google.com> > --- > arch/x86/kvm/mmu/mmu.c | 15 +++++ > arch/x86/kvm/mmu/tdp_iter.c | 17 ++++++ > arch/x86/kvm/mmu/tdp_iter.h | 1 + > arch/x86/kvm/mmu/tdp_mmu.c | 106 ++++++++++++++++++++++++++++++++++++ > arch/x86/kvm/mmu/tdp_mmu.h | 2 + > 5 files changed, 141 insertions(+) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index f09081f9137b0..7a17cca19b0c1 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -5852,6 +5852,10 @@ static void kvm_mmu_zap_all_fast(struct kvm *kvm) > kvm_reload_remote_mmus(kvm); > > kvm_zap_obsolete_pages(kvm); > + > + if (kvm->arch.tdp_mmu_enabled) > + kvm_tdp_mmu_zap_all(kvm); Haven't looked into how this works; is kvm_tdp_mmu_zap_all() additive to what is done by the legacy zapping, or is it a replacement? > + > spin_unlock(&kvm->mmu_lock); > } > @@ -57,8 +58,13 @@ bool is_tdp_mmu_root(struct kvm *kvm, hpa_t hpa) > return root->tdp_mmu_page; > } > > +static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > + gfn_t start, gfn_t end); > + > static void free_tdp_mmu_root(struct kvm *kvm, struct kvm_mmu_page *root) > { > + gfn_t max_gfn = 1ULL << (boot_cpu_data.x86_phys_bits - PAGE_SHIFT); BIT_ULL(...) > + > lockdep_assert_held(&kvm->mmu_lock); > > WARN_ON(root->root_count); > @@ -66,6 +72,8 @@ static void free_tdp_mmu_root(struct kvm *kvm, struct kvm_mmu_page *root) > > list_del(&root->link); > > + zap_gfn_range(kvm, root, 0, max_gfn); > + > free_page((unsigned long)root->spt); > kmem_cache_free(mmu_page_header_cache, root); > } > @@ -193,6 +201,11 @@ hpa_t kvm_tdp_mmu_get_vcpu_root_hpa(struct kvm_vcpu *vcpu) > static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn, > u64 old_spte, u64 new_spte, int level); > > +static int kvm_mmu_page_as_id(struct kvm_mmu_page *sp) > +{ > + return sp->role.smm ? 1 : 0; > +} > + > /** > * handle_changed_spte - handle bookkeeping associated with an SPTE change > * @kvm: kvm instance > @@ -294,3 +307,96 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn, > free_page((unsigned long)pt); > } > } > + > +#define for_each_tdp_pte_root(_iter, _root, _start, _end) \ > + for_each_tdp_pte(_iter, _root->spt, _root->role.level, _start, _end) > + > +/* > + * If the MMU lock is contended or this thread needs to yield, flushes > + * the TLBs, releases, the MMU lock, yields, reacquires the MMU lock, > + * restarts the tdp_iter's walk from the root, and returns true. > + * If no yield is needed, returns false. > + */ > +static bool tdp_mmu_iter_cond_resched(struct kvm *kvm, struct tdp_iter *iter) > +{ > + if (need_resched() || spin_needbreak(&kvm->mmu_lock)) { > + kvm_flush_remote_tlbs(kvm); > + cond_resched_lock(&kvm->mmu_lock); > + tdp_iter_refresh_walk(iter); > + return true; > + } else { > + return false; > + } Kernel style is to not bother with an "else" if the "if" returns. > +} > + > +/* > + * Tears down the mappings for the range of gfns, [start, end), and frees the > + * non-root pages mapping GFNs strictly within that range. Returns true if > + * SPTEs have been cleared and a TLB flush is needed before releasing the > + * MMU lock. > + */ > +static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > + gfn_t start, gfn_t end) > +{ > + struct tdp_iter iter; > + bool flush_needed = false; > + int as_id = kvm_mmu_page_as_id(root); > + > + for_each_tdp_pte_root(iter, root, start, end) { > + if (!is_shadow_present_pte(iter.old_spte)) > + continue; > + > + /* > + * If this is a non-last-level SPTE that covers a larger range > + * than should be zapped, continue, and zap the mappings at a > + * lower level. > + */ > + if ((iter.gfn < start || > + iter.gfn + KVM_PAGES_PER_HPAGE(iter.level) > end) && > + !is_last_spte(iter.old_spte, iter.level)) > + continue; > + > + *iter.sptep = 0; > + handle_changed_spte(kvm, as_id, iter.gfn, iter.old_spte, 0, > + iter.level); > + > + flush_needed = !tdp_mmu_iter_cond_resched(kvm, &iter); > + } > + return flush_needed; > +} > + > +/* > + * Tears down the mappings for the range of gfns, [start, end), and frees the > + * non-root pages mapping GFNs strictly within that range. Returns true if > + * SPTEs have been cleared and a TLB flush is needed before releasing the > + * MMU lock. > + */ > +bool kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, gfn_t start, gfn_t end) > +{ > + struct kvm_mmu_page *root; > + bool flush = false; > + > + for_each_tdp_mmu_root(kvm, root) { > + /* > + * 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); > + > + flush = zap_gfn_range(kvm, root, start, end) || flush; > + > + put_tdp_mmu_root(kvm, root); > + } > + > + return flush; > +} > + > +void kvm_tdp_mmu_zap_all(struct kvm *kvm) > +{ > + gfn_t max_gfn = 1ULL << (boot_cpu_data.x86_phys_bits - PAGE_SHIFT); BIT_ULL > + bool flush; > + > + flush = kvm_tdp_mmu_zap_gfn_range(kvm, 0, max_gfn); > + if (flush) > + kvm_flush_remote_tlbs(kvm); > +} > diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h > index 9274debffeaa1..cb86f9fe69017 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.h > +++ b/arch/x86/kvm/mmu/tdp_mmu.h > @@ -12,4 +12,6 @@ bool is_tdp_mmu_root(struct kvm *kvm, hpa_t root); > hpa_t kvm_tdp_mmu_get_vcpu_root_hpa(struct kvm_vcpu *vcpu); > void kvm_tdp_mmu_put_root_hpa(struct kvm *kvm, hpa_t root_hpa); > > +bool kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, gfn_t start, gfn_t end); > +void kvm_tdp_mmu_zap_all(struct kvm *kvm); > #endif /* __KVM_X86_MMU_TDP_MMU_H */ > -- > 2.28.0.709.gb0816b6eb0-goog >
On 30/09/20 08:15, Sean Christopherson wrote: >> kvm_zap_obsolete_pages(kvm); >> + >> + if (kvm->arch.tdp_mmu_enabled) >> + kvm_tdp_mmu_zap_all(kvm); > > Haven't looked into how this works; is kvm_tdp_mmu_zap_all() additive to > what is done by the legacy zapping, or is it a replacement? It's additive because the shadow MMU is still used for nesting. >> + >> spin_unlock(&kvm->mmu_lock); >> } >> @@ -57,8 +58,13 @@ bool is_tdp_mmu_root(struct kvm *kvm, hpa_t hpa) >> return root->tdp_mmu_page; >> } >> >> +static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, >> + gfn_t start, gfn_t end); >> + >> static void free_tdp_mmu_root(struct kvm *kvm, struct kvm_mmu_page *root) >> { >> + gfn_t max_gfn = 1ULL << (boot_cpu_data.x86_phys_bits - PAGE_SHIFT); > > BIT_ULL(...) Not sure about that. Here the point is not to have a single bit, but to do a power of two. Same for the version below. >> + * If the MMU lock is contended or this thread needs to yield, flushes >> + * the TLBs, releases, the MMU lock, yields, reacquires the MMU lock, >> + * restarts the tdp_iter's walk from the root, and returns true. >> + * If no yield is needed, returns false. >> + */ >> +static bool tdp_mmu_iter_cond_resched(struct kvm *kvm, struct tdp_iter *iter) >> +{ >> + if (need_resched() || spin_needbreak(&kvm->mmu_lock)) { >> + kvm_flush_remote_tlbs(kvm); >> + cond_resched_lock(&kvm->mmu_lock); >> + tdp_iter_refresh_walk(iter); >> + return true; >> + } else { >> + return false; >> + } > > Kernel style is to not bother with an "else" if the "if" returns. I have rewritten all of this in my version anyway. :) Paolo
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index f09081f9137b0..7a17cca19b0c1 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -5852,6 +5852,10 @@ static void kvm_mmu_zap_all_fast(struct kvm *kvm) kvm_reload_remote_mmus(kvm); kvm_zap_obsolete_pages(kvm); + + if (kvm->arch.tdp_mmu_enabled) + kvm_tdp_mmu_zap_all(kvm); + spin_unlock(&kvm->mmu_lock); } @@ -5892,6 +5896,7 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end) struct kvm_memslots *slots; struct kvm_memory_slot *memslot; int i; + bool flush; spin_lock(&kvm->mmu_lock); for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) { @@ -5911,6 +5916,12 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end) } } + if (kvm->arch.tdp_mmu_enabled) { + flush = kvm_tdp_mmu_zap_gfn_range(kvm, gfn_start, gfn_end); + if (flush) + kvm_flush_remote_tlbs(kvm); + } + spin_unlock(&kvm->mmu_lock); } @@ -6077,6 +6088,10 @@ void kvm_mmu_zap_all(struct kvm *kvm) } kvm_mmu_commit_zap_page(kvm, &invalid_list); + + if (kvm->arch.tdp_mmu_enabled) + kvm_tdp_mmu_zap_all(kvm); + spin_unlock(&kvm->mmu_lock); } diff --git a/arch/x86/kvm/mmu/tdp_iter.c b/arch/x86/kvm/mmu/tdp_iter.c index ee90d62d2a9b1..6c1a38429c81a 100644 --- a/arch/x86/kvm/mmu/tdp_iter.c +++ b/arch/x86/kvm/mmu/tdp_iter.c @@ -161,3 +161,20 @@ void tdp_iter_next(struct tdp_iter *iter) done = try_step_side(iter); } } + +/* + * Restart the walk over the paging structure from the root, starting from the + * highest gfn the iterator had previously reached. Assumes that the entire + * paging structure, except the root page, may have been completely torn down + * and rebuilt. + */ +void tdp_iter_refresh_walk(struct tdp_iter *iter) +{ + gfn_t goal_gfn = iter->goal_gfn; + + if (iter->gfn > goal_gfn) + goal_gfn = iter->gfn; + + tdp_iter_start(iter, iter->pt_path[iter->root_level - 1], + iter->root_level, goal_gfn); +} diff --git a/arch/x86/kvm/mmu/tdp_iter.h b/arch/x86/kvm/mmu/tdp_iter.h index b102109778eac..34da3bdada436 100644 --- a/arch/x86/kvm/mmu/tdp_iter.h +++ b/arch/x86/kvm/mmu/tdp_iter.h @@ -49,5 +49,6 @@ u64 *spte_to_child_pt(u64 pte, int level); void tdp_iter_start(struct tdp_iter *iter, u64 *root_pt, int root_level, gfn_t goal_gfn); void tdp_iter_next(struct tdp_iter *iter); +void tdp_iter_refresh_walk(struct tdp_iter *iter); #endif /* __KVM_X86_MMU_TDP_ITER_H */ diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index 653507773b42c..d96fc182c8497 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -2,6 +2,7 @@ #include "mmu.h" #include "mmu_internal.h" +#include "tdp_iter.h" #include "tdp_mmu.h" static bool __read_mostly tdp_mmu_enabled = true; @@ -57,8 +58,13 @@ bool is_tdp_mmu_root(struct kvm *kvm, hpa_t hpa) return root->tdp_mmu_page; } +static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, + gfn_t start, gfn_t end); + static void free_tdp_mmu_root(struct kvm *kvm, struct kvm_mmu_page *root) { + gfn_t max_gfn = 1ULL << (boot_cpu_data.x86_phys_bits - PAGE_SHIFT); + lockdep_assert_held(&kvm->mmu_lock); WARN_ON(root->root_count); @@ -66,6 +72,8 @@ static void free_tdp_mmu_root(struct kvm *kvm, struct kvm_mmu_page *root) list_del(&root->link); + zap_gfn_range(kvm, root, 0, max_gfn); + free_page((unsigned long)root->spt); kmem_cache_free(mmu_page_header_cache, root); } @@ -193,6 +201,11 @@ hpa_t kvm_tdp_mmu_get_vcpu_root_hpa(struct kvm_vcpu *vcpu) static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn, u64 old_spte, u64 new_spte, int level); +static int kvm_mmu_page_as_id(struct kvm_mmu_page *sp) +{ + return sp->role.smm ? 1 : 0; +} + /** * handle_changed_spte - handle bookkeeping associated with an SPTE change * @kvm: kvm instance @@ -294,3 +307,96 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn, free_page((unsigned long)pt); } } + +#define for_each_tdp_pte_root(_iter, _root, _start, _end) \ + for_each_tdp_pte(_iter, _root->spt, _root->role.level, _start, _end) + +/* + * If the MMU lock is contended or this thread needs to yield, flushes + * the TLBs, releases, the MMU lock, yields, reacquires the MMU lock, + * restarts the tdp_iter's walk from the root, and returns true. + * If no yield is needed, returns false. + */ +static bool tdp_mmu_iter_cond_resched(struct kvm *kvm, struct tdp_iter *iter) +{ + if (need_resched() || spin_needbreak(&kvm->mmu_lock)) { + kvm_flush_remote_tlbs(kvm); + cond_resched_lock(&kvm->mmu_lock); + tdp_iter_refresh_walk(iter); + return true; + } else { + return false; + } +} + +/* + * Tears down the mappings for the range of gfns, [start, end), and frees the + * non-root pages mapping GFNs strictly within that range. Returns true if + * SPTEs have been cleared and a TLB flush is needed before releasing the + * MMU lock. + */ +static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, + gfn_t start, gfn_t end) +{ + struct tdp_iter iter; + bool flush_needed = false; + int as_id = kvm_mmu_page_as_id(root); + + for_each_tdp_pte_root(iter, root, start, end) { + if (!is_shadow_present_pte(iter.old_spte)) + continue; + + /* + * If this is a non-last-level SPTE that covers a larger range + * than should be zapped, continue, and zap the mappings at a + * lower level. + */ + if ((iter.gfn < start || + iter.gfn + KVM_PAGES_PER_HPAGE(iter.level) > end) && + !is_last_spte(iter.old_spte, iter.level)) + continue; + + *iter.sptep = 0; + handle_changed_spte(kvm, as_id, iter.gfn, iter.old_spte, 0, + iter.level); + + flush_needed = !tdp_mmu_iter_cond_resched(kvm, &iter); + } + return flush_needed; +} + +/* + * Tears down the mappings for the range of gfns, [start, end), and frees the + * non-root pages mapping GFNs strictly within that range. Returns true if + * SPTEs have been cleared and a TLB flush is needed before releasing the + * MMU lock. + */ +bool kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, gfn_t start, gfn_t end) +{ + struct kvm_mmu_page *root; + bool flush = false; + + for_each_tdp_mmu_root(kvm, root) { + /* + * 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); + + flush = zap_gfn_range(kvm, root, start, end) || flush; + + put_tdp_mmu_root(kvm, root); + } + + return flush; +} + +void kvm_tdp_mmu_zap_all(struct kvm *kvm) +{ + gfn_t max_gfn = 1ULL << (boot_cpu_data.x86_phys_bits - PAGE_SHIFT); + bool flush; + + flush = kvm_tdp_mmu_zap_gfn_range(kvm, 0, max_gfn); + if (flush) + kvm_flush_remote_tlbs(kvm); +} diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h index 9274debffeaa1..cb86f9fe69017 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.h +++ b/arch/x86/kvm/mmu/tdp_mmu.h @@ -12,4 +12,6 @@ bool is_tdp_mmu_root(struct kvm *kvm, hpa_t root); hpa_t kvm_tdp_mmu_get_vcpu_root_hpa(struct kvm_vcpu *vcpu); void kvm_tdp_mmu_put_root_hpa(struct kvm *kvm, hpa_t root_hpa); +bool kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, gfn_t start, gfn_t end); +void kvm_tdp_mmu_zap_all(struct kvm *kvm); #endif /* __KVM_X86_MMU_TDP_MMU_H */
Add functions to zap SPTEs to the TDP MMU. These are needed to tear down TDP MMU roots properly and implement other MMU functions which require tearing down mappings. Future patches will add functions to populate the page tables, but as for this patch there will not be any work for these functions to do. Tested by running kvm-unit-tests and KVM selftests on an Intel Haswell machine. This series introduced no new failures. This series can be viewed in Gerrit at: https://linux-review.googlesource.com/c/virt/kvm/kvm/+/2538 Signed-off-by: Ben Gardon <bgardon@google.com> --- arch/x86/kvm/mmu/mmu.c | 15 +++++ arch/x86/kvm/mmu/tdp_iter.c | 17 ++++++ arch/x86/kvm/mmu/tdp_iter.h | 1 + arch/x86/kvm/mmu/tdp_mmu.c | 106 ++++++++++++++++++++++++++++++++++++ arch/x86/kvm/mmu/tdp_mmu.h | 2 + 5 files changed, 141 insertions(+)