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

[Ackerley Tng]

Yan Zhao <yan.y.zhao@xxxxxxxxx> writes:

> On Tue, Aug 18, 2026 at 08:12:36AM +0000, Ackerley Tng wrote:
>> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
>> guest_memfd. The page reference taken during PFN lookup is not needed for
>> the remainder of the handler, as checking the RMP entry, splitting the
>> 2MB page via PSMASH, and zapping shadow page tables only operate on PFNs.
>>
>> Drop the page reference immediately after retrieving the PFN instead of
>> holding it across the entire handler.
>>
>> A later patch will follow up with completely not returning refcounted pages
>> from kvm_gmem_get_pfn().
>>
>> No functional change intended.
>>
>> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>> ---
>>  arch/x86/kvm/svm/sev.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index b2738362a928b..19c31fa944e34 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -5036,12 +5036,13 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  				    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;
>> +		return;
>>  	}
> Hi Ackerley,
>
> You missed the parts in [1] which save mmu_seq before invoking kvm_gmem_get_pfn(),
> +       mmu_seq = kvm->mmu_invalidate_seq;
> +       smp_rmb();
>
> and check mmu_invalidate_retry_gfn() before snp_rmptable_psmash(pfn):
>
> +       scoped_guard(read_lock)(&kvm->mmu_lock) {
> +               if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>                         goto out;
>
> [1] https://lore.kernel.org/all/an5RJYTwlYeym--O@xxxxxxxxxx/

I thought it wasn't necessary after the fix in patch 1 but the
maintainer is always right, lol and Sashiko is right too. I'm preparing
a v2.

>>  	/*
>> @@ -5088,8 +5089,6 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  	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.699.gb54405d56f-goog
>>