On 7/16/25 00:56, Nikunj A Dadhania wrote:
> Require a minimum GHCB version of 2 when starting SEV-SNP guests through
> KVM_SEV_INIT2. When a VMM attempts to start an SEV-SNP guest with an
> incompatible GHCB version (less than 2), reject the request early rather
> than allowing the guest kernel to start with an incorrect protocol version
> and fail later with GHCB_SNP_UNSUPPORTED guest termination.
>
> Hypervisor logs the guest termination with GHCB_SNP_UNSUPPORTED error code:
>
> kvm_amd: SEV-ES guest requested termination: 0x0:0x2
>
> SNP guest fails with the below error message:
>
> KVM: unknown exit reason 24
> EAX=00000000 EBX=00000000 ECX=00000000 EDX=00a00f11
> ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
> EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 00000000 0000ffff 00009300
> CS =f000 ffff0000 0000ffff 00009b00
> SS =0000 00000000 0000ffff 00009300
> DS =0000 00000000 0000ffff 00009300
> FS =0000 00000000 0000ffff 00009300
> GS =0000 00000000 0000ffff 00009300
> LDT=0000 00000000 0000ffff 00008200
> TR =0000 00000000 0000ffff 00008b00
> GDT= 00000000 0000ffff
> IDT= 00000000 0000ffff
> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
> DR6=00000000ffff0ff0 DR7=0000000000000400
> EFER=0000000000000000
>
> Fixes: 4af663c2f64a ("KVM: SEV: Allow per-guest configuration of GHCB protocol version")
> Cc: Thomas Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Michael Roth <michael.roth@xxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
You kept the stable email.
Minor comment below about placement, but otherwise...
Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
>
> ---
>
> Changes since v1:
> * Add failure logs in the commit and drop @stable tag (Sean)
> ---
> arch/x86/kvm/svm/sev.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 95668e84ab86..fdc1309c68cb 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -406,6 +406,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
> struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> struct sev_platform_init_args init_args = {0};
> bool es_active = vm_type != KVM_X86_SEV_VM;
> + bool snp_active = vm_type == KVM_X86_SNP_VM;
> u64 valid_vmsa_features = es_active ? sev_supported_vmsa_features : 0;
> int ret;
>
> @@ -424,6 +425,9 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
> if (unlikely(sev->active))
> return -EINVAL;
>
> + if (snp_active && data->ghcb_version && data->ghcb_version < 2)
> + return -EINVAL;
> +
Would it make sense to move this up a little bit so that it follows the
other ghcb_version check? This way the checks are grouped.
Thanks,
Tom
> sev->active = true;
> sev->es_active = es_active;
> sev->vmsa_features = data->vmsa_features;
> @@ -437,7 +441,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
> if (sev->es_active && !sev->ghcb_version)
> sev->ghcb_version = GHCB_VERSION_DEFAULT;
>
> - if (vm_type == KVM_X86_SNP_VM)
> + if (snp_active)
> sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>
> ret = sev_asid_new(sev);
> @@ -455,7 +459,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
> }
>
> /* This needs to happen after SEV/SNP firmware initialization. */
> - if (vm_type == KVM_X86_SNP_VM) {
> + if (snp_active) {
> ret = snp_guest_req_init(kvm);
> if (ret)
> goto e_free;
>
> base-commit: 772d50d9b87bec08b56ecee0a880d6b2ee5c7da5
