Skip to content

Commit dab285e

Browse files
committed
KVM: x86/mmu: Alloc TDP MMU roots while holding mmu_lock for read
Allocate TDP MMU roots while holding mmu_lock for read, and instead use tdp_mmu_pages_lock to guard against duplicate roots. This allows KVM to create new roots without forcing kvm_tdp_mmu_zap_invalidated_roots() to yield, e.g. allows vCPUs to load new roots after memslot deletion without forcing the zap thread to detect contention and yield (or complete if the kernel isn't preemptible). Note, creating a new TDP MMU root as an mmu_lock reader is safe for two reasons: (1) paths that must guarantee all roots/SPTEs are *visited* take mmu_lock for write and so are still mutually exclusive, e.g. mmu_notifier invalidations, and (2) paths that require all roots/SPTEs to *observe* some given state without holding mmu_lock for write must ensure freshness through some other means, e.g. toggling dirty logging must first wait for SRCU readers to recognize the memslot flags change before processing existing roots/SPTEs. Link: https://lore.kernel.org/r/20240111020048.844847-8-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent f5238c2 commit dab285e

File tree

1 file changed

+22
-33
lines changed

1 file changed

+22
-33
lines changed

arch/x86/kvm/mmu/tdp_mmu.c

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -223,51 +223,42 @@ static void tdp_mmu_init_child_sp(struct kvm_mmu_page *child_sp,
223223
tdp_mmu_init_sp(child_sp, iter->sptep, iter->gfn, role);
224224
}
225225

226-
static struct kvm_mmu_page *kvm_tdp_mmu_try_get_root(struct kvm_vcpu *vcpu)
227-
{
228-
union kvm_mmu_page_role role = vcpu->arch.mmu->root_role;
229-
int as_id = kvm_mmu_role_as_id(role);
230-
struct kvm *kvm = vcpu->kvm;
231-
struct kvm_mmu_page *root;
232-
233-
for_each_valid_tdp_mmu_root_yield_safe(kvm, root, as_id) {
234-
if (root->role.word == role.word)
235-
return root;
236-
}
237-
238-
return NULL;
239-
}
240-
241226
int kvm_tdp_mmu_alloc_root(struct kvm_vcpu *vcpu)
242227
{
243228
struct kvm_mmu *mmu = vcpu->arch.mmu;
244229
union kvm_mmu_page_role role = mmu->root_role;
230+
int as_id = kvm_mmu_role_as_id(role);
245231
struct kvm *kvm = vcpu->kvm;
246232
struct kvm_mmu_page *root;
247233

248234
/*
249-
* Check for an existing root while holding mmu_lock for read to avoid
235+
* Check for an existing root before acquiring the pages lock to avoid
250236
* unnecessary serialization if multiple vCPUs are loading a new root.
251237
* E.g. when bringing up secondary vCPUs, KVM will already have created
252238
* a valid root on behalf of the primary vCPU.
253239
*/
254240
read_lock(&kvm->mmu_lock);
255-
root = kvm_tdp_mmu_try_get_root(vcpu);
256-
read_unlock(&kvm->mmu_lock);
257241

258-
if (root)
259-
goto out;
242+
for_each_valid_tdp_mmu_root_yield_safe(kvm, root, as_id) {
243+
if (root->role.word == role.word)
244+
goto out_read_unlock;
245+
}
260246

261-
write_lock(&kvm->mmu_lock);
247+
spin_lock(&kvm->arch.tdp_mmu_pages_lock);
262248

263249
/*
264-
* Recheck for an existing root after acquiring mmu_lock for write. It
265-
* is possible a new usable root was created between dropping mmu_lock
266-
* (for read) and acquiring it for write.
250+
* Recheck for an existing root after acquiring the pages lock, another
251+
* vCPU may have raced ahead and created a new usable root. Manually
252+
* walk the list of roots as the standard macros assume that the pages
253+
* lock is *not* held. WARN if grabbing a reference to a usable root
254+
* fails, as the last reference to a root can only be put *after* the
255+
* root has been invalidated, which requires holding mmu_lock for write.
267256
*/
268-
root = kvm_tdp_mmu_try_get_root(vcpu);
269-
if (root)
270-
goto out_unlock;
257+
list_for_each_entry(root, &kvm->arch.tdp_mmu_roots, link) {
258+
if (root->role.word == role.word &&
259+
!WARN_ON_ONCE(!kvm_tdp_mmu_get_root(root)))
260+
goto out_spin_unlock;
261+
}
271262

272263
root = tdp_mmu_alloc_sp(vcpu);
273264
tdp_mmu_init_sp(root, NULL, 0, role);
@@ -280,14 +271,12 @@ int kvm_tdp_mmu_alloc_root(struct kvm_vcpu *vcpu)
280271
* is ultimately put by kvm_tdp_mmu_zap_invalidated_roots().
281272
*/
282273
refcount_set(&root->tdp_mmu_root_count, 2);
283-
284-
spin_lock(&kvm->arch.tdp_mmu_pages_lock);
285274
list_add_rcu(&root->link, &kvm->arch.tdp_mmu_roots);
286-
spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
287275

288-
out_unlock:
289-
write_unlock(&kvm->mmu_lock);
290-
out:
276+
out_spin_unlock:
277+
spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
278+
out_read_unlock:
279+
read_unlock(&kvm->mmu_lock);
291280
/*
292281
* Note, KVM_REQ_MMU_FREE_OBSOLETE_ROOTS will prevent entering the guest
293282
* and actually consuming the root if it's invalidated after dropping

0 commit comments

Comments
 (0)