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

On Thu, Aug 27, 2026, Ackerley Tng wrote:

Sean Christopherson [off-list ref] writes:

quoted

On Wed, Aug 26, 2026, Ackerley Tng wrote:

quoted

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.

quoted

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.