| Message ID | 20220303193842.370645-18-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: Overhaul TDP MMU zapping and flushing | expand |
On Thu, Mar 03, 2022, Paolo Bonzini wrote: > From: Sean Christopherson <seanjc@google.com> > > Now that all callers of zap_gfn_range() hold mmu_lock for write, drop > support for zapping with mmu_lock held for read. That all callers hold > mmu_lock for write isn't a random coincidence; now that the paths that > need to zap _everything_ have their own path, the only callers left are > those that need to zap for functional correctness. And when zapping is > required for functional correctness, mmu_lock must be held for write, > otherwise the caller has no guarantees about the state of the TDP MMU > page tables after it has run, e.g. the SPTE(s) it zapped can be > immediately replaced by a vCPU faulting in a page. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > Reviewed-by: Ben Gardon <bgardon@google.com> > Message-Id: <20220226001546.360188-17-seanjc@google.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Mingwei Zhang <mizhang@google.com> > --- > arch/x86/kvm/mmu/tdp_mmu.c | 24 ++++++------------------ > 1 file changed, 6 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index 970376297b30..f3939ce4a115 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -844,15 +844,9 @@ bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp) > * function cannot yield, it will not release the MMU lock or reschedule and > * the caller must ensure it does not supply too large a GFN range, or the > * operation can cause a soft lockup. > - * > - * If shared is true, this thread holds the MMU lock in read mode and must > - * account for the possibility that other threads are modifying the paging > - * structures concurrently. If shared is false, this thread should hold the > - * MMU lock in write mode. > */ > static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > - gfn_t start, gfn_t end, bool can_yield, bool flush, > - bool shared) > + gfn_t start, gfn_t end, bool can_yield, bool flush) > { > bool zap_all = (start == 0 && end >= tdp_mmu_max_gfn_host()); > struct tdp_iter iter; > @@ -865,14 +859,13 @@ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > > end = min(end, tdp_mmu_max_gfn_host()); > > - kvm_lockdep_assert_mmu_lock_held(kvm, shared); > + lockdep_assert_held_write(&kvm->mmu_lock); > > rcu_read_lock(); > > for_each_tdp_pte_min_level(iter, root, min_level, start, end) { > -retry: > if (can_yield && > - tdp_mmu_iter_cond_resched(kvm, &iter, flush, shared)) { > + tdp_mmu_iter_cond_resched(kvm, &iter, flush, false)) { > flush = false; > continue; > } > @@ -891,12 +884,8 @@ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, > !is_last_spte(iter.old_spte, iter.level)) > continue; > > - if (!shared) { > - tdp_mmu_set_spte(kvm, &iter, 0); > - flush = true; > - } else if (tdp_mmu_zap_spte_atomic(kvm, &iter)) { > - goto retry; > - } > + tdp_mmu_set_spte(kvm, &iter, 0); > + flush = true; > } > > rcu_read_unlock(); > @@ -915,8 +904,7 @@ bool __kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id, gfn_t start, > struct kvm_mmu_page *root; > > for_each_tdp_mmu_root_yield_safe(kvm, root, as_id) > - flush = zap_gfn_range(kvm, root, start, end, can_yield, flush, > - false); > + flush = zap_gfn_range(kvm, root, start, end, can_yield, flush); > > return flush; > } > -- > 2.31.1 > >
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index 970376297b30..f3939ce4a115 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -844,15 +844,9 @@ bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp) * function cannot yield, it will not release the MMU lock or reschedule and * the caller must ensure it does not supply too large a GFN range, or the * operation can cause a soft lockup. - * - * If shared is true, this thread holds the MMU lock in read mode and must - * account for the possibility that other threads are modifying the paging - * structures concurrently. If shared is false, this thread should hold the - * MMU lock in write mode. */ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, - gfn_t start, gfn_t end, bool can_yield, bool flush, - bool shared) + gfn_t start, gfn_t end, bool can_yield, bool flush) { bool zap_all = (start == 0 && end >= tdp_mmu_max_gfn_host()); struct tdp_iter iter; @@ -865,14 +859,13 @@ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, end = min(end, tdp_mmu_max_gfn_host()); - kvm_lockdep_assert_mmu_lock_held(kvm, shared); + lockdep_assert_held_write(&kvm->mmu_lock); rcu_read_lock(); for_each_tdp_pte_min_level(iter, root, min_level, start, end) { -retry: if (can_yield && - tdp_mmu_iter_cond_resched(kvm, &iter, flush, shared)) { + tdp_mmu_iter_cond_resched(kvm, &iter, flush, false)) { flush = false; continue; } @@ -891,12 +884,8 @@ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root, !is_last_spte(iter.old_spte, iter.level)) continue; - if (!shared) { - tdp_mmu_set_spte(kvm, &iter, 0); - flush = true; - } else if (tdp_mmu_zap_spte_atomic(kvm, &iter)) { - goto retry; - } + tdp_mmu_set_spte(kvm, &iter, 0); + flush = true; } rcu_read_unlock(); @@ -915,8 +904,7 @@ bool __kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id, gfn_t start, struct kvm_mmu_page *root; for_each_tdp_mmu_root_yield_safe(kvm, root, as_id) - flush = zap_gfn_range(kvm, root, start, end, can_yield, flush, - false); + flush = zap_gfn_range(kvm, root, start, end, can_yield, flush); return flush; }