Re: [PATCH v10 11/41] KVM: guest_memfd: Ensure pages are not in use before conversion

[Sean Christopherson <seanjc@google.com>]

On Mon, Aug 17, 2026, Yan Zhao wrote:
> On Thu, Aug 13, 2026 at 04:20:05PM -0700, Sean Christopherson wrote:
> > On Thu, Aug 13, 2026, Rick P Edgecombe wrote:
> > > > > 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?

Ah, sorry, my bad.  I was speed reading and missed that the key to your suggested
"*page = NULL" change was that the reference was put _before_
filemap_invalidate_unlock_shared(), i.e. before dropping
the invalidate lock and thus before __kvm_gmem_set_attributes() will walk the
folios to look for outstanding references.  I was thinking that putting the
reference right away was just shrinking the timing window, but putting the
reference while still holding the invalidate lock closes the window entirely.

So, I take back what I said about this not being ABI, and about this not blocking
in-place conversion.  It most definitely affects ABI, and so needs to be addressed
before merging in-place conversion.  The only question is if we want to commit to
guaranteeing that conversion will succeed in this scenario, or if we want to take
the easy way out and formally document that conversion can fail with EAGAIN at any
time, even if userspace has never mmap()'d the memory in question.

I'm leaning pretty strongly towards guaranteeing conversion will succeed.  We'll
still need to document the EAGAIN behavior, but IMO there's a massive difference
between conversion failing if there's a lingering reference acquired via a VMA,
conversion failing because a vCPU page fault raced with conversion.  E.g. being
able to assert success in a very curated test, as the stress test presumably does,
would be extremely valuable for helping detect/prevent edge case bugs.

The argument against guaranteeing success is that we might make our future lives
harder, e.g. if it turns out there are legitimate, hard-to-solve edge cases.  But
I'm ok with that risk, as it seems highly unlikely to be problematic in practice,
and there is real benefit to guaranteeing success.

> > As for in-place conversion, this is not a blocker.
> Sorry. I didn't intend to block in-place conversion.

LOL, what we intend and what happens aren't always the same. :-)

> I encountered this issue during testing, so reported it.

Thanks for doing so!  I'd *much* rather sort these issues out *before* merging
code, even if it means delaying the merge by a bit.