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

[Kim Phillips <kim.phillips@amd.com>]

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