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

[Xiaoyao Li <xiaoyao.li@intel.com>]

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@intel.com> 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.