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

[PATCH 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.

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.

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 | 45 +++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 16 deletions(-)


base-commit: a806d364ef288a6443a1337820ea8410a7ccc6b3
-- 
2.55.0.679.g6767b8d81c-goog

[PATCH 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()")
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 c519e8e8d646..621b0a42f2a1 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5036,6 +5036,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);
@@ -5076,14 +5080,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.679.g6767b8d81c-goog

[PATCH 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.

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 | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 621b0a42f2a1..c6cac893cbad 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5184,10 +5184,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;
@@ -5199,10 +5195,21 @@ 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 roots
+		 * to be invalidated after the initial MMU reload.  WARN, but
+		 * continue and re-reload the MMU to try and keep the VM alive.
+		 */
+		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.679.g6767b8d81c-goog

[PATCH 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, as retry is uncommon, 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
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 c6cac893cbad..379f570ef04f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5184,10 +5184,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;
@@ -5199,6 +5195,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.679.g6767b8d81c-goog

[PATCH 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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 379f570ef04f..76e3cd717324 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5210,6 +5210,16 @@ 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))
+			continue;
+
 		r = kvm_tdp_mmu_map(vcpu, &fault);
 	} while (r == RET_PF_RETRY);
 
-- 
2.55.0.679.g6767b8d81c-goog

Re: [PATCH 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 2 potential issue(s) to consider:
- [High] The `continue` statement executed on a stale page fault inadvertently terminates the retry loop, causing a fatal `-EIO` failure instead of retrying.
- [High] The KVM MMU invalidation retry protocol order is violated by capturing the sequence after acquiring `mmu_lock` and after the PFN is resolved.
--

--- Patch [4]: [PATCH 4/4] KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN" ---
commit 8ecbe739b111e2504c32794278902d5137202a16
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"
    
This commit hardens the "map private PFN" flow against stale page faults prior
to mapping the PFN into the guest. It adds a sanity check using a sequence
counter snapshot taken under the mmu_lock.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 379f570ef04f8..76e3cd717324b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5210,6 +5210,16 @@ 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;

[Severity: High]
Does capturing the invalidation sequence after acquiring mmu_lock violate
the KVM MMU invalidation retry protocol order?

The sequence is explicitly captured inside the mmu_lock critical section
after the PFN has already been resolved by the caller. This bypasses the
mandatory generation safety sequence (capture sequence, resolve PFN, acquire
lock).

Additionally, could this still cause false positives since
mmu_invalidate_retry_gfn() checks mmu_invalidate_in_progress?

> +		if (is_page_fault_stale(vcpu, &fault))
> +			continue;

[Severity: High]
Does this continue statement inadvertently exit the loop without retrying?

In C, a continue statement within a do-while loop jumps to the while
condition evaluation, not the beginning of the loop block. If r is not
RET_PF_RETRY when this continue is hit, the condition r == RET_PF_RETRY
will evaluate to false.

Will this cause the function to exit the loop prematurely and return -EIO?

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

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The WARN_ON_ONCE check for obsolete roots in kvm_tdp_mmu_map_private_pfn is largely ineffective due to being placed after kvm_mmu_reload, which clears the request bit.
--

--- Patch [3]: [PATCH 3/4] KVM: x86/mmu: Top-up memory caches when retrying "map private PFN" ---
commit 7a1e8c6872a95084f82e79ad3438f87300a97a91
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Top-up memory caches when retrying "map private PFN"

This commit moves the top-up of memory caches into the retry loop in
kvm_tdp_mmu_map_private_pfn(). This ensures that the caches are replenished
on every attempt to map the PFN, hardening KVM against potential cache
consumption issues during TDX post-populate callbacks.

Closes: https://lore.kernel.org/all/20260718061050.E17B01F000E9@smtp.kernel.org

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index c6cac893cbad8..379f570ef04f8 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5184,10 +5184,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;

[ ... ]

> @@ -5199,6 +5195,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);

[Severity: Medium]
In kvm_tdp_mmu_map_private_pfn(), is the WARN_ON_ONCE() check for
obsolete roots still effective here?

Just a few lines down from this chunk:

    WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));

Because kvm_mmu_reload() is called higher up in the loop, which internally
invokes kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu), it will
clear the request bit.

If an invalidation occurs during the kvm_tdp_mmu_map() execution, the loop
correctly retries, but the next iteration's kvm_mmu_reload() will silently
clear the request bit before execution reaches the WARN_ON_ONCE() statement.

This seems to leave the assertion only covering the small window between the
reload and the WARN itself, masking potential regressions where roots are
unexpectedly invalidated during the map operation.

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

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

[Sean Christopherson]

