Re: [PATCH v2 5/5] KVM: guest_memfd: GUP source pages prior to populating guest memory

[Huang, Kai]

On Mon, 2025-12-15 at 09:34 -0600, Michael Roth wrote:
> Currently the post-populate callbacks handle copying source pages into
> private GPA ranges backed by guest_memfd, where kvm_gmem_populate()
> acquires the filemap invalidate lock, then calls a post-populate
> callback which may issue a get_user_pages() on the source pages prior to
> copying them into the private GPA (e.g. TDX).
> 
> This will not be compatible with in-place conversion, where the
> userspace page fault path will attempt to acquire filemap invalidate
> lock while holding the mm->mmap_lock, leading to a potential ABBA
> deadlock[1].

Nit: there's no link to mention [1].


[...]

> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Co-developed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Co-developed-by: Vishal Annapurve <vannapurve@xxxxxxxxxx>
> Signed-off-by: Vishal Annapurve <vannapurve@xxxxxxxxxx>
> Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
> 

[...]

> +	if (src_page) {
> +		void *src_vaddr = kmap_local_pfn(page_to_pfn(src_page));

Nit: maybe you can use kmap_local_page(src_page) directly.

> +		void *dst_vaddr = kmap_local_pfn(pfn);
>  
> -		if (copy_from_user(vaddr, src, PAGE_SIZE)) {
> -			ret = -EFAULT;
> -			goto out;
> -		}
> -		kunmap_local(vaddr);
> +		memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
> +
> +		kunmap_local(src_vaddr);
> +		kunmap_local(dst_vaddr);
>  	}
>  
>  	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> @@ -2325,17 +2325,19 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  	if (ret && !snp_page_reclaim(kvm, pfn) &&
>  	    sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
>  	    sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
> -		void *vaddr = kmap_local_pfn(pfn);
> +		void *src_vaddr = kmap_local_pfn(page_to_pfn(src_page));

Ditto.