Re: [PATCH v11 15/46] KVM: guest_memfd: Call arch make_shared callback for to-shared conversion

[Sean Christopherson]

On Thu, Aug 27, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@xxxxxxxxxx> writes:
> 
> > On Wed, Aug 26, 2026, Ackerley Tng wrote:
> >> Omit support for calling the arch hook to make private, since SNP, the only
> >> implementer of the arch make-private hook today, would actually prefer
> >> making private only just before faulting memory into the NPTs.
> >>
> >> Calling the make-private arch hook would require iterating both bindings
> >> and the filemap to find the intersection of bindings and allocated
> >> folios.
> >
> > Why would KVM need to iterate over the bindings?  Only the RMP needs to be updated,
> > whether or not the RMP is currently reachable is irrelevant, no?
> >
> > Subsequent calls to kvm_arch_gmem_make_private() from kvm_gmem_get_pfn() would be
> > superfluous, but that's already possible, e.g. if an NPT mappings is removed for
> > whatever reason.
> >
> 
> make_private takes a gfn and pfn, so __kvm_gmem_set_attributes() would
> need to iterate bindings to get gfns and filemap to find folios to get
> pfns.

Oooh, right, unassigned a page in the RMP only needs the PFN, but assigned a page
needs the ASID and GFN.

> > I don't care terribly about how SNP handles this, but I do want accurate reasoning
> > and justification so that if/when we revisit any of this in the future, we can make
> > informed decisions.  Because unless I'm missing something, this is an optimization
> > choice (eager vs. lazy to-private conversions), not a complexity tradeoff, and it's
> > not clear to me how we decided the lazy approach would provide better performance.
> >
> 
> How's this, to replace the entire commit message? I hope it captures
> points from this discussion:
> 
>   When memory in guest_memfd is converted from private to shared, the
>   platform-specific state associated with the guest-private pages must
>   be invalidated or cleaned up.
> 
>   Iterate over the folios in the affected range and call the
>   kvm_arch_gmem_make_shared() hook for each PFN range. This allows
>   architectures to update hardware metadata or encryption states to
>   transition pages to the shared state, instead of leaving hardware
>   state as private while guest_memfd tracks it as shared. Transitioning
>   hardware state ensures that guest_memfd upholds the guarantee that
>   userspace only maps shared memory.

Nit, don't reference functions by name when it's easy-ish to avoid doing so.  And
don't give a play-by-play: the patch makes it pretty obvious the code is iterating
over folios, what isn't obvious is *why* the code does that.

>   Invoke this helper after indicating to KVM's mmu code that an
>   invalidation is in progress to stop in-flight page faults from
>   succeeding. Calling the invalidation helper also calls the arch
>   invalidate hook. For SNP, this kicks any vCPU with a registered VMSA
>   within the range being converted out of the guest. This ensures that
>   make_shared never fails due to the VMSA page being in-use and is
>   important because if make_shared fails, the RMP table would track the
>   page as private while guest_memfd is unaware and tracks the page as
>   shared.

Exaclty what SNP does isn't relevant.  Or rather, it's but on example of how this
needs to work.  I.e. the conversion needs to happen within the invalidtion sequence
because thems the rules for KVM.

>   Omit support for calling the arch hook to make private during
>   to-private conversions. Making private lazily at fault time aligns
>   with how it works on other platforms like TDX.

To me, this isn't a valid argument.  We've fully committed to relying on vendor
specific behavior, and SNP can't truly work like TDX because the underlying
implementations are so different.

>   Furthermore, making pages shared only requires PFNs, which are
>   obtained by iterating folios in the filemap. In contrast, making pages
>   private in the RMP also requires the GFN, which would require
>   iterating bindings to get GFNs and the filemap to get PFNs from
>   allocated folios. Deferring the transition to fault time avoids this
>   additional complexity.

It's not just complexity, it's that the bindings might not even exist.  I.e. for
all intents and purposes, doing on-demand updates is mandatory, because that's the
only time a relevant memslot binding is guaranteed to exist.

All in all, this?

   When doing in-place conversion from PRIVATE to SHARED, immediately inform
   arch code of the conversion for all allocated pages/folios, e.g. so that
   arch code can put hardware metadata tables in the correct state.  Eagerly
   updating the table for to SHARED conversions avoids having to implement
   on-demand updates, e.g. when faulting in host userspace mappings.  Skip
   the entire flow if the arch doesn't implement conversion callbacks, as
   getting folios from the filemap is noticeably expensive, especially when
   converting large chunks of memory.

   Deliberately don't eagerly update the metadata table on conversions from
   SHARED to PRIVATE, because assigning a page to a VM (versus "returning" it
   to the host) requires the exact GFN associated with the page, i.e would
   require walking the memslot bindings.  And because KVM *must* do on-demand
   metadata updates when getting a PFN for KVM-internal usage, as that's the
   only time a relevant memslot binding is guaranteed to exist.

   Note!  Inform arch code of the conversion within the protection of the
   invalidation sequence, to ensure that any existing mappings are dropped
   before hardware is updated, and to ensure that new mappings can't be
   established until after the conversion is complete.