On Thu, Aug 06, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index c6cac893cbad8..379f570ef04f8 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -5184,10 +5184,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;
> 
> [ ... ]
> 
> > @@ -5199,6 +5195,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);
> 
> [Severity: Medium]
> In kvm_tdp_mmu_map_private_pfn(), is the WARN_ON_ONCE() check for
> obsolete roots still effective here?
> 
> Just a few lines down from this chunk:
> 
>     WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> 
> Because kvm_mmu_reload() is called higher up in the loop, which internally
> invokes kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu), it will
> clear the request bit.
> 
> If an invalidation occurs during the kvm_tdp_mmu_map() execution, the loop
> correctly retries, but the next iteration's kvm_mmu_reload() will silently
> clear the request bit before execution reaches the WARN_ON_ONCE() statement.
> 
> This seems to leave the assertion only covering the small window between the
> reload and the WARN itself, masking potential regressions where roots are
> unexpectedly invalidated during the map operation.

Yep.  It's honestly more for documentation than it is for actually catching bugs.
Though I do think that if a bug were introduced, there would actually be a decent
chance of catching it because KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is set while holding
mmu_lock for write.

Moving the WARN up isn't an option, because it could get a false positive on the
first iteration.  And the odds of this firing are stupidly low, i.e. it's not
worth reworking the code to skip the WARN on the first iteration.

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

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

[Sean Christopherson]

On Thu, Aug 06, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 379f570ef04f8..76e3cd717324b 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -5210,6 +5210,16 @@ 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;
> 
> [Severity: High]
> Does capturing the invalidation sequence after acquiring mmu_lock violate
> the KVM MMU invalidation retry protocol order?

Yes, see the comment and changelog.

> The sequence is explicitly captured inside the mmu_lock critical section
> after the PFN has already been resolved by the caller. This bypasses the
> mandatory generation safety sequence (capture sequence, resolve PFN, acquire
> lock).
> 
> Additionally, could this still cause false positives since
> mmu_invalidate_retry_gfn() checks mmu_invalidate_in_progress?

Yes, it's possible, but it would require zappings GFNs on either side of the GFN
being mapped, such that the target GFN is contained by mmu_invalidate_range_{start,end}.
The odds of that happening in practice are low enough that I'm comfortable eating
false positives even though strictly speaking they _should_ be irrelevant.

> > +		if (is_page_fault_stale(vcpu, &fault))
> > +			continue;
> 
> [Severity: High]
> Does this continue statement inadvertently exit the loop without retrying?
> 
> In C, a continue statement within a do-while loop jumps to the while
> condition evaluation, not the beginning of the loop block. If r is not
> RET_PF_RETRY when this continue is hit, the condition r == RET_PF_RETRY
> will evaluate to false.
> 
> Will this cause the function to exit the loop prematurely and return -EIO?

Doh.  And this is why I'm more than happy to deal with Sashiko's paranoia :-)

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Moving `kvm_mmu_reload()` inside the retry loop after `mmu_topup_memory_caches()` causes deterministic memory cache underflows on the first page fault if roots are reloaded.
- [Medium] Failure to re-reload the MMU upon detecting an obsolete root.

Pre-existing issues:
- [High] Memory cache depletion during the `RET_PF_RETRY` loop due to missing `mmu_topup_memory_caches()` inside the loop.
--

--- Patch [2]: [PATCH 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation ---
commit 22d402057a7ea26b753baa57f8845602fe5a1b81
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 if the root is invalidated after the initial reload.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 621b0a42f2a14..c6cac893cbad8 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c

[ ... ]

> @@ -5184,10 +5184,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;

[Severity: High]
Since mmu_topup_memory_caches() is outside the loop, but kvm_mmu_reload()
was moved inside, is it possible for the reload to consume cache items
when allocating new roots?

If the root is invalid and kvm_mmu_load() allocates new roots, this
consumes items from mmu_page_header_cache and mmu_shadow_page_cache. When
execution proceeds to kvm_tdp_mmu_map() expecting fully topped up
caches, could this lead to a cache underflow and a host warning or panic?

> @@ -5199,10 +5195,21 @@ 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 roots
> +		 * to be invalidated after the initial MMU reload.  WARN, but
> +		 * continue and re-reload the MMU to try and keep the VM alive.
> +		 */
> +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> +

[Severity: Medium]
The comment mentions "continue and re-reload the MMU", but does this actually
fall through directly to kvm_tdp_mmu_map() with an obsolete root?

If the mapping succeeds, it might return RET_PF_FIXED. This would exit the
loop instead of forcing a retry. Should there be a continue statement after
the WARN_ON_ONCE to enforce the retry behavior described in the comment?

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

[Severity: High]
This isn't a bug introduced by this patch, but retrying on RET_PF_RETRY
in this loop seems vulnerable to memory cache depletion without the cache topup.

If kvm_tdp_mmu_map() encounters contention, frees its locally allocated shadow
pages, and returns RET_PF_RETRY, the loop repeats without calling
mmu_topup_memory_caches() again. Could this underflow the cache during
repeated retries under high contention?

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

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

[Sean Christopherson]

On Thu, Aug 06, 2026, sashiko-bot@kernel.org wrote:
> @@ -5184,10 +5184,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;
> 
> [Severity: High]
> Since mmu_topup_memory_caches() is outside the loop, but kvm_mmu_reload()
> was moved inside, is it possible for the reload to consume cache items
> when allocating new roots?
> 
> If the root is invalid and kvm_mmu_load() allocates new roots, this
> consumes items from mmu_page_header_cache and mmu_shadow_page_cache. When
> execution proceeds to kvm_tdp_mmu_map() expecting fully topped up
> caches, could this lead to a cache underflow and a host warning or panic?

Literally the next patch...

> > @@ -5199,10 +5195,21 @@ 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 roots
> > +		 * to be invalidated after the initial MMU reload.  WARN, but
> > +		 * continue and re-reload the MMU to try and keep the VM alive.
> > +		 */
> > +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> > +
> 
> [Severity: Medium]
> The comment mentions "continue and re-reload the MMU", but does this actually
> fall through directly to kvm_tdp_mmu_map() with an obsolete root?

Yes, addressed two patches from now.

> If the mapping succeeds, it might return RET_PF_FIXED. This would exit the
> loop instead of forcing a retry. Should there be a continue statement after
> the WARN_ON_ONCE to enforce the retry behavior described in the comment?

Eh, I'd rather hedge in the changelog.

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

[Huang, Kai]

On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> 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()")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> 

Reviewed-by: Kai Huang <kai.huang@intel.com>

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

[Edgecombe, Rick P]

On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> 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()")
> Signed-off-by: Sean Christopherson <seanjc@google.com>


Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

But, did you hit this somehow with an unmodified test and kernel? The log makes
me think it was hit in a normal test run. (which I failed to reproduce)

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

[Sean Christopherson]

On Fri, Aug 07, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> > 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()")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> 
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> 
> But, did you hit this somehow with an unmodified test and kernel? The log makes
> me think it was hit in a normal test run. (which I failed to reproduce)

Yes and yes.  I often test by running all selftests in parallel.  My guess is that
PREEMPT_LAZY plus oversubscribed CPUs allowed the timing condition to be hit.  It
wasn't anywhere near 100% reproducible on my end either, maybe 1 out of every 10
runs?  20 runs?  (of the entire suite of selftests).

P.S. I was very proud of myself: because the failure was so flaky, I used perf
and bpftrace to debug the issue instead of my usual hack-the-kernel approach. :-)

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

