KVM: mmu: Add slots_arch_lock for memslot arch fields

Add a new lock to protect the arch-specific fields of memslots if they
need to be modified in a kvm->srcu read critical section. A future
commit will use this lock to lazily allocate memslot rmaps for x86.

Signed-off-by: Ben Gardon <bgardon@google.com>
Message-Id: <20210518173414.450044-5-bgardon@google.com>
[Add Documentation/ hunk. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/Documentation/virt/kvm/locking.rst b/Documentation/virt/kvm/locking.rst
index 1fc860c007a3c5899ab832da4fa164e7fafbfb64..35eca377543dfd19e4f0b1ad8632399061094b22 100644 (file)
--- a/Documentation/virt/kvm/locking.rst
+++ b/Documentation/virt/kvm/locking.rst
@@ -16,6 +16,11 @@ The acquisition orders for mutexes are as follows:
 - kvm->slots_lock is taken outside kvm->irq_lock, though acquiring
   them together is quite rare.
 
+- Unlike kvm->slots_lock, kvm->slots_arch_lock is released before
+  synchronize_srcu(&kvm->srcu).  Therefore kvm->slots_arch_lock
+  can be taken inside a kvm->srcu read-side critical section,
+  while kvm->slots_lock cannot.
+
 On x86:
 
 - vcpu->mutex is taken outside kvm->arch.hyperv.hv_lock

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8583ed3ff3447d0eb1cc288cb216f680f1b19065..11b9b11a5e9b06417e781cad0a7cff155fd1880f 100644 (file)
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -523,6 +523,15 @@ struct kvm {
 #endif /* KVM_HAVE_MMU_RWLOCK */
 
        struct mutex slots_lock;
+
+       /*
+        * Protects the arch-specific fields of struct kvm_memory_slots in
+        * use by the VM. To be used under the slots_lock (above) or in a
+        * kvm->srcu critical section where acquiring the slots_lock would
+        * lead to deadlock with the synchronize_srcu in
+        * install_new_memslots.
+        */
+       struct mutex slots_arch_lock;
        struct mm_struct *mm; /* userspace tied to this vm */
        struct kvm_memslots __rcu *memslots[KVM_ADDRESS_SPACE_NUM];
        struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d65be9461493736cebfbaf02d6fe746cf6251750..fa7e7ebefc7962dc767abfa991001cfa0f89e104 100644 (file)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -909,6 +909,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
        mutex_init(&kvm->lock);
        mutex_init(&kvm->irq_lock);
        mutex_init(&kvm->slots_lock);
+       mutex_init(&kvm->slots_arch_lock);
        INIT_LIST_HEAD(&kvm->devices);
 
        BUILD_BUG_ON(KVM_MEM_SLOTS_NUM > SHRT_MAX);
@@ -1281,6 +1282,14 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm,
        slots->generation = gen | KVM_MEMSLOT_GEN_UPDATE_IN_PROGRESS;
 
        rcu_assign_pointer(kvm->memslots[as_id], slots);
+
+       /*
+        * Acquired in kvm_set_memslot. Must be released before synchronize
+        * SRCU below in order to avoid deadlock with another thread
+        * acquiring the slots_arch_lock in an srcu critical section.
+        */
+       mutex_unlock(&kvm->slots_arch_lock);
+
        synchronize_srcu_expedited(&kvm->srcu);
 
        /*
@@ -1352,9 +1361,27 @@ static int kvm_set_memslot(struct kvm *kvm,
        struct kvm_memslots *slots;
        int r;
 
+       /*
+        * Released in install_new_memslots.
+        *
+        * Must be held from before the current memslots are copied until
+        * after the new memslots are installed with rcu_assign_pointer,
+        * then released before the synchronize srcu in install_new_memslots.
+        *
+        * When modifying memslots outside of the slots_lock, must be held
+        * before reading the pointer to the current memslots until after all
+        * changes to those memslots are complete.
+        *
+        * These rules ensure that installing new memslots does not lose
+        * changes made to the previous memslots.
+        */
+       mutex_lock(&kvm->slots_arch_lock);
+
        slots = kvm_dup_memslots(__kvm_memslots(kvm, as_id), change);
-       if (!slots)
+       if (!slots) {
+               mutex_unlock(&kvm->slots_arch_lock);
                return -ENOMEM;
+       }
 
        if (change == KVM_MR_DELETE || change == KVM_MR_MOVE) {
                /*
@@ -1365,10 +1392,9 @@ static int kvm_set_memslot(struct kvm *kvm,
                slot->flags |= KVM_MEMSLOT_INVALID;
 
                /*
-                * We can re-use the old memslots, the only difference from the
-                * newly installed memslots is the invalid flag, which will get
-                * dropped by update_memslots anyway.  We'll also revert to the
-                * old memslots if preparing the new memory region fails.
+                * We can re-use the memory from the old memslots.
+                * It will be overwritten with a copy of the new memslots
+                * after reacquiring the slots_arch_lock below.
                 */
                slots = install_new_memslots(kvm, as_id, slots);
 
@@ -1380,6 +1406,17 @@ static int kvm_set_memslot(struct kvm *kvm,
                 *      - kvm_is_visible_gfn (mmu_check_root)
                 */
                kvm_arch_flush_shadow_memslot(kvm, slot);
+
+               /* Released in install_new_memslots. */
+               mutex_lock(&kvm->slots_arch_lock);
+
+               /*
+                * The arch-specific fields of the memslots could have changed
+                * between releasing the slots_arch_lock in
+                * install_new_memslots and here, so get a fresh copy of the
+                * slots.
+                */
+               kvm_copy_memslots(slots, __kvm_memslots(kvm, as_id));
        }
 
        r = kvm_arch_prepare_memory_region(kvm, new, mem, change);
@@ -1395,8 +1432,13 @@ static int kvm_set_memslot(struct kvm *kvm,
        return 0;
 
 out_slots:
-       if (change == KVM_MR_DELETE || change == KVM_MR_MOVE)
+       if (change == KVM_MR_DELETE || change == KVM_MR_MOVE) {
+               slot = id_to_memslot(slots, old->id);
+               slot->flags &= ~KVM_MEMSLOT_INVALID;
                slots = install_new_memslots(kvm, as_id, slots);
+       } else {
+               mutex_unlock(&kvm->slots_arch_lock);
+       }
        kvfree(slots);
        return r;
 }