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

On Mon, Aug 17, 2026, Yan Zhao wrote:

On Thu, Aug 13, 2026 at 04:20:05PM -0700, Sean Christopherson wrote:

quoted

On Thu, Aug 13, 2026, Rick P Edgecombe wrote:

quoted

quoted

quoted

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.

quoted

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.

quoted

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.