Re: [PATCH v11 15/46] KVM: guest_memfd: Call arch make_shared callback for to-shared conversion

[Sean Christopherson <seanjc@google.com>]

On Wed, Aug 26, 2026, Ackerley Tng wrote:
> Omit support for calling the arch hook to make private, since SNP, the only
> implementer of the arch make-private hook today, would actually prefer
> making private only just before faulting memory into the NPTs.
> 
> Calling the make-private arch hook would require iterating both bindings
> and the filemap to find the intersection of bindings and allocated
> folios.

Why would KVM need to iterate over the bindings?  Only the RMP needs to be updated,
whether or not the RMP is currently reachable is irrelevant, no?

Subsequent calls to kvm_arch_gmem_make_private() from kvm_gmem_get_pfn() would be
superfluous, but that's already possible, e.g. if an NPT mappings is removed for
whatever reason.

> On top of that, SNP would need to figure out whether to actually
> make private based on whether the memory is about to be faulted, or
> whether it is a conversion.

This is a non-issue, no?  As above, sev_gmem_make_private() already bails early
if the page is already assigned in the RMP.

I don't care terribly about how SNP handles this, but I do want accurate reasoning
and justification so that if/when we revisit any of this in the future, we can make
informed decisions.  Because unless I'm missing something, this is an optimization
choice (eager vs. lazy to-private conversions), not a complexity tradeoff, and it's
not clear to me how we decided the lazy approach would provide better performance.

> Calling the make-shared arch hook and not the make-private arch hook does
> leak SNP-specific details into guest_memfd (as in, why only make-shared
> during conversions but not make-private?), but the additional complexity is
> not worth taking on until guest_memfd has a user actually requiring an arch
> make-private call.

...

> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end)
> +{
> +	struct folio_batch fbatch;
> +	pgoff_t next = start;
> +	int i;
> +
> +	folio_batch_init(&fbatch);
> +	while (filemap_get_folios(inode->i_mapping, &next, end - 1, &fbatch)) {
> +		for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> +			struct folio *folio = fbatch.folios[i];
> +			pgoff_t start_index, end_index;
> +			kvm_pfn_t start_pfn;
> +			kvm_pfn_t nr_pages;
> +
> +			start_index = max(start, folio->index);
> +			end_index = min(end, folio_next_index(folio));
> +			/*
> +			 * end_index is either in folio or points to
> +			 * the first page of the next folio. Hence,
> +			 * all pages in range [start_index, end_index)
> +			 * are contiguous.
> +			 */
> +			start_pfn = folio_file_pfn(folio, start_index);
> +			nr_pages = end_index - start_index;
> +
> +			kvm_arch_gmem_make_shared(start_pfn, nr_pages);
> +		}
> +
> +		folio_batch_release(&fbatch);
> +		cond_resched();
> +	}
> +}
> +#else
> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end) {}
> +#endif
> +
>  static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>  				     size_t nr_pages, uint64_t attrs,
>  				     pgoff_t *err_index)
> @@ -624,7 +661,12 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>  
>  	filter = to_private ? KVM_FILTER_SHARED : KVM_FILTER_PRIVATE;
>  	kvm_gmem_invalidate_start(inode, start, end, filter);
> +
> +	if (!to_private && kvm_arch_has_gmem_convert())
> +		kvm_gmem_make_shared(inode, start, end);
> +
>  	mas_store_prealloc(&mas, xa_mk_value(attrs));
> +
>  	kvm_gmem_invalidate_end(inode, start, end);

The real reason I responded...

Thinking about the Secure AVIC mess made me realize zapping NPTs for SNP VMs isn't
strictly necessary in this path.  The PFN isn't changing, just the attributes, and
that's (obviously) tracked in the RMP.  KVM doesn't need to zap SPTEs to induce a
fault, because the mismatched C-bit vs. RMP status will cause an #NPF(RMP), and
AFAICT kvm_mmu_page_fault() will do the right thing.  A misbehaving guest could
continue to access the shared data (assuming we stick with lazy conversions), but
that should be fine?  E.g. it's not really any different than implicit conversions.

In other words, couldn't we do this (as an on-top optimization)?  The only wrinkle
I can think of is that it could delay reconstituion of a hugepage, especially if
we opted for eager conversion (because the guest wouldn't hit #NPFs to trigger the
hugepage promotion).

diff --git arch/x86/kvm/mmu/mmu.c arch/x86/kvm/mmu/mmu.c
index 62f751952ad8..61f3e270ab61 100644
--- arch/x86/kvm/mmu/mmu.c
+++ arch/x86/kvm/mmu/mmu.c
@@ -1670,6 +1670,7 @@ static bool __kvm_rmap_zap_gfn_range(struct kvm *kvm,
 
 bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
 {
+       unsigned long shared_private =  KVM_FILTER_SHARED | KVM_FILTER_PRIVATE;
        bool flush = false;
 
        /*
@@ -1683,6 +1684,10 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
        lockdep_assert_once(kvm->mmu_invalidate_in_progress ||
                            lockdep_is_held(&kvm->slots_lock));
 
+       if (gmem_in_place_conversion && !kvm_has_mirrored_tdp(kvm) &&
+           ((range->attr_filter & shared_private) != shared_private))
+               return false;
+
        if (kvm_memslots_have_rmaps(kvm))
                flush = __kvm_rmap_zap_gfn_range(kvm, range->slot,
                                                 range->start, range->end,