Re: KVM: x86/mmu: __kvm_rmap_lock() preemption assert trips on PREEMPT_RT

[Sean Christopherson]

On Thu, Aug 27, 2026, David Woodhouse wrote:
> While soaking unrelated KVM changes on a PREEMPT_RT + lockdep kernel
> I hit this, which I don't believe has been reported before:

Heh, not on-list.  I've hit it the few times I've run PREEMPT_RT, but I (obviously)
haven't cared enough to fix it, especially given the lack of external reports.
Which makes sense; I know people run KVM with PREEMPT_RT, but running nested,
memory-overcommitted VMs with PREEMPT_RT seems beyond crazy.

>   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]
> 
> That's the lockdep_assert_preemption_disabled() in __kvm_rmap_lock(),
> from commit 4834eaded91e ("KVM: x86/mmu: Add infrastructure to allow
> walking rmaps outside of mmu_lock").
> 
> I don't think this one is just lockdep vs. PREEMPT_RT causing false
> positives — the rmap lock is a hand-crafted bit-spinlock, and if a lock
> holder is preempted that leaves every other walker of that rmap
> spinning and waiting for it (with no tracked owner for PI to boost).
>
> On PREEMPT_RT we genuinely get here without preemption disabled,
> because kvm->mmu_lock is a sleeping lock now. Any shadow-MMU fault
> on an RT kernel should trip it — this one is the !TDP nested EPT path
> (ept_fetch()), and it fires within seconds of running a nested guest
> with lockdep enabled.

Yep.

> I guess we fix it by turning the assertion into a preempt_disable() of
> its own? Not sufficiently confident in that conclusion to send it in
> 'diff -up' form though...

Or in the spirit of PREEMPT_RT, make the aging code acquire mmu_lock?  As above,
I have a hard time believing anyone cares about aging throughput of nested VMs
when running PREEMPT_RT, certainly not enough to want to disable preemption for
any amount of time.

Not the prettiest code, but it does seem like the right thing to do for PREEMPT_RT.
Compile-tested only at this point.  I'll take it for a spin, unless someone has a
better idea.

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 81c30e2c74f3..31a372d0697c 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -913,11 +913,15 @@ static struct kvm_memory_slot *gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu
  */
 #define KVM_RMAP_LOCKED	BIT(1)
 
-static unsigned long __kvm_rmap_lock(struct kvm_rmap_head *rmap_head)
+static unsigned long __kvm_rmap_lock(struct kvm *kvm,
+				     struct kvm_rmap_head *rmap_head)
 {
 	unsigned long old_val, new_val;
 
-	lockdep_assert_preemption_disabled();
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		lockdep_assert_preemption_disabled();
+	else
+		lockdep_assert_held(&kvm->mmu_lock);
 
 	/*
 	 * Elide the lock if the rmap is empty, as lockless walkers (read-only
@@ -973,7 +977,7 @@ static unsigned long kvm_rmap_lock(struct kvm *kvm,
 {
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
-	return __kvm_rmap_lock(rmap_head);
+	return __kvm_rmap_lock(kvm, rmap_head);
 }
 
 static void __kvm_rmap_unlock(struct kvm_rmap_head *rmap_head,
@@ -1008,14 +1012,17 @@ 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;
 
-	preempt_disable();
-	rmap_val = __kvm_rmap_lock(rmap_head);
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_disable();
 
-	if (!rmap_val)
+	rmap_val = __kvm_rmap_lock(kvm, rmap_head);
+
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !rmap_val)
 		preempt_enable();
 
 	return rmap_val;
@@ -1030,7 +1037,9 @@ static void kvm_rmap_unlock_readonly(struct kvm_rmap_head *rmap_head,
 	KVM_MMU_WARN_ON(old_val != kvm_rmap_get(rmap_head));
 
 	__kvm_rmap_unlock(rmap_head, old_val);
-	preempt_enable();
+
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
 }
 
 /*
@@ -1745,11 +1754,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))