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

[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@infradead.org>
Closes: https://lore.kernel.org/all/8d47b43e1829ac92703723e6a1a4afc7a2eaacb5.camel@infradead.org
Fixes: 4834eaded91e ("KVM: x86/mmu: Add infrastructure to allow walking rmaps outside of mmu_lock")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 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

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

[James Houghton]

On Wed, Sep 2, 2026 at 2:29 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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@infradead.org>
> Closes: https://lore.kernel.org/all/8d47b43e1829ac92703723e6a1a4afc7a2eaacb5.camel@infradead.org
> Fixes: 4834eaded91e ("KVM: x86/mmu: Add infrastructure to allow walking rmaps outside of mmu_lock")
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Thanks, Sean. A small comment below. Feel free to add:

Reviewed-by: James Houghton <jthoughton@google.com>

> ---
>  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

Future me would be happier if there were a comment here. :)

Also, how about BUILD_BUG_ON(!CONFIG_KVM_MMU_LOCKLESS_AGING)?

> +
>         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
>
>

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

[Sean Christopherson]

On Wed, Sep 02, 2026, James Houghton wrote:
> On Wed, Sep 2, 2026 at 2:29 PM Sean Christopherson <seanjc@google.com> wrote:
> > @@ -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
> 
> Future me would be happier if there were a comment here. :)
> 
> Also, how about BUILD_BUG_ON(!CONFIG_KVM_MMU_LOCKLESS_AGING)?

I completely forgot we even had that Kconfig.  I'm somewhat tempted to gate the
alternative implementation on KVM_MMU_LOCKLESS_AGING=n, not take mmu_lock here,
and then "select KVM_MMU_LOCKLESS_AGING if PREEMPT_RT=n".

My main hesitation is that taking mmu_lock for read would be impossible without
extending kvm_handle_hva_range() even further, which IMO isn't worth doing unless
multiple architectures want to take mmu_lock for read.

Ugh, I shouldn't have thought those thoughts.  s390 does exactly that.

Oh, wait, never mind.  That doesn't work for x86, because we want truly lockless
aging for the TDP MMU.  So ignore that train of thought, this patch is the way
to go.  +1 to adding a BUILD_BUG_ON() and comment thought.

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

On Wed, 2026-09-02 at 14:17 -0700, Sean Christopherson wrote:
> 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@infradead.org>
> Closes: https://lore.kernel.org/all/8d47b43e1829ac92703723e6a1a4afc7a2eaacb5.camel@infradead.org
> Fixes: 4834eaded91e ("KVM: x86/mmu: Add infrastructure to allow walking rmaps outside of mmu_lock")
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Tested-by: David Woodhouse <dwmw@amazon.co.uk>


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]