[Edgecombe, Rick P]

On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> @@ -5199,10 +5195,21 @@ 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 roots
> +		 * to be invalidated after the initial MMU reload.  WARN, but
> +		 * continue and re-reload the MMU to try and keep the VM alive.
> +		 */

It might be a little confusing when no MMU reload follows the comment. Maybe
mention that the MMU reload is above? Either way ok to me.

> +		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> +
>  		r = kvm_tdp_mmu_map(vcpu, &fault);

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

[Edgecombe, Rick P]

On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> 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.

I think it is the same for the other caches consumed by the fault. I guess
"e.g." covers it. But it's not new after DPAMT.

> 
> Harden KVM even though consuming an entry on failure is considered a KVM
> bug, as retry is uncommon
> 

The locks held by the sole call path will prevent retries from being needed due
to TDX-specific details. So in the place where this code lives, it is a bug. But
can't really be hit. To me "retry is uncommon" sounds like it's a rare case that
is hittable. I guess you mean only in the uncommon case of bugs.

> , 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
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---

Thanks! And...

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

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

[Sean Christopherson]

On Fri, Aug 07, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> > 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.
> 
> I think it is the same for the other caches consumed by the fault. I guess
> "e.g." covers it. But it's not new after DPAMT.

I don't think so?  Especially since as you point out below, nothing else can
muck with the SPTEs.  The TDP MMU only consumes an cache entry if it successfully
creates a SPTE, and since nothing can muck with SPTEs, anything created on the
first attempt will still be there on subsequent attempts.  I.e. the TDP MMU might
create SPTEs that are ultimately unused, but I don't think it can exhaust a cache.

> > Harden KVM even though consuming an entry on failure is considered a KVM
> > bug, as retry is uncommon
> > 
> 
> The locks held by the sole call path will prevent retries from being needed due
> to TDX-specific details. So in the place where this code lives, it is a bug. But
> can't really be hit. To me "retry is uncommon" sounds like it's a rare case that
> is hittable. I guess you mean only in the uncommon case of bugs.

Ah, I was thinking a different task could pre-fault memory, but pre-fault isn't
allowed until the VM is TD_STATE_RUNNABLE, and KVM_TDX_INIT_MEM_REGION is only
usable if the VM is *not* TD_STATE_RUNNABLE.

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

[Sean Christopherson]

On Fri, Aug 07, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-08-06 at 14:40 -0700, Sean Christopherson wrote:
> > @@ -5199,10 +5195,21 @@ 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 roots
> > +		 * to be invalidated after the initial MMU reload.  WARN, but
> > +		 * continue and re-reload the MMU to try and keep the VM alive.
> > +		 */
> 
> It might be a little confusing when no MMU reload follows the comment. Maybe
> mention that the MMU reload is above? Either way ok to me.

Yeah, I don't love the comment either.  The subtlety that it doesn't capture is
that KVM_REQ_MMU_FREE_OBSOLETE_ROOTS doesn't actually mean the current root is
invalid, just that it might be invalid and the vCPU needs to check.

How about this?  

		/*
		 * 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);
>