[PATCH 3/3] KVM: x86/mmu: always take tdp_mmu_pages_lock

From: Paolo Bonzini
Date: Thu Sep 28 2023 - 12:30:52 EST


It is cheap to take tdp_mmu_pages_lock in all write-side critical sections.
We already do it all the time when zapping with read_lock(), so it is not
a problem to do it from the kvm_tdp_mmu_zap_all() path (aka
kvm_arch_flush_shadow_all(), aka VM destruction and MMU notifier release).

Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
---
Documentation/virt/kvm/locking.rst | 6 ++----
arch/x86/kvm/mmu/tdp_mmu.c | 17 ++++++-----------
2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/Documentation/virt/kvm/locking.rst b/Documentation/virt/kvm/locking.rst
index 3a034db5e55f..381eb0e7d947 100644
--- a/Documentation/virt/kvm/locking.rst
+++ b/Documentation/virt/kvm/locking.rst
@@ -43,10 +43,8 @@ On x86:

- vcpu->mutex is taken outside kvm->arch.hyperv.hv_lock and kvm->arch.xen.xen_lock

-- kvm->arch.mmu_lock is an rwlock. kvm->arch.tdp_mmu_pages_lock and
- kvm->arch.mmu_unsync_pages_lock are taken inside kvm->arch.mmu_lock, and
- cannot be taken without already holding kvm->arch.mmu_lock (typically with
- ``read_lock`` for the TDP MMU, thus the need for additional spinlocks).
+- kvm->arch.mmu_lock is an rwlock and is taken outside
+ kvm->arch.tdp_mmu_pages_lock and kvm->arch.mmu_unsync_pages_lock

Everything else is a leaf: no other lock is taken inside the critical
sections.
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index b9abfa78808a..f61bc842067f 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -285,24 +285,19 @@ static void tdp_unaccount_mmu_page(struct kvm *kvm, struct kvm_mmu_page *sp)
* the MMU lock and the operation must synchronize with other
* threads that might be adding or removing pages.
*/
-static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp,
- bool shared)
+static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
{
+ lockdep_assert_held(&kvm->mmu_lock);
+
tdp_unaccount_mmu_page(kvm, sp);

if (!sp->nx_huge_page_disallowed)
return;

- if (shared)
- spin_lock(&kvm->arch.tdp_mmu_pages_lock);
- else
- lockdep_assert_held_write(&kvm->mmu_lock);
-
+ spin_lock(&kvm->arch.tdp_mmu_pages_lock);
sp->nx_huge_page_disallowed = false;
untrack_possible_nx_huge_page(kvm, sp);
-
- if (shared)
- spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
+ spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
}

/**
@@ -331,7 +326,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)

trace_kvm_mmu_prepare_zap_page(sp);

- tdp_mmu_unlink_sp(kvm, sp, shared);
+ tdp_mmu_unlink_sp(kvm, sp);

for (i = 0; i < SPTE_ENT_PER_PAGE; i++) {
tdp_ptep_t sptep = pt + i;
--
2.39.1