Re: [PATCH 02/60] KVM: SVM: Add support for the SEV-SNP #HV doorbell page NAE event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The SVM_VMGEXIT_HVDB_PAGE NAE event is unconditionally handled for all SEV-SNP guests without verifying if the Restricted Injection feature is active.
--

commit 365a94cc5c8c1fc632b2af221069f82dee7dea53
Author: Melody Wang <huibo.wang@amd.com>

KVM: SVM: Add support for the SEV-SNP #HV doorbell page NAE event

To support Restricted Injection, the SEV-SNP guest must register a doorbell
page for use with #HV. This is done using the #HV doorbell page NAE event.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 62b5befe0eed9..f022be532695a 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3553,6 +3553,10 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
>  		    control->exit_info_1 == control->exit_info_2)
>  			goto vmgexit_err;
>  		break;
> +	case SVM_VMGEXIT_HVDB_PAGE:
> +		if (!is_sev_snp_guest(vcpu))
> +			goto vmgexit_err;

[Severity: High]
Should this validation also verify that the Restricted Injection feature is
enabled for the vCPU?

If SVM_VMGEXIT_HVDB_PAGE is allowed for all SEV-SNP guests without checking
SVM_SEV_FEAT_RESTRICTED_INJECTION, a guest might successfully register doorbell
pages on a VM where Restricted Injection was not negotiated. Could this cause
migration to fail if the VM is moved to an older host or one that enforces the
feature gate?

> +		break;
>  	default:
>  		reason = GHCB_ERR_INVALID_EVENT;
>  		goto vmgexit_err;

[ ... ]

> @@ -4321,6 +4325,65 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
>  	return 1; /* resume guest */
>  }
>  
> +static int sev_snp_hv_doorbell_page(struct vcpu_svm *svm)
> +{
> +	struct kvm_vcpu *vcpu = &svm->vcpu;
> +	struct kvm_host_map hvdb_map;
> +	gpa_t hvdb_gpa;
> +	u64 request;
> +
> +	if (!is_sev_snp_guest(vcpu))
> +		return -EINVAL;

[Severity: High]
Similarly here in sev_snp_hv_doorbell_page(), is it necessary to ensure
the Restricted Injection feature is active?

KVM typically requires new guest-visible behaviors to be explicitly
negotiated. Would checking something like sev_snp_is_rinj_active(vcpu) be
needed here to prevent silently enabling the feature for guests that haven't
requested it?

> +
> +	request = svm->vmcb->control.exit_info_1;
> +	hvdb_gpa = svm->vmcb->control.exit_info_2;

[ ... ]

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