Re: [PATCH v5 7/7] KVM: guest_memfd: Rework PREPARE config and hook into a more generic CONVERT

[Xiaoyao Li]

On 7/22/2026 12:24 AM, Sean Christopherson wrote:
> On Fri, Jul 17, 2026, Xiaoyao Li wrote:
> > On 7/17/2026 5:42 AM, Ackerley Tng wrote:
> > > Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes:
> > > @@ -798,7 +798,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct
> > > kvm_memory_slot *slot,
> > >    		folio_mark_uptodate(folio);
> > >    	}
> > >
> > > -	r = kvm_gmem_make_private(kvm, slot, gfn, folio);
> > > +	r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> > >
> > >    	folio_unlock(folio);
> > >
> > > I'll move just this renaming to [1] like you suggested.
> > >
> > > I think it's okay to continue to always call prepare_folio(), and within
> > > the prepare_folio() function, only do conversion when the CONVERT CONFIG
> > > is defined.
> >
> > I don't think so.
> >
> > This patch not only renames kvm_arch_gmem_prepare() to
> > kvm_arch_gmem_convert(), but also adds one more parameter
> >
> >    'bool to_private'
> >
> > and hardcodes the new parameter to true. This mean the arch callback will
> > convert the folio to private unconditionally in kvm_prepare_folio() when
> > CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT is enabled.
> >
> > Note the CONFIG is not a per-VM thing but a build time thing. The
> > unconditionally-converting-to-private semantic can also be applied to
> > gmem-only memslot for non-Coco VMs whenever the HAVE_KVM_ARCH_GMEM_CONVERT
> > is enabled when building the kernel.
> >
> > Though the code won't do anything for gmem-only memslot for non-Coco VMs,
> > the literal semantic of the function and parameter is wrong.
>
> Agreed.  How about I add a patch to guard the call with:
>
> 	if (kvm_arch_has_private_mem(kvm) &&
> 	    !(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED))
> 		r = kvm_gmem_make_private(kvm, slot, gfn, folio);
>
> which is semantically correct pre-in-place conversion.  And then when in-place
> conversion comes along, that will get switched to:
>
> 	if (kvm_gmem_is_private_mem(inode, index))
> 		r = kvm_gmem_make_private(kvm, slot, gfn, folio);
>
> Which IMO yields a very clean and intuitive diff.
>
> Actually, even better would be to insert a patch to provide kvm_gmem_is_private_mem()
> and kvm_gmem_is_shared_mem() as part of this prep, and pull in "KVM: guest_memfd:
> Only prepare folios for private pages" with a massaged shortlog+changelog.

yeah. This looks good!

> I.e. have this at the end of this prep work:
>
> static bool kvm_gmem_is_private_mem(struct inode *inode, pgoff_t index)
> {
> 	return kvm_arch_has_private_mem(kvm) &&
> 	       !(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED);
> }

I'm wondering if we really need to check kvm_arch_has_private_mem() 
here. It looks checking GUEST_MEMFD_FLAG_INIT_SHARED is sufficient.

> static bool kvm_gmem_is_shared_mem(struct inode *inode, pgoff_t index)
> {
> 	return !kvm_gmem_is_private_mem(inode, index);
> }
>
> 	if (!kvm_gmem_is_shared_mem(inode, vmf->pgoff))
> 		return VM_FAULT_SIGBUS;
>
> 	if (kvm_gmem_is_private_mem(inode, index))
> 		r = kvm_gmem_make_private(kvm, slot, gfn, folio);
>
> 	
> And then "KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings"
> isn't changing the semantics of the callers, it's only changing the internal
> plumbing for PRIVATE vs. SHARED.