[PATCH v2 0/4] KVM: x86/mmu: Fix pre-fault and map private loops

[PATCH v2 0/4] KVM: x86/mmu: Fix pre-fault and map private loops

[Sean Christopherson]

Fix a bug in the pre-fault path where KVM fails to reload an invalidated
MMU root, which puts the KVM_PRE_FAULT_MEMORY task into an infinite loop
(although it's breakable, so not fatal to the host).  My best guess is that
the test started failing once PREEMPT_LAZY was enabled by default.  Note,
the bug is *really* easy to repro with a to-be-proposed patch to have KVM
do auto-pre-faulting[*], i.e. prefetch surrounding pages on fault.

Then harden the similar "map private PFN" to also guard against unexpected
root invalidations, because Sashiko keeps pointing out that it's theoretically
possible for that code to end up in the same type of infinite loop.

[*] https://lore.kernel.org/all/ao4CufEI_pRCjbMF@google.com

v2:
 - Collect reviews. [Rick, Kai]
 - Tweak the slots_comment in kvm_tdp_mmu_map_private_pfn() to better capture
   the nuances of KVM_REQ_MMU_FREE_OBSOLETE_ROOTS. [Rick]
 - Call out in the changelog for patch 2 that simply warning on
   KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is flawed, but handled in a subsequent
   patch. [Sashiko]
 - Make it more clear that encountering retry in kvm_tdp_mmu_map_private_pfn()
   can only happen if there are KVM bugs. [Rick]
 - Set r to RET_PF_RETRY when a stale page fault is detected. [Sashiko]

v1: https://lore.kernel.org/all/20260806214050.78058-1-seanjc@google.com

Sean Christopherson (4):
  KVM: x86/mmu: Reload MMU on *every* page pre-fault attempt/iteration
  KVM: x86/mmu: Harden "map private PFN" against unexpected root
    invalidation
  KVM: x86/mmu: Top-up memory caches when retrying "map private PFN"
  KVM: x86/mmu: Add sanity check to detect stale page faults in "map
    private PFN"

 arch/x86/kvm/mmu/mmu.c | 48 ++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 16 deletions(-)


base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH v2 1/4] KVM: x86/mmu: Reload MMU on *every* page pre-fault attempt/iteration

[Sean Christopherson]

Reload the MMU (which is a nop if the MMU doesn't need to be reloaded) on
every attempt to pre-fault a guest page, i.e. when the page fault path
signals that the caller should retry.  If the synchronize_srcu_expedited()
in kvm_invalidate_memslot() completes before kvm_vcpu_pre_fault_memory()
grabs SRCU, but kvm_mmu_reload() in the pre-fault path completes before
kvm_invalidate_memslot() triggers x86's "fast zap all", then the pre-fault
task will reach kvm_tdp_page_prefault() with an invalid root.

Attempting to fault-in memory with an invalid root ultimately puts
kvm_tdp_page_prefault() into an infinite (breakable) retry loop, which
manifests most obviously as a hang in the pre_fault_memory_test selftest,
but also eventually causes RCU (SRCU?) to complain.

  INFO: rcu_tasks detected stalls on tasks:
  000000000cda47bd: .. nvcsw: 6/6 holdout: 1 idle_cpu: -1/25
  task:pre_fault_memor state:R  running task     stack:12696
  pid:95588 tgid:95588 ppid:95584  task_flags:0x400000 flags:0x00080801
  Call Trace:
   <TASK>
   lock_release+0x4e/0x320
   __get_user_pages+0x546/0xcd0
   up_read+0x1b/0x30
   get_user_pages_unlocked+0xee/0x350
   hva_to_pfn+0xd3/0x3d0 [kvm]
   lock_release+0x4e/0x320
   xa_load+0x5c/0x170
   xa_load+0x14c/0x170
   __kvm_faultin_pfn+0xd9/0x130 [kvm]
   lock_acquire+0x65/0x2b0
   lock_release+0x4e/0x320
   kvm_mmu_faultin_pfn+0x1e1/0x690 [kvm]
   gup_fast_fallback+0x63e/0xdf0
   kvm_tdp_page_fault+0xeb/0x140 [kvm]
   kvm_mmu_do_page_fault+0x12e/0x200 [kvm]
   kvm_arch_vcpu_pre_fault_memory+0x16e/0x200 [kvm]
   kvm_vcpu_pre_fault_memory+0xc1/0x1f0 [kvm]
   kvm_vcpu_pre_fault_memory+0x116/0x1f0 [kvm]
   kvm_vcpu_ioctl+0x3a4/0x6b0 [kvm]
   clockevents_program_event+0x5d/0x170
   __se_sys_ioctl+0x6d/0xb0
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
   do_syscall_64+0x10a/0x480
   __irq_exit_rcu+0x8e/0x140
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
   </TASK>

Fixes: 6e01b7601dfe ("KVM: x86: Implement kvm_arch_vcpu_pre_fault_memory()")
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..3220f05387b5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5061,6 +5061,10 @@ static int kvm_tdp_page_prefault(struct kvm_vcpu *vcpu, gpa_t gpa,
 		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu))
 			return -EIO;
 
