Re: [PATCH v5 4/7] KVM: guest_memfd: Fold __kvm_gmem_prepare_folio() into its sole caller

[Sean Christopherson <seanjc@google.com>]

+Mike

On Tue, Jul 21, 2026, Sean Christopherson wrote:
> On Tue, Jul 14, 2026, Ackerley Tng wrote:
> > Sean Christopherson <seanjc@google.com> writes:
> > 
> > >
> > > [...snip...]
> > >
> > > @@ -97,11 +85,15 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
> > >  	 * The order will be passed when creating the guest_memfd, and
> > >  	 * checked when creating memslots.
> > >  	 */
> > > -	WARN_ON(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
> > > +	WARN_ON_ONCE(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
> > > +	gfn = ALIGN_DOWN(gfn, folio_nr_pages(folio));
> > >  	index = kvm_gmem_get_index(slot, gfn);
> > > -	index = ALIGN_DOWN(index, folio_nr_pages(folio));
> > >
> > > -	return __kvm_gmem_prepare_folio(kvm, slot, index, folio);
> > > +	return kvm_arch_gmem_prepare(kvm, gfn, folio_file_pfn(folio, index),
> > 
> > Could this just be folio_pfn(folio) since this function is
> > kvm_gmem_prepare_folio() and guest_memfd will always try to prepare the
> > entire folio?
> 
> No?  Maybe?  For this patch, I'm just trying to shuffle code araound.  Even for
> this series, I'd prefer not to make any more semantic changes than are needed to
> get to a sane state, and to prepare for in-place conversion.  If there's a need
> and/or a good reason to use folio_pfn(), by all means, send a patch.

I take that back.  After far too much staring and digging into the history (and
future) of this code, I finally see what you were getting at.  Because KVM
"prepares" entire folios, grabbing a specific page in the folio is nonsensical,
just grab the base pfn and be done with it.

However, I'm not convinced to committing harder to per-folio "preparation" is the
way to go.  The pre-folio prepation was added when guest_memfd actually tracked
preparedness.  At the time, it made sense, because tracking per-folio meant we
didn't need to add a separate data structure to track that information, and doing
per-folio tracking only works if the entire folio is prepared (or not).

Now that we dropped that code in favor having SNP query the RMP, per-folio
preparation, i.e. per-folio conversions to private, doesn't make as much sense.
There is still technically an argument for per-folio conversion, *if* what I
described below is even allowed by SNP.

If SNP allows the RMP size to be greater than the NPT size, then per-folio
conversion would allow KVM to assign a hugepage in the RMP even if it can only
be mapped into the NPT with a smaller page, e.g. because of memslot alignment.
The documentation of that reasoning would be something like this:

	/* 
	 * If the memory is private from KVM's perspective, and hardware tracks
	 * VM-assigned private memory in a dedicated data structure, i.e. not
	 * in the stage-2 page tables, then call into arch code to assign the
	 * entire folio to the guest.  Assigning the entire folio, e.g. instead
	 * of only the memory being mapped into the guest, allows KVM to assign
	 * an entire hugepage of memory in the out-of-band structure even if
	 * KVM can only map a smaller page size into the MMU, e.g. because the
	 * gmem hugepage is spread across multiple memslots.
	 */
	if (kvm_gmem_is_private_mem(file_inode(file), index))
		r = kvm_gmem_make_private(kvm, slot, gfn, folio);

But it's not clear to me that SNP even supports mismatched page sizes, because
several of the flows in the APM explicitly state that triggers an #NPF, and this
comment in KVM's sev_handle_rmp_fault() also suggests that's not allow.

	 * 2) Guest access via NPT can trigger an #NPF if the NPT mapping is
	 *    smaller than what is indicated by the 2MB RMP entry for the PFN
	 *    that backs the GPA.

And even *if* that's allowed by SNP, I'm not at all convinced it's worth doing
in KVM, because it should be a rare situation.  E.g. maybe for memory at the top
of TOLUD that has holes for non-RAM crud or something?

So instead of using folio_pfn(), I'm leaning strongly towards entirely dropping
what is currently kvm_gmem_prepare_folio(), and instead converting exactly what
is being mapped into the guest.  Because I've messed up the gfn and index handling
twice now, I'm still struggling to wrap my head around the math, *and* I really
don't want to have two separate flows computing "how much memory can be assigned
and at what size" once hugepage support comes along.

I.e. do this:

	pgoff_t index = kvm_gmem_get_index(slot, gfn);
	struct folio *folio;
	int r = 0, __order;

	max_order = max_order ?: &__order;

	CLASS(gmem_get_file, file)(slot);
	if (!file)
		return -EFAULT;

	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
	if (IS_ERR(folio))
		return PTR_ERR(folio);

	if (!folio_test_uptodate(folio)) {
		clear_highpage(folio_page(folio, 0));
		folio_mark_uptodate(folio);
	}

	if (kvm_gmem_is_private_mem(file_inode(file), index))
		r = kvm_arch_gmem_make_private(kvm, gfn, *pfn, *max_order);


Am I missing something?  Anyone feel strongly about keeping per-folio converions
for when SNP supports hugepages?