Re: [PATCH v2 6/8] KVM: SEV: Add SNP vCPU state get and set commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM_SEV_SNP_GET_VCPU_STATE silently hides the configured VMSA GPA during transient MMU invalidations, causing state loss during live migration.
- [High] KVM_SEV_SNP_SET_VCPU_STATE incorrectly rejects valid 2MB-aligned VMSA GPAs.
--

commit ccc907edf743bac6d650ebf0b64bb61800fedfea
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: SEV: Add SNP vCPU state get and set commands

Add KVM_SEV_SNP_GET_VCPU_STATE and KVM_SEV_SNP_SET_VCPU_STATE as
vCPU-scoped KVM_MEMORY_ENCRYPT_OP commands and advertise
KVM_CAP_SNP_VCPU_STATE.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7a9ef1bc54e9d..3e0727e251a93 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -2585,6 +2587,89 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	return ret;
>  }
>  
> +static int snp_get_vcpu_state(struct kvm_vcpu *vcpu,
> +			      struct kvm_sev_cmd *argp)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_sev_snp_vcpu_state state = {};
> +
> +	if (!is_sev_snp_guest(vcpu))
> +		return -ENOTTY;
> +	if (!to_kvm_sev_info(kvm)->snp_context)
> +		return -EINVAL;
> +
> +	guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
> +
> +	if (VALID_PAGE(svm->sev_es.snp_guest_vmsa_gpa) &&
> +	    VALID_PAGE(svm->vmcb->control.vmsa_pa)) {

[Severity: High]
Does this safely handle transient MMU invalidations?

When a concurrent host MMU invalidation (e.g., from page migration) overlaps
with a VMSA reload, __sev_snp_reload_vmsa() will hit the retry path and set
vmsa_pa to INVALID_PAGE while correctly queuing KVM_REQ_VMSA_PAGE_RELOAD.

If userspace queries the state to migrate the VM during this window, this
check evaluates to false because vmsa_pa is INVALID_PAGE. Consequently, it
returns VMSA_VALID = 0 even though snp_guest_vmsa_gpa is correctly
configured. Can this cause the configured VMSA GPA to be silently
dropped and lead to state loss on the destination VM?

> +		state.vmsa_gpa = svm->sev_es.snp_guest_vmsa_gpa;
> +		state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_VMSA_VALID;
> +	}
[ ... ]
> +static int snp_set_vcpu_state(struct kvm_vcpu *vcpu,
> +			      struct kvm_sev_cmd *argp)
> +{
[ ... ]
> +	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_VMSA_VALID) {
> +		if (!PAGE_ALIGNED(state.vmsa_gpa) ||
> +		    !page_address_valid(vcpu, state.vmsa_gpa) ||
> +		    IS_ALIGNED(state.vmsa_gpa, PMD_SIZE))

[Severity: High]
Is it intended to reject perfectly valid 4KB-aligned VMSA GPAs if they happen
to fall on a 2MB boundary?

A guest OS or userspace VMM linearly allocating memory might naturally land on
a 2MB-aligned address (e.g., 0x200000). The IS_ALIGNED(state.vmsa_gpa,
PMD_SIZE) condition evaluates to true for these addresses and rejects them
with -EINVAL, which can cause sporadic VM boot or migration failures.

> +			return -EINVAL;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908103338.427254-1-joro@8bytes.org?part=6