Re: [PATCH v4 2/2] KVM: SEV: Configure "ALLOWED_SEV_FEATURES" VMCB Field

[Kim Phillips <kim.phillips@xxxxxxx>]

On 3/6/25 1:27 PM, Tom Lendacky wrote:
> On 3/5/25 18:38, Kim Phillips wrote:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 0bc708ee2788..7f6cb950edcf 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -793,6 +793,14 @@ static int sev_launch_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
> >   	return ret;
> >   }
> >   
> > +static u64 allowed_sev_features(struct kvm_sev_info *sev)
> > +{
> > +	if (cpu_feature_enabled(X86_FEATURE_ALLOWED_SEV_FEATURES))
> > +		return sev->vmsa_features | VMCB_ALLOWED_SEV_FEATURES_VALID;
> > +
> > +	return 0;
> > +}
> > +
> >   static int sev_es_sync_vmsa(struct vcpu_svm *svm)
> >   {
> >   	struct kvm_vcpu *vcpu = &svm->vcpu;
> > @@ -891,6 +899,7 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
> >   static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
> >   				    int *error)
> >   {
> > +	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> >   	struct sev_data_launch_update_vmsa vmsa;
> >   	struct vcpu_svm *svm = to_svm(vcpu);
> >   	int ret;
> > @@ -900,6 +909,8 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
> >   		return -EINVAL;
> >   	}
> >   
> > +	svm->vmcb->control.allowed_sev_features = allowed_sev_features(sev);
>
> I think you can move this to sev_es_init_vmcb() and have it just in that
> one place instead of each launch update routine.

Agreed.  I'll remove the above and add the following in the next version:

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0bc708ee2788..f9ec139901ef 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4449,6 +4449,7 @@ void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm)
 
 static void sev_es_init_vmcb(struct vcpu_svm *svm)
 {
+       struct kvm_sev_info *sev = to_kvm_sev_info(svm->vcpu.kvm);
        struct vmcb *vmcb = svm->vmcb01.ptr;
        struct kvm_vcpu *vcpu = &svm->vcpu;
 
@@ -4464,6 +4465,10 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
        if (svm->sev_es.vmsa && !svm->sev_es.snp_has_guest_vmsa)
                svm->vmcb->control.vmsa_pa = __pa(svm->sev_es.vmsa);
 
+       if (cpu_feature_enabled(X86_FEATURE_ALLOWED_SEV_FEATURES))
+               svm->vmcb->control.allowed_sev_features = sev->vmsa_features |
+                                                         VMCB_ALLOWED_SEV_FEATURES_VALID;
+
        /* Can't intercept CR register access, HV can't modify CR registers */
        svm_clr_intercept(svm, INTERCEPT_CR0_READ);
        svm_clr_intercept(svm, INTERCEPT_CR4_READ);

Thanks for your review!

Kim