Re: [PATCH v3 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Michael Roth]

On Thu, Aug 20, 2026 at 11:32:35PM +0000, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> 
> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> guest_memfd.
> 
> Drop the page reference immediately after retrieving the PFN instead of
> holding it across the entire handler, and adopt the KVM MMU invalidation
> protocol.
> 
> To avoid wrongly warning about not finding an assigned RMP entry if an
> invalidation had taken place, check for invalidations before warning.

I'm ok either way, but something like the following I think would read a
bit clearer:

  When handling an RMP fault, KVM retrieves the PFN for a private GPA from
  guest_memfd as well as a refcounted struct page. This reference is
  held for the duration of the RMP fault handling to ensure the page isn't
  reallocated to another guest while processing the RMP fault.
  
  A later patch will follow up with completely not returning refcounted pages
  from kvm_gmem_get_pfn(), so prepare for that by dropping the page reference
  immediately after retrieving the PFN and, in place of that, implement the
  stardard KVM MMU invalidation logic to address the above scenario.

  While here, also make use of this KVM MMU invalidation logic to avoid
  warning about not finding an assigned RMP entry by first checking if an
  invalidation had taken place since entering the RMP fault handler.

But after writing that I realize the last paragraph is pertaining to a
new hunk added in v3, which is sort of a bug fix (in the same that patch #1 is
a fix anyway), since it fixes spurious warnings that were previously
triggerable via truncate() racing with RMP faults. However, that's exactly the
same scenario patch #1 is trying to fix, except in this case the race happens
before this path reaches the PSMASH vs. after the PSMASH like patch #1
addresses.

However, unlike patch #1 where we only need to check once again
after-the-fact, it's not possible to reliably check the before case without
the MMU invalidation logic in place, so I'm okay with squashing that
additional check in here, but decoupling that change (prep-for-something-else
vs. bug-fix made possible by prep-for-something-else) and moving it to a
patch #3 would help with de-tangle things for future git blame's.

(sorry if this seems like nit-picking; this function unfortunately spends a
lot of time in the spotlight so trying to make life easier for people making
sense of it)

Thanks,

Mike

> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> guest_memfd.
> 
> Drop the page reference immediately after retrieving the PFN instead of
> holding it across the entire handler, and adopt the KVM MMU invalidation
> protocol.
> 
> To avoid wrongly warning about not finding an assigned RMP entry if an
> invalidation had taken place, check for invalidations before warning.
> 
> When the RMP level is 4K, the function exits. That doesn't need checking
> for invalidations, since if it is 4K and there was an invalidation, not
> psmashing and not zapping is the right thing to do.
> 
> If the RMP level is 2M (the only other option), use the invalidation
> protocol before attempting to psmash. This ensures that if the page is
> truncated and freed, and then re-allocated to another SNP VM (the RMP entry
> is now assigned, but to another SNP VM), psmashing would be correctly
> skipped.
> 
> A later patch will follow up with completely not returning refcounted pages
> from kvm_gmem_get_pfn().
> 
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Reviewed-by: Michael Roth <michael.roth@xxxxxxx>
> Co-developed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> ---
>  arch/x86/kvm/svm/sev.c | 47 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b2738362a928b..563870342a2ba 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	struct kvm_memory_slot *slot;
>  	struct kvm *kvm = vcpu->kvm;
>  	int order, rmp_level, ret;
> +	unsigned long mmu_seq;
>  	struct page *page;
>  	bool assigned;
>  	kvm_pfn_t pfn;
> @@ -5030,18 +5031,26 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  		return;
>  	}
>  
> +	mmu_seq = kvm->mmu_invalidate_seq;
> +	smp_rmb();
> +
>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>  	if (ret) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>  				    gpa);
>  		return;
>  	}
> +	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>  	if (ret || !assigned) {
> -		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
> -				    gpa, pfn, ret);
> -		goto out_no_trace;
> +		guard(read_lock)(&kvm->mmu_lock);
> +
> +		if (!mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
> +			pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
> +					    gpa, pfn, ret);
> +
> +		return;
>  	}
>  
>  	/*
> @@ -5069,27 +5078,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	if (rmp_level == PG_LEVEL_4K)
>  		goto out;
>  
> -	ret = snp_rmptable_psmash(pfn);
> -	if (ret) {
> -		/*
> -		 * Look it up again. If it's 4K now then the PSMASH may have
> -		 * raced with another process and the issue has already resolved
> -		 * itself. If it's not assigned, then this must have raced with
> -		 * another process that made this page shared.
> -		 */
> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> +	scoped_guard(read_lock, &kvm->mmu_lock) {
> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>  			goto out;
>  
> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> -				    gpa, pfn, ret);
> +		ret = snp_rmptable_psmash(pfn);
> +		if (ret) {
> +			/*
> +			 * Look it up again. If it's 4K now then the PSMASH may
> +			 * have raced with another process and the issue has
> +			 * already resolved itself. If it's not assigned, then
> +			 * this must have raced with another process that made
> +			 * this page shared.
> +			 */
> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> +				goto out;
> +
> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> +					    gpa, pfn, ret);
> +		}
>  	}
>  
>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
>  out:
>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
> -out_no_trace:
> -	kvm_release_page_unused(page);
>  }
>  
>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
> 
> -- 
> 2.55.0.766.g2966f0265a-goog
>