[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aU33qm9wwfdY5301@yzhao56-desk.sh.intel.com>
Date: Fri, 26 Dec 2025 10:49:14 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Michael Roth <michael.roth@....com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-coco@...ts.linux.dev"
<linux-coco@...ts.linux.dev>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"thomas.lendacky@....com" <thomas.lendacky@....com>, "pbonzini@...hat.com"
<pbonzini@...hat.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
"vbabka@...e.cz" <vbabka@...e.cz>, "ashish.kalra@....com"
<ashish.kalra@....com>, "liam.merwick@...cle.com" <liam.merwick@...cle.com>,
"david@...hat.com" <david@...hat.com>, "Annapurve, Vishal"
<vannapurve@...gle.com>, "ackerleytng@...gle.com" <ackerleytng@...gle.com>,
"aik@....com" <aik@....com>, "Weiny, Ira" <ira.weiny@...el.com>
Subject: Re: [PATCH v2 1/5] KVM: guest_memfd: Remove partial hugepage
handling from kvm_gmem_populate()
> -static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pfn,
> - void __user *src, int order, void *opaque)
> +static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> + void __user *src, void *opaque)
> {
> struct sev_gmem_populate_args *sev_populate_args = opaque;
> + struct sev_data_snp_launch_update fw_args = {0};
> struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> - int n_private = 0, ret, i;
> - int npages = (1 << order);
> - gfn_t gfn;
> + bool assigned = false;
> + int level;
> + int ret;
>
> if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src))
> return -EINVAL;
>
> - for (gfn = gfn_start, i = 0; gfn < gfn_start + npages; gfn++, i++) {
> - struct sev_data_snp_launch_update fw_args = {0};
> - bool assigned = false;
> - int level;
> -
> - ret = snp_lookup_rmpentry((u64)pfn + i, &assigned, &level);
> - if (ret || assigned) {
> - pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
> - __func__, gfn, ret, assigned);
> - ret = ret ? -EINVAL : -EEXIST;
> - goto err;
> - }
> + ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
> + if (ret || assigned) {
> + pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
> + __func__, gfn, ret, assigned);
> + ret = ret ? -EINVAL : -EEXIST;
> + goto out;
> + }
>
> - if (src) {
> - void *vaddr = kmap_local_pfn(pfn + i);
> + if (src) {
> + void *vaddr = kmap_local_pfn(pfn);
>
> - if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
> - ret = -EFAULT;
> - goto err;
Please consider apply the following fix before this patch. Thanks!
commit 2714522d42263e0e250f21a0b171c10c4bb17ed3
Author: Yan Zhao <yan.y.zhao@...el.com>
Date: Mon Nov 10 11:22:28 2025 +0800
KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()
sev_gmem_post_populate() needs to unmap the target vaddr after
copy_from_user() to the vaddr fails.
Fixes: dee5a47cc7a4 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
Signed-off-by: Yan Zhao <yan.y.zhao@...el.com>
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..261d9ef8631b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2296,6 +2296,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
void *vaddr = kmap_local_pfn(pfn + i);
if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
+ kunmap_local(vaddr);
ret = -EFAULT;
goto err;
}
> - }
> - kunmap_local(vaddr);
> + if (copy_from_user(vaddr, src, PAGE_SIZE)) {
> + ret = -EFAULT;
> + goto out;
> }
> -
> - ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> - sev_get_asid(kvm), true);
> - if (ret)
> - goto err;
> -
> - n_private++;
> -
> - fw_args.gctx_paddr = __psp_pa(sev->snp_context);
> - fw_args.address = __sme_set(pfn_to_hpa(pfn + i));
> - fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
> - fw_args.page_type = sev_populate_args->type;
> -
> - ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
> - &fw_args, &sev_populate_args->fw_error);
> - if (ret)
> - goto fw_err;
> + kunmap_local(vaddr);
> }
>
> - return 0;
> + ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> + sev_get_asid(kvm), true);
> + if (ret)
> + goto out;
> +
> + fw_args.gctx_paddr = __psp_pa(sev->snp_context);
> + fw_args.address = __sme_set(pfn_to_hpa(pfn));
> + fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
> + fw_args.page_type = sev_populate_args->type;
>
> -fw_err:
> + ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
> + &fw_args, &sev_populate_args->fw_error);
> /*
> * If the firmware command failed handle the reclaim and cleanup of that
> - * PFN specially vs. prior pages which can be cleaned up below without
> - * needing to reclaim in advance.
> + * PFN before reporting an error.
> *
> * Additionally, when invalid CPUID function entries are detected,
> * firmware writes the expected values into the page and leaves it
> @@ -2336,26 +2322,20 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> * information to provide information on which CPUID leaves/fields
> * failed CPUID validation.
> */
> - if (!snp_page_reclaim(kvm, pfn + i) &&
> + 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 + i);
> + void *vaddr = kmap_local_pfn(pfn);
>
> - if (copy_to_user(src + i * PAGE_SIZE, vaddr, PAGE_SIZE))
> + if (copy_to_user(src, vaddr, PAGE_SIZE))
> pr_debug("Failed to write CPUID page back to userspace\n");
>
> kunmap_local(vaddr);
> }
>
> - /* pfn + i is hypervisor-owned now, so skip below cleanup for it. */
> - n_private--;
> -
> -err:
> - pr_debug("%s: exiting with error ret %d (fw_error %d), restoring %d gmem PFNs to shared.\n",
> - __func__, ret, sev_populate_args->fw_error, n_private);
> - for (i = 0; i < n_private; i++)
> - kvm_rmp_make_shared(kvm, pfn + i, PG_LEVEL_4K);
> -
> +out:
> + pr_debug("%s: exiting with return code %d (fw_error %d)\n",
> + __func__, ret, sev_populate_args->fw_error);
> return ret;
> }
Powered by blists - more mailing lists