Re: [PATCH v4 18/18] KVM: guest_memfd: Combine .gmem_prepare()+.gmem_invalidate() into .gmem_convert()

[Fuad Tabba]

On Mon, 13 Jul 2026 at 16:41, Ackerley Tng <ackerleytng@xxxxxxxxxx> wrote:
>
> Fuad Tabba <fuad.tabba@xxxxxxxxx> writes:
>
> >
> > [...snip...]
> >
>
> Thanks for bringing [1] up, I didn't make the link when this change was
> being discussed.
>
> pKVM definitely needs a callback when the folio is released from
> guest_memfd's ownership.
>
> >
> > free_folio is the only to_private=false caller today, but the cover
> > letter sketches the other one, a private->shared conversion through
> > kvm_gmem_make_shared(inode, start, end). Once that lands both reach
> > the hook with to_private=false, and as far as I can tell, the only
> > thing separating them is kvm == NULL / INVALID_GPA. That isn't
>
> On the folio teardown path kvm has to be non-NULL so that SNP can find
> out which VM to unassign from, so you're right that INVALID_GPA won't be
> sufficient to separate conversion to private and teardown, for pKVM,
> from the arch perspective.
>
> > documented as a contract, and nothing in-tree reads it on that branch
> > (the kvm/gpa check is under if (to_private), and
> > sev_gmem_make_shared() doesn't take a kvm at all), so nothing would
> > catch it regressing. The sketch is also inode-scoped, so I'm not sure
> > the conversion path passes a non-NULL kvm at all; if it doesn't, the
> > two aren't distinguishable.
> >
> >>
> >> [...snip...]
> >>
> >
> > The Kconfig now couples them too: an arch can't take teardown without also
> > taking conversion. pKVM wants teardown and not conversion, so that's a problem
> > for us today, not only once the second caller lands.
> >
> > This is the same ground as v8 15/46, which ended with the two split under
> > separate Kconfigs. Suzuki confirmed CCA disables both; pKVM wants teardown only,
> > since our conversions are guest-initiated and EL2 has already done the
> > transition by the time the host runs the generic path [1].
> >
> > I'd rather keep the free path on its own callback. If you want a single hook,
> > passing the caller explicitly instead of leaving it implied by kvm would work
> > too. Merging the two conversion directions is a genuine cleanup, it's the free
> > path I'd like to keep separate.
> >
>
> Is it okay if later, we add a .gmem_teardown callback with a new CONFIG
> flag that pKVM can use?

That works for me, and I agree it's nicer to introduce the callback along
with its first user.

Sean, is that the plan you have in mind as well? Two things that matter for
pKVM when it lands: the new CONFIG needs to be independent of the conversion
one, so that free_folio fires for an arch that selects only teardown (pKVM
wants teardown and not conversion), and the teardown callback stays distinct
from a private->shared conversion. Good point on teardown needing a non-NULL
kvm for SNP, incidentally, that's another reason keying off the sentinel
wouldn't have worked.

If that's where this is heading, I'm happy.

Cheers,
/fuad

>
> If that's okay, this merge (of function names and of the patch series)
> can proceed. It's also nicer that the new callback and CONFIG are
> explicitly (re?)-introduced with pKVM support so the introduction is
> chronologically linked with pKVM support too :)
>
> > Thanks,
> > /fuad
> >
> > [1] https://lore.kernel.org/all/CA+EHjTwLRzrq0vY0-OosENJ2Gkknerux53aVBj0zGRQUkRQP=A@xxxxxxxxxxxxxx/
> >
> >>
> >> [...snip...]
> >>