Revert "KVM: x86/mmu: Allow zap gfn range to operate under the mmu read lock"

This together with the next patch will fix a future race between
kvm_zap_gfn_range and the page fault handler, which will happen
when AVIC memslot is going to be only partially disabled.

The performance impact is minimal since kvm_zap_gfn_range is only
called by users, update_mtrr() and kvm_post_set_cr0().

Both only use it if the guest has non-coherent DMA, in order to
honor the guest's UC memtype.

MTRR and CD setup only happens at boot, and generally in an area
where the page tables should be small (for CD) or should not
include the affected GFNs at all (for MTRRs).

This is based on a patch suggested by Sean Christopherson:
https://lkml.org/lkml/2021/7/22/1025

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20210810205251.424103-2-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 5daa8a9..9a71996 100644 (file)
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5682,8 +5682,9 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
        int i;
        bool flush = false;
 
+       write_lock(&kvm->mmu_lock);
+
        if (kvm_memslots_have_rmaps(kvm)) {
-               write_lock(&kvm->mmu_lock);
                for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) {
                        slots = __kvm_memslots(kvm, i);
                        kvm_for_each_memslot(memslot, slots) {
@@ -5703,22 +5704,18 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
                }
                if (flush)
                        kvm_flush_remote_tlbs_with_address(kvm, gfn_start, gfn_end);
-               write_unlock(&kvm->mmu_lock);
        }
 
        if (is_tdp_mmu_enabled(kvm)) {
-               flush = false;
-
-               read_lock(&kvm->mmu_lock);
                for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
                        flush = kvm_tdp_mmu_zap_gfn_range(kvm, i, gfn_start,
-                                                         gfn_end, flush, true);
-               if (flush)
-                       kvm_flush_remote_tlbs_with_address(kvm, gfn_start,
-                                                          gfn_end);
-
-               read_unlock(&kvm->mmu_lock);
+                                                         gfn_end, flush);
        }
+
+       if (flush)
+               kvm_flush_remote_tlbs_with_address(kvm, gfn_start, gfn_end);
+
+       write_unlock(&kvm->mmu_lock);
 }
 
 static bool slot_rmap_write_protect(struct kvm *kvm,

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index a314643..fb1b2dc 100644 (file)
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -818,21 +818,15 @@ retry:
  * 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.
- *
- * 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 in write mode.
  */
 bool __kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id, gfn_t start,
-                                gfn_t end, bool can_yield, bool flush,
-                                bool shared)
+                                gfn_t end, bool can_yield, bool flush)
 {
        struct kvm_mmu_page *root;
 
-       for_each_tdp_mmu_root_yield_safe(kvm, root, as_id, shared)
+       for_each_tdp_mmu_root_yield_safe(kvm, root, as_id, false)
                flush = zap_gfn_range(kvm, root, start, end, can_yield, flush,
-                                     shared);
+                                     false);
 
        return flush;
 }
@@ -843,8 +837,7 @@ void kvm_tdp_mmu_zap_all(struct kvm *kvm)
        int i;
 
        for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
-               flush = kvm_tdp_mmu_zap_gfn_range(kvm, i, 0, -1ull,
-                                                 flush, false);
+               flush = kvm_tdp_mmu_zap_gfn_range(kvm, i, 0, -1ull, flush);
 
        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 b224d12..358f447 100644 (file)
--- a/arch/x86/kvm/mmu/tdp_mmu.h
+++ b/arch/x86/kvm/mmu/tdp_mmu.h
@@ -20,14 +20,11 @@ void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root,
                          bool shared);
 
 bool __kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id, gfn_t start,
-                                gfn_t end, bool can_yield, bool flush,
-                                bool shared);
+                                gfn_t end, bool can_yield, bool flush);
 static inline bool kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id,
-                                            gfn_t start, gfn_t end, bool flush,
-                                            bool shared)
+                                            gfn_t start, gfn_t end, bool flush)
 {
-       return __kvm_tdp_mmu_zap_gfn_range(kvm, as_id, start, end, true, flush,
-                                          shared);
+       return __kvm_tdp_mmu_zap_gfn_range(kvm, as_id, start, end, true, flush);
 }
 static inline bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
 {
@@ -44,7 +41,7 @@ static inline bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
         */
        lockdep_assert_held_write(&kvm->mmu_lock);
        return __kvm_tdp_mmu_zap_gfn_range(kvm, kvm_mmu_page_as_id(sp),
-                                          sp->gfn, end, false, false, false);
+                                          sp->gfn, end, false, false);
 }
 
 void kvm_tdp_mmu_zap_all(struct kvm *kvm);