linux-kernel - Re: [PATCH v4 2/2] KVM: SEV: Configure "ALLOWED_SEV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fd3c11f3-42b5-45e9-b827-d9047a246e59@amd.com>
Date: Thu, 6 Mar 2025 17:04:07 -0600
From: Kim Phillips <kim.phillips@....com>
To: Tom Lendacky <thomas.lendacky@....com>, <kvm@...r.kernel.org>,
	<linux-coco@...ts.linux.dev>, <linux-kernel@...r.kernel.org>
CC: Michael Roth <michael.roth@....com>, Ashish Kalra <ashish.kalra@....com>,
	"Nikunj A . Dadhania" <nikunj@....com>, Borislav Petkov <bp@...en8.de>, "Dave
 Hansen" <dave.hansen@...ux.intel.com>, Sean Christopherson
	<seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, Ingo Molnar
	<mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner
	<tglx@...utronix.de>, Kishon Vijay Abraham I <kvijayab@....com>
Subject: Re: [PATCH v4 2/2] KVM: SEV: Configure "ALLOWED_SEV_FEATURES" VMCB
 Field

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