All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Fuad Tabba <fuad.tabba@linux.dev>
Cc: "Ackerley Tng" <ackerleytng@google.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Michael Roth" <michael.roth@amd.com>,
	"Hyunwoo Kim" <imv4bel@gmail.com>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Jörg Rödel" <joro@8bytes.org>
Subject: Re: [PATCH v4 18/18] KVM: guest_memfd: Combine .gmem_prepare()+.gmem_invalidate() into .gmem_convert()
Date: Mon, 13 Jul 2026 12:56:53 -0700	[thread overview]
Message-ID: <alVDBWOl5tGZb1Wm@google.com> (raw)
In-Reply-To: <CA+EHjTxLKvj-qOQFgZ66Pq+ULQ0D98QU65gGe_2wWnR5_fTa0A@mail.gmail.com>

On Mon, Jul 13, 2026, Fuad Tabba wrote:
> On Mon, 13 Jul 2026 at 16:41, Ackerley Tng <ackerleytng@google.com> wrote:
> >
> > Fuad Tabba <fuad.tabba@linux.dev> writes:
> >
> > >
> > > [...snip...]
> > >
> >
> > Thanks for bringing [1] up, I didn't make the link when this change was
> > being discussed.
> >
> > pKVM definitely needs a callback when the folio is released from
> > guest_memfd's ownership.
> >
> > >
> > > free_folio is the only to_private=false caller today, but the cover
> > > letter sketches the other one, a private->shared conversion through
> > > kvm_gmem_make_shared(inode, start, end). Once that lands both reach
> > > the hook with to_private=false, and as far as I can tell, the only
> > > thing separating them is kvm == NULL / INVALID_GPA. That isn't
> >
> > On the folio teardown path kvm has to be non-NULL so that SNP can find
> > out which VM to unassign from, so you're right that INVALID_GPA won't be
> > sufficient to separate conversion to private and teardown, for pKVM,
> > from the arch perspective.
> >
> > > documented as a contract, and nothing in-tree reads it on that branch
> > > (the kvm/gpa check is under if (to_private), and
> > > sev_gmem_make_shared() doesn't take a kvm at all), so nothing would
> > > catch it regressing. The sketch is also inode-scoped, so I'm not sure
> > > the conversion path passes a non-NULL kvm at all; if it doesn't, the
> > > two aren't distinguishable.
> > >
> > >>
> > >> [...snip...]
> > >>
> > >
> > > The Kconfig now couples them too: an arch can't take teardown without also
> > > taking conversion. pKVM wants teardown and not conversion, so that's a problem
> > > for us today, not only once the second caller lands.
> > >
> > > This is the same ground as v8 15/46, which ended with the two split under
> > > separate Kconfigs. Suzuki confirmed CCA disables both; pKVM wants teardown only,
> > > since our conversions are guest-initiated and EL2 has already done the
> > > transition by the time the host runs the generic path [1].
> > >
> > > I'd rather keep the free path on its own callback. If you want a single hook,
> > > passing the caller explicitly instead of leaving it implied by kvm would work
> > > too. Merging the two conversion directions is a genuine cleanup, it's the free
> > > path I'd like to keep separate.
> > >
> >
> > Is it okay if later, we add a .gmem_teardown callback with a new CONFIG
> > flag that pKVM can use?
> 
> That works for me, and I agree it's nicer to introduce the callback along
> with its first user.
> 
> Sean, is that the plan you have in mind as well? Two things that matter for
> pKVM when it lands: the new CONFIG needs to be independent of the conversion
> one, so that free_folio fires for an arch that selects only teardown (pKVM
> wants teardown and not conversion), and the teardown callback stays distinct
> from a private->shared conversion. Good point on teardown needing a non-NULL
> kvm for SNP,

Converting back to SHARED doesn't require a non-NULL @kvm pointer.  SNP needs
the ASID and thus the VM when assigning memory to a VM, but converting memory
back to SHARED, i.e. returning to the host, doesn't need that information.

> incidentally, that's another reason keying off the sentinel wouldn't have
> worked.

It can work, but it would be all kinds of gross, and arguably wrong.  To pass
a non-NULL @kvm for the true convert-to-shared (not reclaim), guest_memfd would
need to iterate over all bindings to get at the @kvm instances.  That's "fine",
but wasteful and confusing because the SNP conversion is a platform-level thing
(as above, unassigning from a VM doesn't require any information about the VM).

> If that's where this is heading, I'm happy.

Yeah, this is roughly what the total diff will look like.

diff --git arch/x86/include/asm/kvm-x86-ops.h arch/x86/include/asm/kvm-x86-ops.h
index 6091987860bd..1764bad03b17 100644
--- arch/x86/include/asm/kvm-x86-ops.h
+++ arch/x86/include/asm/kvm-x86-ops.h
@@ -146,7 +146,7 @@ KVM_X86_OP(vcpu_deliver_sipi_vector)
 KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
 KVM_X86_OP_OPTIONAL(get_untagged_addr)
 KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)
-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#if defined(CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT) || defined(CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM)
 KVM_X86_OP_OPTIONAL_RET0(gmem_convert)
 #endif
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
diff --git arch/x86/include/asm/kvm_host.h arch/x86/include/asm/kvm_host.h
index a70e734af122..08c4af487fa1 100644
--- arch/x86/include/asm/kvm_host.h
+++ arch/x86/include/asm/kvm_host.h
@@ -1724,7 +1724,7 @@ struct kvm_x86_ops {
 
 	gva_t (*get_untagged_addr)(struct kvm_vcpu *vcpu, gva_t gva, unsigned int flags);
 	void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu);
-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#if defined(CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT) || defined(CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM)
 	int (*gmem_convert)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 			    kvm_pfn_t nr_pages, int max_order, bool to_private);
 #endif
