Re: [PATCH v3 10/12] KVM: SEV: Forcefully invalidate SNP VMSA if its backing gmem page is zapped

[Ackerley Tng]

Sean Christopherson <seanjc@xxxxxxxxxx> writes:

> On Wed, Jul 08, 2026, Sean Christopherson wrote:
>> On Wed, Jul 08, 2026, Ackerley Tng wrote:
>> > Sean Christopherson <seanjc@xxxxxxxxxx> writes:
>> >
>> > >
>> > > [...snip...]
>> > >
>> > > @@ -1902,6 +1905,7 @@ struct kvm_x86_ops {
>> > >  	void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu);
>> > >  	int (*gmem_prepare)(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
>> > >  #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>> > > +	void (*gmem_invalidate_range)(struct kvm *kvm, struct kvm_gfn_range *range);
>> > >  	void (*gmem_reclaim_memory)(kvm_pfn_t start, kvm_pfn_t end);
>> > >  #endif
>> >
>> > I suggested CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM_MEMORY before looking at
>> > this patch.
>> >
>> > Considering this, it seems like gmem_invalidate_range is a true
>> > invalidation request and .gmem_reclaim_memory is too late for
>> > invalidation as you explained in the commit message of the renaming
>> > patch.
>> >
>> > Perhaps the new .gmem_invalidate_range should take over
>> > CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE, and .gmem_reclaim_memory should
>> > use a new CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM_MEMORY?
>> >
>> > Are they both invalidations in your opinion?
>>
>> No, definitely not, I just didn't want to add another Kconfig.  And I still don't,
>> but I agree that lumping .gmem_reclaim_memory() into GMEM_INVALIDATE doesn't work,
>> because the behavior of sev_gmem_reclaim_memory() and sev_gmem_invalidate_range()
>> are completely unrelated.
>>
>> > If we're clumping SNP configs together,
>> > CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE is also only enabled by SNP...
>>
>> Yeah, but while I don't like a Kconfig explosion, I don't really want to end up
>> with CONFIG_HAVE_KVM_ARCH_GMEM_SNP_HOOKS either.
>>
>> Oh, wait, isn't CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE going away?  I was going to say
>> we could bundle .prepare() and .reclaim() together, because conceptually they are
>> two bookends for updating out-of-band metadata, but that doesn't make a whole lot
>> of sense if .prepare() is going to disappear.

Notes from guest_memfd biweekly today:

1. .gmem_prepare() is not going away! Preparation tracking within
   guest_memfd already went away.

2. Prepare/reclaim can be combined into a single function, taking pfn,
   size (or order, order is preferred since it also communicates the
   order at which to prepare/reclaim) and whether guest_memfd needs the
   pfn to become private or shared.

3. Having prepare/reclaim take private/shared also removes leaking some
   arch-specific details into guest_memfd, like "why only
   .gmem_prepare() on conversion to private?"

>
> For consistency with .gmem_prepare(), what about going with .gmem_reclaim() instead
> of .gmem_reclaim_memory()?  E.g.
>
> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
> static void kvm_gmem_free_folio(struct folio *folio)
> {
> 	struct page *page = folio_page(folio, 0);
> 	kvm_pfn_t pfn = page_to_pfn(page);
> 	int order = folio_order(folio);
>
> 	kvm_arch_gmem_reclaim(pfn, pfn + (1ul << order));
> }
> #endif