+		r = kvm_mmu_reload(vcpu);
+		if (r)
+			return r;
+
 		cond_resched();
 		r = kvm_mmu_do_page_fault(vcpu, gpa, error_code, true, NULL, level);
 	} while (r == RET_PF_RETRY);
@@ -5101,14 +5105,6 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
 	if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa)))
 		return -EINVAL;
 
-	/*
-	 * reload is efficient when called repeatedly, so we can do it on
-	 * every iteration.
-	 */
-	r = kvm_mmu_reload(vcpu);
-	if (r)
-		return r;
-
 	direct_bits = 0;
 	if (kvm_arch_has_private_mem(vcpu->kvm) &&
 	    kvm_mem_is_private(vcpu->kvm, gpa_to_gfn(range->gpa)))
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH v2 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

[Sean Christopherson]

Move kvm_tdp_mmu_map_private_pfn()'s reload of the MMU into its tight loop
so that an unexpected root invalidation has a better chance of being
handled gracefully, even though it should be impossible for the vCPU's root
to be invalidated after the initial reload.  As is, encountering an invalid
root is *guaranteed* to put the task into an infinite loop (albeit a
breakable loop that honors NEED_RESCHED).

Add a WARN to try and detect bugs that break KVM's expectations, along with
a comment to explain why it should be impossible for the root to be
invalidated.

Note, the loop in question doesn't actually check for a stale page fault,
i.e. likely won't detect an invalid loop in the first place.  That bug will
be addressed shortly.

Cc: Kai Huang <kai.huang@intel.com>
Cc: Yan Zhao <yan.y.zhao@intel.com>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 3220f05387b5..1969c26861e5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5209,10 +5209,6 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 	if (kvm_gfn_is_write_tracked(kvm, fault.slot, fault.gfn))
 		return -EPERM;
 
-	r = kvm_mmu_reload(vcpu);
-	if (r)
-		return r;
-
 	r = mmu_topup_memory_caches(vcpu, false);
 	if (r)
 		return r;
@@ -5224,10 +5220,22 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
 			return -EIO;
 
+		r = kvm_mmu_reload(vcpu);
+		if (r)
+			return r;
+
 		cond_resched();
 
 		guard(read_lock)(&kvm->mmu_lock);
 
+		/*
+		 * Because slots_lock is held, it should be impossible for *any*
+		 * roots to be invalidated after the initial MMU reload.  WARN,
+		 * but continue on; the above MMU reload will do the right thing
+		 * if the current root is actually invalid.
+		 */
+		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
+
 		r = kvm_tdp_mmu_map(vcpu, &fault);
 	} while (r == RET_PF_RETRY);
 
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH v2 3/4] KVM: x86/mmu: Top-up memory caches when retrying "map private PFN"

[Sean Christopherson]

