From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f202.google.com (mail-pf1-f202.google.com [209.85.210.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3EE633DB640 for ; Thu, 21 May 2026 13:21:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779369712; cv=none; b=Bd56cS4c/dcJ6VDo/LEjiXXYtzyCnwn3dcyLJFu21u28fkwj55R1z/q/AcObDC9dw2UvFK3FDBgYZyDJ1HgHVj+ng0hNg+HjZd4TgMFlcrFyOIotDRIsVx39mgsdMKvN9egvPiPOS4XdvGGELUTd5jxbTBlBgOoFLpiJf9QF0GI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779369712; c=relaxed/simple; bh=GR5EzgovptfxCzgfSgkwiZJkZXAcFVBl5/RrfGUEW8E=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=fc9w4IKiW7o0Rb4ZXdycNCxwuRVSGPJ64OrsRAT5l7CnseiwzhJYLOv0PJLNPojWiJofK7VVoKoXrKyugnIIgrX0g/UvTgb1XJkgGG+6Up6gkjB6zEcF2SEFIbszWsZ9w3tIX1SUmk/oTMTGVdJtepoo3d9VkOFZHhKUiVrcyf8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=IjlkRrzH; arc=none smtp.client-ip=209.85.210.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="IjlkRrzH" Received: by mail-pf1-f202.google.com with SMTP id d2e1a72fcca58-82fa7c6699fso9677141b3a.1 for ; Thu, 21 May 2026 06:21:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1779369710; x=1779974510; darn=lists.linux.dev; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=iIrwVyrI6nQx/ALkh7jG2SEyqSQ/V8q8plyAwbOOpCU=; b=IjlkRrzHNtsnx/rBUqcNZVLWeIKJLVkGTmxEASRWufM2Hx8duTjXzk8/BP2fTtH+gU AShq8/k3S4oNcpTK6VtTebwTteC3RZMpvMeLmUrGHwEHbPY3WWaMyxFhMMUhkNVQ9P0+ vbMhajN4k/jaJbM6PnSoCD1eUfCgI4xmW7yiv0bJmuCJ0dsV7ijW4W/g/tFO9TQkd2MH 8Vs6oHoREsjRbTc/MaCUfQSvJdBKVZa5MG6kSGElH3hQ+XOAXUeYKYZTSpSGRw0xXFIc 3jX5n4VdpbIr2CSjUpihCVT8XIg/PzXPlKeDHZssbS6gzrUleSlb/KQpiXLo1KPgIPg1 MafQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779369710; x=1779974510; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=iIrwVyrI6nQx/ALkh7jG2SEyqSQ/V8q8plyAwbOOpCU=; b=DsXfd8ttJpxNViQYzDalreMwhxPfE+IGmfdSntddh4B9p+uXNgIDjoHpu932VVyA/E BOqVhwhoTWsHcwoeqhWnQOrJkS++71eD2HgU8UDvdTy0LttEtO6O7KHNmlAqmmXzEmAU vXsfnon0M6V4SYCskYCX1Is6GK0M4nWm6aN7ddwEcYMOT/xnwsQvqXvtIiHeSuoIn+3Y QJ36/lKMc6TFy6ICWFK+w9+WdOVIMqxB2QVee9x90YYn59FjXtjvH1CjJ3xxXAm5uRnM t5+e9CWj/c3dUgt+f+TSf1HouW8ta8GEcWpZTyMRWSMOY5QXK4l0y176N2UB1STlDAv2 Lm/A== X-Forwarded-Encrypted: i=1; AFNElJ9wbYddf4pD349BFLlZ4oYNS66LRIbxKrwuDOn9xOW498LzIkkcjY9ugU7G2rqzT32InFnBEAL6ZVoZ@lists.linux.dev X-Gm-Message-State: AOJu0YxnLHogRIoQXRBCVFNaIUhayCenQyhw+5PoidIa/nreun7z9kTi 1GX9NJRTW5MCJs4Wi5LTkvou4F8ARHjgJgTLxApDLseqLZUtGLuXWUTA/stPdQt8N3xh4HpXEeR r8vYfFQ== X-Received: from pfbmb13.prod.google.com ([2002:a05:6a00:760d:b0:82f:4abd:a354]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:2793:b0:835:cc47:6fe7 with SMTP id d2e1a72fcca58-8414adf7c0emr2878031b3a.30.1779369710072; Thu, 21 May 2026 06:21:50 -0700 (PDT) Date: Thu, 21 May 2026 06:21:49 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4@google.com> <20260507-gmem-inplace-conversion-v6-21-91ab5a8b19a4@google.com> Message-ID: Subject: Re: [PATCH v6 21/43] KVM: SEV: Make 'uaddr' parameter optional for KVM_SEV_SNP_LAUNCH_UPDATE From: Sean Christopherson To: Fuad Tabba Cc: ackerleytng@google.com, aik@amd.com, andrew.jones@linux.dev, binbin.wu@linux.intel.com, brauner@kernel.org, chao.p.peng@linux.intel.com, david@kernel.org, ira.weiny@intel.com, jmattson@google.com, jthoughton@google.com, michael.roth@amd.com, oupton@kernel.org, pankaj.gupta@amd.com, qperret@google.com, rick.p.edgecombe@intel.com, rientjes@google.com, shivankg@amd.com, steven.price@arm.com, willy@infradead.org, wyihan@google.com, yan.y.zhao@intel.com, forkloop@google.com, pratyush@kernel.org, suzuki.poulose@arm.com, aneesh.kumar@kernel.org, liam@infradead.org, Paolo Bonzini , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Jonathan Corbet , Shuah Khan , Shuah Khan , Vishal Annapurve , Andrew Morton , Chris Li , Kairui Song , Kemeng Shi , Nhat Pham , Baoquan He , Barry Song , Axel Rasmussen , Yuanchu Xie , Wei Xu , Youngjun Park , Qi Zheng , Shakeel Butt , Kiryl Shutsemau , Jason Gunthorpe , Vlastimil Babka , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-coco@lists.linux.dev Content-Type: text/plain; charset="us-ascii" On Thu, May 21, 2026, Fuad Tabba wrote: > Hi, > > On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay > wrote: > > > > From: Michael Roth > > > > For vm_memory_attributes=1, in-place conversion/population is not > > supported, so the initial contents necessarily must need to come > > from a separate src address, which is enforced by the current > > implementation. However, for vm_memory_attributes=0, it is possible for > > guest memory to be initialized directly from userspace by mmap()'ing the > > guest_memfd and writing to it while the corresponding GPA ranges are in > > a 'shared' state before converting them to the 'private' state expected > > by KVM_SEV_SNP_LAUNCH_UPDATE. > > > > Update the handling/documentation for KVM_SEV_SNP_LAUNCH_UPDATE to allow > > for 'uaddr' to be set to NULL when vm_memory_attributes=0, which > > SNP_LAUNCH_UPDATE will then use to determine when it should/shouldn't > > copy in data from a separate memory location. Continue to enforce > > non-NULL for the original vm_memory_attributes=1 case. > > > > Signed-off-by: Michael Roth > > [Added src_page check in error handling path when the firmware command fails] > > [Dropped ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES] > > Signed-off-by: Ackerley Tng > > I'm not very familiar with the SEV-SNP populate flows, but it looks > like Sashiko is on to something: > https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=21 > > - a potential read-only page overwrite, because src_page is acquired > via get_user_pages_fast() without the FOLL_WRITE flag, but is then > overwritten via memcpy Oof, yeah, that's bad. Adding FOLL_WRITE to kvm_gmem_populate() feels wrong, and could break uABI, but doing gup() in SNP code would reintroduce the AB-BA issue with filemap_invalidate_lock(). Aha! Not if we use get_user_page_fast_only(). Ugh, but then we'd have to plumb the userspace address into the post-populated callback. Hrm. Given that no one has yelled about overwriting their CPUID page, and given that the CPUID page is likely dynamically created and thus is unlikely to be a read-only mapping (e.g. versus the initial image), maybe this? diff --git arch/x86/kvm/svm/sev.c arch/x86/kvm/svm/sev.c index 37d4cfa5d980..c73c028d72c1 100644 --- arch/x86/kvm/svm/sev.c +++ arch/x86/kvm/svm/sev.c @@ -2456,6 +2456,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp) sev_populate_args.type = params.type; count = kvm_gmem_populate(kvm, params.gfn_start, src, npages, + params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID, sev_gmem_post_populate, &sev_populate_args); if (count < 0) { argp->error = sev_populate_args.fw_error; diff --git arch/x86/kvm/vmx/tdx.c arch/x86/kvm/vmx/tdx.c index f97bcf580e6d..33f35be4455b 100644 --- arch/x86/kvm/vmx/tdx.c +++ arch/x86/kvm/vmx/tdx.c @@ -3188,7 +3188,7 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c }; gmem_ret = kvm_gmem_populate(kvm, gpa_to_gfn(region.gpa), u64_to_user_ptr(region.source_addr), - 1, tdx_gmem_post_populate, &arg); + 1, false, tdx_gmem_post_populate, &arg); if (gmem_ret < 0) { ret = gmem_ret; break; diff --git include/linux/kvm_host.h include/linux/kvm_host.h index 61a3430957f2..b83cda2870ba 100644 --- include/linux/kvm_host.h +++ include/linux/kvm_host.h @@ -2596,7 +2596,8 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, struct page *page, void *opaque); -long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages, +long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, + long npages, bool writable, kvm_gmem_populate_cb post_populate, void *opaque); #endif diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c index a35a55571a2d..6553d4e032ce 100644 --- virt/kvm/guest_memfd.c +++ virt/kvm/guest_memfd.c @@ -858,7 +858,8 @@ static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot, return ret; } -long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages, +long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, + long npages, bool writable, kvm_gmem_populate_cb post_populate, void *opaque) { struct kvm_memory_slot *slot; @@ -892,8 +893,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long if (src) { unsigned long uaddr = (unsigned long)src + i * PAGE_SIZE; + unsigned int flags = writable ? FOLL_WRITE : 0; - ret = get_user_pages_fast(uaddr, 1, 0, &src_page); + ret = get_user_pages_fast(uaddr, 1, flags, &src_page); if (ret < 0) break; if (ret != 1) { > - an ordering violation with the kunmap_local() calls Yeesh, that's a new one for me. Thankfully this is 64-bit only, so it's not an issue. > These predate this patch series and are just being touched by the > 'src_page' addition, but if Sashiko's right, these should probably be > fixed sooner rather than later. Yeah, ditto with the offset wrapping case.