On Thu, Aug 13, 2026 at 04:20:05PM -0700, Sean Christopherson wrote:
> On Thu, Aug 13, 2026, Rick P Edgecombe wrote:
> > On Thu, 2026-08-13 at 11:51 -0700, Ackerley Tng wrote:
> > > "Edgecombe, Rick P" <[email protected]> writes:
> > >
> > > > On Tue, 2026-08-11 at 10:35 -0700, Ackerley Tng wrote:
> > > > > > > Would like to see what Sean thinks of this. Either way, is it
> > > > > > > okay to
> > > > > > > follow up after conversions lands?
> > > > > > Let's see what Sean thinks of this :)
> > > > > > I raised this because the issue was encountered by one TDX's stress
> > > > > > selftest.
> > > > >
> > > > > Which stress selftest is this? I can try running this on my side too.
> > > >
> > > > We have some selftests that are built on the basic TDX selftests. One
> > > > just
> > > > hammers the MMU stuff with a bunch of zaps and also weird stuff from
> > > > the guest.
> > > > It was eventually too much work to try to keep the internal
> > > > enhancements rebased
> > >
> > > Would like all the comments we can get on TDX selftests v14 [1]!
> >
> > I think we had a few. Let me try to round up some more folks.
> >
> > >
> > > > nicely so we actually just run an old branch's TDX selftests against
> > > > newer
> > > > kernels. So the branch is a bit of a pile, and not really suitable for
> > > > sharing.
> > > > We plan to clean it and upstream it when the path clears. So it would
> > > > really
> > > > help to get those basic ones upstream. We remain happy to help, so
> > > > please let us
> > > > know.
> > >
> > > I guess at this point I'm hoping y'all and Sean are okay that this
> > > conversions series merges, and we let this stress test failure be
> > > handled later. I'll be around to fix things :)
> > >
> > > I'd say the line of sight to fixing this would be when the KVM MMU only
> > > gets PFNs (and no pages at all) from guest_memfd.
> >
> > Hmm, I think we shouldn't upstream a uABI that we don't have line of sight
> > to
> > making robust. So it would be good to settle this thread at least.
>
> This isn't uABI. You're talking about hitting a race condition between one
> task
Hmm. Perhaps it is not a uABI issue, since users are allowed to retry. However,
it is hard to convince me that it makes sense to require users to retry a
private-to-shared conversion before a GFN has ever been mapped, given that a
retry is not required when the GFN is currently in use by the guest.
> converting a page and another faulting in the same page. An NMI, SMI, or IRQ
> at
> just the right/wrong time, especially on a preemptible kernel, could lead to
> the
> same test failures, even if KVM drops the refcount "immediately".
Could you elaborate on how an NMI, SMI, or IRQ at just the right/wrong time
could lead to the same test failures?
Do you mean they can cause a fault to be retried? Our test failure is an EAGAIN
returned from a private-to-shared conversion before the page has even been
mapped as private.
> That said, I am 100% in favor of not handing the caller a struct page. Now
> that
> the TDX APIs no longer require one, it's more than feasible. But, we
> absolutely
> shouldn't just nullify the pointer, we should drop the param entirely. Not
> just
> because it's cleaner, but because it also forces an audit of the callers to
> see
> if they subtly require a refcount (spoiler alert).
Yeah, I also considered dropping the param entirely and was terrified by the
lines of changes :)
If you are in favor of not handing the caller a struct page, the following
changes should also be required on top of your change.
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 8ef16ccf26ce..d5aa197d2cbf 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1613,7 +1613,6 @@ static int gmem_abort(const struct kvm_s2_fault_desc
*s2fd)
enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
struct kvm_pgtable *pgt = s2fd->vcpu->arch.hw_mmu->pgt;
unsigned long mmu_seq;
- struct page *page;
struct kvm *kvm = s2fd->vcpu->kvm;
void *memcache = NULL;
kvm_pfn_t pfn;
@@ -1681,7 +1680,6 @@ static int gmem_abort(const struct kvm_s2_fault_desc
*s2fd)
}
out_unlock:
- kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_W);
kvm_fault_unlock(kvm);
if ((prot & KVM_PGTABLE_PROT_W) && !ret)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index c982a6454fc9..43523bb17621 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1360,7 +1360,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool
*is_gmem)
bool write_fault, writable;
unsigned long mmu_seq;
struct vncr_tlb *vt;
- struct page *page;
+ struct page *page = NULL;
u64 va, pfn, gfn;
int ret;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3d3eb8387cd0..c5ba2c8cad74 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4017,7 +4017,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu,
gpa_t gpa)
struct kvm *kvm = vcpu->kvm;
gfn_t gfn = gpa_to_gfn(gpa);
unsigned long mmu_seq;
- struct page *page;
kvm_pfn_t pfn;
lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
@@ -4077,8 +4076,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu,
gpa_t gpa)
else
svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
read_unlock(&kvm->mmu_lock);
-
- kvm_release_page_clean(page);
}
/*
> The lone holdout at this point is sev_handle_rmp_fault(), which could end up
> PSMASH-ing a PFN that has since been freed by KVM. Assuming holding mmu_lock
> while doing RMP operations is ok, something like the below? Completely
> untested.
Tested successfully after applying the above fix and the typo correction.
@@ -5073,7 +5070,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t
gpa, u64 error_code)
if (rmp_level == PG_LEVEL_4K)
goto out;
- scoped_guard(read_lock)(&kvm->mmu_lock) {
+ scoped_guard(read_lock, &kvm->mmu_lock) {
if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
goto out;
> As for in-place conversion, this is not a blocker.
Sorry. I didn't intend to block in-place conversion.
I encountered this issue during testing, so reported it.