When mapping a private PFN in TDX's post-populate callback, top-up the
memory caches on every attempt to map the PFN to harden against bugs in the
map flow that could consume cache entries even if mapping ultimately fails.
E.g. as pointed out by Sashiko, the in-progress Dynamic PAMT support could
consume PAMT cache entries on TDX-Module lock contention.

Harden KVM even though consuming an entry on failure is considered a KVM
bug.  Retry should only be encountered if KVM is buggy (the locks held by
the sole call path will prevent retries from being needed due to
TDX-specific details, and memory can be faulted in only once the VM is
TD_STATE_RUNNABLE, and KVM_TDX_INIT_MEM_REGION is only usable if the VM is
*not* TD_STATE_RUNNABLE), top-up is "free" if there's no work to be done,
and populating a TDX guest's memory is a slow path, i.e. there's no
meaningful downside to the hardening.

Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260718061050.E17B01F000E9@smtp.kernel.org
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1969c26861e5..19a501029f08 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5209,10 +5209,6 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 	if (kvm_gfn_is_write_tracked(kvm, fault.slot, fault.gfn))
 		return -EPERM;
 
-	r = mmu_topup_memory_caches(vcpu, false);
-	if (r)
-		return r;
-
 	do {
 		if (signal_pending(current))
 			return -EINTR;
@@ -5224,6 +5220,10 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 		if (r)
 			return r;
 
+		r = mmu_topup_memory_caches(vcpu, false);
+		if (r)
+			return r;
+
 		cond_resched();
 
 		guard(read_lock)(&kvm->mmu_lock);
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH v2 4/4] KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"

[Sean Christopherson]

Harden the "map private PFN" flow against potentially-fatal bugs or future
KVM changes by checking for a stale "fault" prior to actually mapping the
PFN into the guest.  While it should be impossible for the "page fault" to
become stale, the sanity check is cheap, whereas a broken assumption would
have a high probability of leading to a guest-expoitable use-after-free.

Snapshot the invalidation sequence after acquiring mmu_lock to avoid false
positives, even though doing so completely voids anys and all protection
against unexpected invalidations.  Pretty much the entire point of
kvm_tdp_mmu_map_private_pfn() is that it allows mapping a PFN that was
gifted by the caller, i.e. the caller would have to mess up its one and
only responsibility.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 19a501029f08..79c450d677b4 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5236,6 +5236,18 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 		 */
 		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
 
+		/*
+		 * Snapshot the invalidation sequence counter after acquiring
+		 * mmu_lock, as guest_memfd guarantees the validity of the pfn,
+		 * i.e. any concurrent invalidations are guaranteed to be
+		 * irrelevant.
+		 */
+		fault.mmu_seq = vcpu->kvm->mmu_invalidate_seq;
+		if (is_page_fault_stale(vcpu, &fault)) {
+			r = RET_PF_RETRY;
+			continue;
+		}
+
 		r = kvm_tdp_mmu_map(vcpu, &fault);
 	} while (r == RET_PF_RETRY);
 
-- 
2.55.0.860.g4b6b3295ed-goog

Re: [PATCH v2 4/4] KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Snapshotting `mmu_invalidate_seq` after acquiring `mmu_lock` and after PFN resolution violates the mandatory MMU retry protocol sequence, bypassing generation-safety checks against concurrent invalidations.
--

commit 3d8d7ec872ba8330e37f8fdcf1804dda38c1cf1e
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"

