[PATCH] KVM: x86/mmu: Always guard rmaps with mmu_lock on PREEMPT_RT=y kernels

[Sean Christopherson]

For all intents and purposes, revert KVM's ability to walk rmaps outside of
mmu_lock when running on a realtime (PREEMPT_RT=y) kernel.  I.e. don't use
a non-sleepable bit-spinlock to protect rmap entries, as realtime kernels
are highly unlikely to benefit from increased aging throughput and reduced
jitter for memory-overcommitted nested VMs, whereas using a non-sleepable
lock is currently buggy and goes against the spirit of realtime kernels.

Because KVM's rmap locks are hand-crafted bit-spinlocks, preemption must be
disabled before acquiring the lock, otherwise a preempted lock holder will
result in all other walkers of the locked rmap to spin and wait, with no
tracked owner for PI to boost.  For non-RT kernels, acquiring mmu_lock
suffices, as mmu_lock is a non-sleepable rwlock.  But on RT, where mmu_lock
becomes sleepable, preemption is left enabled for rmap writers:

  WARNING: arch/x86/kvm/mmu/mmu.c:920 at __kvm_rmap_lock+0x1a7/0x1e0 [kvm], CPU#16: vmx_apic_update/3708
  CPU: 16 UID: 0 PID: 3708 Comm: vmx_apic_update Not tainted 7.2.0-rc7 #52 PREEMPT_{RT,LAZY}
  RIP: 0010:__kvm_rmap_lock+0x1a7/0x1e0 [kvm]
  Call Trace:
   pte_list_add+0x67/0x4d0 [kvm]
   __link_shadow_page+0x249/0x480 [kvm]
   ept_fetch+0x4d5/0x1220 [kvm]
   ept_page_fault+0x60b/0x850 [kvm]
   kvm_mmu_do_page_fault+0x252/0x690 [kvm]

Alternatively, KVM could manually disable preemption when grabbing an rmap
lock, but as above, that isn't what RT kernels generally want, and it's
actually more complex to implement (cleanly).

To not completely lose the scaling advantage of per-rmap locks, take
mmu_lock for read in the aging path, i.e. allow multiple concurrent aging
tasks, as the aging code needs to use atomic SPTE accesses no matter what,
i.e. no extra code/work is required to guard against concurrent aging of
SPTEs.

Reported-by: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/all/8d47b43e1829ac92703723e6a1a4afc7a2eaacb5.camel@xxxxxxxxxxxxx
Fixes: 4834eaded91e ("KVM: x86/mmu: Add infrastructure to allow walking rmaps outside of mmu_lock")
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
 arch/x86/kvm/mmu/mmu.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..5bf833550f84 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -895,6 +895,7 @@ static struct kvm_memory_slot *gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu
  */
 #define KVM_RMAP_MANY	BIT(0)
 
+#ifndef CONFIG_PREEMPT_RT
 /*
  * rmaps and PTE lists are mostly protected by mmu_lock (the shadow MMU always
  * operates with mmu_lock held for write), but rmaps can be walked without
@@ -1008,7 +1009,8 @@ static unsigned long kvm_rmap_get(struct kvm_rmap_head *rmap_head)
  * actual locking is the same, but the caller is disallowed from modifying the
  * rmap, and so the unlock flow is a nop if the rmap is/was empty.
  */
-static unsigned long kvm_rmap_lock_readonly(struct kvm_rmap_head *rmap_head)
+static unsigned long kvm_rmap_lock_readonly(struct kvm *kvm,
+					    struct kvm_rmap_head *rmap_head)
 {
 	unsigned long rmap_val;
 
@@ -1032,6 +1034,35 @@ static void kvm_rmap_unlock_readonly(struct kvm_rmap_head *rmap_head,
 	__kvm_rmap_unlock(rmap_head, old_val);
 	preempt_enable();
 }
+#else
+static unsigned long kvm_rmap_get(struct kvm_rmap_head *rmap_head)
+{
+	return atomic_long_read(&rmap_head->val);
+}
+static unsigned long kvm_rmap_lock(struct kvm *kvm,
+				   struct kvm_rmap_head *rmap_head)
+{
+	lockdep_assert_held_write(&kvm->mmu_lock);
+	return kvm_rmap_get(rmap_head);
+}
+
+static void kvm_rmap_unlock(struct kvm *kvm,
+			    struct kvm_rmap_head *rmap_head,
+			    unsigned long new_val)
+{
+	atomic_long_set_release(&rmap_head->val, new_val);
+}
+
+static unsigned long kvm_rmap_lock_readonly(struct kvm *kvm,
+					    struct kvm_rmap_head *rmap_head)
+{
+	lockdep_assert_held_read(&kvm->mmu_lock);
+	return kvm_rmap_get(rmap_head);
+}
+
+static void kvm_rmap_unlock_readonly(struct kvm_rmap_head *rmap_head,
+				     unsigned long old_val) { }
+#endif
 
 /*
  * Returns the number of pointers in the rmap chain, not counting the new one.
@@ -1745,11 +1776,15 @@ static bool kvm_rmap_age_gfn_range(struct kvm *kvm,
 	gfn_t gfn;
 	int level;
 
+#ifdef CONFIG_PREEMPT_RT
+	guard(read_lock)(&kvm->mmu_lock);
+#endif
+
 	for (level = PG_LEVEL_4K; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {
 		for (gfn = range->start; gfn < range->end;
 		     gfn += KVM_PAGES_PER_HPAGE(level)) {
 			rmap_head = gfn_to_rmap(gfn, level, range->slot);
-			rmap_val = kvm_rmap_lock_readonly(rmap_head);
+			rmap_val = kvm_rmap_lock_readonly(kvm, rmap_head);
 
 			for_each_rmap_spte_lockless(rmap_val, &iter, sptep, old_spte) {
 				if (!is_accessed_spte(old_spte))

base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.970.g62bdec98f9-goog