diff --git arch/x86/kvm/Kconfig arch/x86/kvm/Kconfig
index 52e75d83a4d2..538ed1e80332 100644
--- arch/x86/kvm/Kconfig
+++ arch/x86/kvm/Kconfig
@@ -161,6 +161,7 @@ config KVM_AMD_SEV
 	select ARCH_HAS_CC_PLATFORM
 	select KVM_GENERIC_MEMORY_ATTRIBUTES
 	select HAVE_KVM_ARCH_GMEM_CONVERT
+	select HAVE_KVM_ARCH_GMEM_RECLAIM
 	select HAVE_KVM_ARCH_GMEM_INVALIDATE
 	select HAVE_KVM_ARCH_GMEM_POPULATE
 	help
diff --git arch/x86/kvm/x86.c arch/x86/kvm/x86.c
index f0c49a92d041..11017f49b94a 100644
--- arch/x86/kvm/x86.c
+++ arch/x86/kvm/x86.c
@@ -10625,6 +10625,16 @@ int kvm_arch_gmem_convert(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 {
 	return kvm_x86_call(gmem_convert)(kvm, gfn, pfn, nr_pages, max_order, to_private);
 }
+#endif
+
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
+void kvm_arch_gmem_reclaim(kvm_pfn_t pfn, kvm_pfn_t nr_pages, int max_order)
+{
+	WARN_ON_ONCE(kvm_x86_call(gmem_convert)(NULL, -1ull, pfn, nr_pages, max_order, false));
+}
+#endif
+
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
 void kvm_arch_gmem_invalidate_range(struct kvm *kvm, struct kvm_gfn_range *range)
 {
 	kvm_x86_call(gmem_invalidate_range)(kvm, range);
diff --git include/linux/kvm_host.h include/linux/kvm_host.h
index 9391ffcc90ff..fdbd697d0337 100644
--- include/linux/kvm_host.h
+++ include/linux/kvm_host.h
@@ -2614,6 +2614,10 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
 		       kvm_gmem_populate_cb post_populate, void *opaque);
 #endif
 
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
+void kvm_arch_gmem_reclaim(kvm_pfn_t pfn, kvm_pfn_t nr_pages, int order);
+#endif
+
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
 void kvm_arch_gmem_invalidate_range(struct kvm *kvm, struct kvm_gfn_range *range);
 #endif
diff --git virt/kvm/Kconfig virt/kvm/Kconfig
index 53803c5777a4..c3c0ee253fc7 100644
--- virt/kvm/Kconfig
+++ virt/kvm/Kconfig
@@ -111,6 +111,10 @@ config HAVE_KVM_ARCH_GMEM_CONVERT
        bool
        depends on KVM_GUEST_MEMFD
 
+config HAVE_KVM_ARCH_GMEM_RECLAIM
+       bool
+       depends on KVM_GUEST_MEMFD
+
 config HAVE_KVM_ARCH_GMEM_INVALIDATE
        bool
        depends on KVM_GUEST_MEMFD
diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
index aeb765831c32..173a7ed008d1 100644
--- virt/kvm/guest_memfd.c
+++ virt/kvm/guest_memfd.c
@@ -516,13 +516,11 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
 	return MF_DELAYED;
 }
 
-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
 static void kvm_gmem_free_folio(struct folio *folio)
 {
-	WARN_ON_ONCE(kvm_arch_gmem_convert(NULL, -1ull,
-					   folio_file_pfn(folio, 0),
-					   folio_nr_pages(folio),
-					   folio_order(folio), false));
+	kvm_arch_gmem_reclaim(folio_file_pfn(folio, 0), folio_nr_pages(folio),
+			      folio_order(folio));
 }
 #endif
 