This commit adds a check for stale page faults prior to mapping a private PFN
into the guest. It intentionally snapshots the invalidation sequence after
acquiring the MMU lock to avoid false positives.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 19a501029f089..79c450d677b40 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5236,6 +5236,18 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
>  		 */
>  		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
>  
> +		/*
> +		 * Snapshot the invalidation sequence counter after acquiring
> +		 * mmu_lock, as guest_memfd guarantees the validity of the pfn,
> +		 * i.e. any concurrent invalidations are guaranteed to be
> +		 * irrelevant.
> +		 */
> +		fault.mmu_seq = vcpu->kvm->mmu_invalidate_seq;
> +		if (is_page_fault_stale(vcpu, &fault)) {

[Severity: High]
Does capturing mmu_invalidate_seq after acquiring mmu_lock and after PFN
resolution bypass the mandatory MMU retry protocol safety checks?

According to the KVM memory management guidelines, the invalidation sequence
should be captured before resolving the PFN, and the lock must be acquired
afterward. By snapshotting the sequence here and immediately checking it
via is_page_fault_stale(), the check effectively compares the counter to
itself, which guarantees it will pass.

Could this bypass allow the installation of stale translations or cause a
use-after-free if the caller's guarantees ever fail in the future?

> +			r = RET_PF_RETRY;
> +			continue;
> +		}
> +
>  		r = kvm_tdp_mmu_map(vcpu, &fault);
>  	} while (r == RET_PF_RETRY);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826164214.756512-1-seanjc@google.com?part=4

Re: [PATCH v2 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The WARN_ON_ONCE diagnostic to detect unexpected root invalidations evaluates after the request bit is silently cleared by kvm_mmu_reload(), rendering it effectively useless.
--

commit 5bfe54f32e3516c8d2a1f4dc3a29739732e31eb5
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

This commit moves the MMU reload inside the tight loop in
kvm_tdp_mmu_map_private_pfn() to better handle unexpected root invalidations.
It also adds a warning to detect unexpected invalidations that break KVM
expectations.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 3220f05387b58..1969c26861e5a 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
[ ... ]
> @@ -5224,10 +5220,22 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
>  		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
>  			return -EIO;
>  
> +		r = kvm_mmu_reload(vcpu);
> +		if (r)
> +			return r;
> +
>  		cond_resched();
>  
>  		guard(read_lock)(&kvm->mmu_lock);
>  
> +		/*
> +		 * Because slots_lock is held, it should be impossible for *any*
> +		 * roots to be invalidated after the initial MMU reload.  WARN,
> +		 * but continue on; the above MMU reload will do the right thing
> +		 * if the current root is actually invalid.
> +		 */
> +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));

[Severity: Medium]
Will this WARN_ON_ONCE ever trigger?

Looking at kvm_mmu_reload(), it calls kvm_check_request() which consumes and
clears the request bit:

static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
{
        if (kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu))
                kvm_mmu_free_obsolete_roots(vcpu);
...

Since kvm_mmu_reload() is called unconditionally just above in the same loop,
won't the KVM_REQ_MMU_FREE_OBSOLETE_ROOTS bit always be cleared by the time
execution reaches this check in kvm_tdp_mmu_map_private_pfn()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826164214.756512-1-seanjc@google.com?part=2

Re: [PATCH v2 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

[Sean Christopherson]

On Wed, Aug 26, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 3220f05387b58..1969c26861e5a 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> [ ... ]
> > @@ -5224,10 +5220,22 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
> >  		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
> >  			return -EIO;
> >  
> > +		r = kvm_mmu_reload(vcpu);
> > +		if (r)
> > +			return r;
> > +
> >  		cond_resched();
> >  
> >  		guard(read_lock)(&kvm->mmu_lock);
> >  
> > +		/*
> > +		 * Because slots_lock is held, it should be impossible for *any*
> > +		 * roots to be invalidated after the initial MMU reload.  WARN,
> > +		 * but continue on; the above MMU reload will do the right thing
> > +		 * if the current root is actually invalid.
> > +		 */
> > +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> 
> [Severity: Medium]
> Will this WARN_ON_ONCE ever trigger?
> 
> Looking at kvm_mmu_reload(), it calls kvm_check_request() which consumes and
> clears the request bit:
> 
> static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
> {
>         if (kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu))
>                 kvm_mmu_free_obsolete_roots(vcpu);
> ...
> 
> Since kvm_mmu_reload() is called unconditionally just above in the same loop,
> won't the KVM_REQ_MMU_FREE_OBSOLETE_ROOTS bit always be cleared by the time
> execution reaches this check in kvm_tdp_mmu_map_private_pfn()?

More than likely, but not always since it's a cross-vCPU request.  As stated in
v1, the goal is more about documentation than about finding bugs.