Re: [PATCH 12/24] KVM: SEV: Add an anonymous "psc" struct to track current PSC metadata

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] KVM incorrectly passes the 2M page size flag to userspace for partially completed 2M PSC entries, despite forcing 4K handling.
--

commit c91cb923b18a5adb2ee4beca4e478b8c40f1fbc2
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Add an anonymous "psc" struct to track current PSC metadata

This commit groups Page State Change fields into an anonymous psc struct 
within vcpu_sev_es_state to improve code readability. It also renames 
several local variables to clarify that they point to guest-accessible data.
Lastly, it clarifies that inflight represents the batch size of pages.

Message-ID: <20260501202250.2115252-13-seanjc@google.com>

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 1982d13e71d9e..9f6543cebedf5 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

[ ... ]

> @@ -4000,14 +3995,14 @@ static int snp_begin_psc(struct vcpu_svm *svm)
>  	 * KVM_HC_MAP_GPA_RANGE exit.
>  	 */
>  	while (++idx <= idx_end) {
> -		struct psc_entry entry = READ_ONCE(entries[idx]);
> +		struct psc_entry entry = READ_ONCE(guest_psc->entries[idx]);
>  
>  		if (entry.operation != entry_start.operation ||
>  		    entry.gfn != entry_start.gfn + npages ||
>  		    entry.cur_page || !!entry.pagesize != huge)
>  			break;
>  
> -		svm->sev_es.psc_inflight++;
> +		sev_es->psc.batch_size++;
>  		npages += huge ? 512 : 1;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the Page State
Change updates in arch/x86/kvm/svm/sev.c:snp_begin_psc(), I noticed a
potential problem when handling partially completed 2M requests.

When a 2M request is interrupted, the guest resubmits it with
entry_start.pagesize set to true and entry_start.cur_page > 0. KVM handles
this by forcing 4K handling for the remaining pages, setting huge = false,
adjusting npages, and advancing the gfn.

However, when KVM forwards this request to userspace via the
KVM_HC_MAP_GPA_RANGE hypercall right below this loop, it populates the page
size argument using the original guest request:

switch (entry_start.operation) {
case VMGEXIT_PSC_OP_PRIVATE:
case VMGEXIT_PSC_OP_SHARED:
    ...
    vcpu->run->hypercall.args[2] |= entry_start.pagesize
                                    ? KVM_MAP_GPA_RANGE_PAGE_SZ_2M
                                    : KVM_MAP_GPA_RANGE_PAGE_SZ_4K;

Could this result in passing KVM_MAP_GPA_RANGE_PAGE_SZ_2M along with a
4K-aligned (but not 2M-aligned) GPA and a non-512-multiple page count?

If host userspace performs alignment checks on the mapping request, would
this cause it to reject the malformed hypercall and fatally terminate the VM?

Would it be safer to use the adjusted huge boolean instead of
entry_start.pagesize when populating these arguments?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529183549.1104619-1-pbonzini@redhat.com?part=12