@@ -530,7 +528,7 @@ static const struct address_space_operations kvm_gmem_aops = {
 	.dirty_folio = noop_dirty_folio,
 	.migrate_folio	= kvm_gmem_migrate_folio,
 	.error_remove_folio = kvm_gmem_error_folio,
-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
 	.free_folio = kvm_gmem_free_folio,
 #endif
 };

      reply	other threads:[~2026-07-13 19:56 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 20:49 [PATCH v4 00/18] KVM: SEV: Fix RMP #PF due to freeing in-use VMSA Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 01/18] KVM: SEV: Track the GPA of the guest-controlled VMSA used for SNP guests Sean Christopherson
2026-07-09 21:07   ` sashiko-bot
2026-07-10  0:28     ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 02/18] KVM: SEV: Extract loading of guest-provided VMSA to a separate helper Sean Christopherson
2026-07-09 21:06   ` sashiko-bot
2026-07-09 20:49 ` [PATCH v4 03/18] KVM: SEV: Mark vCPU RUNNABLE after AP_CREATE, even if VMSA is unusable Sean Christopherson
2026-07-09 21:10   ` sashiko-bot
2026-07-09 20:49 ` [PATCH v4 04/18] KVM: SEV: Wire up kvm_x86_ops.gmem_xxx() if and only if CONFIG_KVM_AMD_SEV=y Sean Christopherson
2026-07-10  0:11   ` Ackerley Tng
2026-07-10  0:25     ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 05/18] KVM: x86: Serialize writes to disabled_quirks using kvm->lock Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 06/18] KVM: x86: Ensure runtime reads of disabled_quirks are resolved once Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 07/18] KVM: x86/mmu: Fold kvm_mmu_zap_memslot() into kvm_arch_flush_shadow_memslot() Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 08/18] KVM: x86/mmu: Split kvm_mmu_zap_all_fast() into "front" and "back" halves Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 09/18] KVM: x86/mmu: Use split "zap all fast" helpers when invalidating memslot Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 10/18] KVM: SEV: Forcefully invalidate SNP VMSA if its backing gmem page is zapped Sean Christopherson
2026-07-09 21:06   ` sashiko-bot
2026-07-10  0:30     ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 11/18] KVM: SEV: Mark vCPU has having guest-provided VMSA even if its invalid Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 12/18] KVM: x86: Guard .gmem_prepare() declarations with HAVE_KVM_GMEM_PREPARE=y Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 13/18] KVM: guest_memfd: Pass GPA, not GFN, to prepare() hook Sean Christopherson
2026-07-10  0:36   ` Ackerley Tng
2026-07-10 22:08     ` Sean Christopherson
2026-07-10 22:43       ` Ackerley Tng
2026-07-10 23:44         ` Sean Christopherson
2026-07-13 22:28           ` Ackerley Tng
2026-07-14  0:00             ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 14/18] KVM: guest_memfd: Drop the redundant printk on arch gmem_prepare() failure Sean Christopherson
2026-07-10  0:18   ` Ackerley Tng
2026-07-09 20:49 ` [PATCH v4 15/18] KVM: guest_memfd: Fold __kvm_gmem_prepare_folio() into its sole caller Sean Christopherson
2026-07-09 21:11   ` sashiko-bot
2026-07-10  0:32     ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 16/18] KVM: guest_memfd: Explicitly pass number of pages to kvm_arch_gmem_prepare() Sean Christopherson
2026-07-09 21:12   ` sashiko-bot
2026-07-10  0:23   ` Ackerley Tng
2026-07-09 20:49 ` [PATCH v4 17/18] KVM: guest_memfd: Align the gfn as well as the pfn when "preparing" a folio Sean Christopherson
2026-07-09 21:15   ` sashiko-bot
2026-07-10  0:29     ` Ackerley Tng
2026-07-10  0:33       ` Sean Christopherson
2026-07-09 20:49 ` [PATCH v4 18/18] KVM: guest_memfd: Combine .gmem_prepare()+.gmem_invalidate() into .gmem_convert() Sean Christopherson
2026-07-10  0:34   ` Ackerley Tng
2026-07-12 19:05   ` Fuad Tabba
2026-07-13 15:41     ` Ackerley Tng
2026-07-13 18:57       ` Fuad Tabba
2026-07-13 19:56         ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alVDBWOl5tGZb1Wm@google.com \
    --to=seanjc@google.com \
    --cc=ackerleytng@google.com \
    --cc=fuad.tabba@linux.dev \
    --cc=imv4bel@gmail.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=thomas.lendacky@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.