linux-kernel - Re: [PATCH v4 4/5] KVM: SEV: Introduce new min,max sev_es and 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f4f9a3cb-eb8e-4d7d-8fe1-5cceca5507c6@amd.com>
Date: Wed, 4 Jun 2025 18:12:23 -0500
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Tom Lendacky <thomas.lendacky@....com>, seanjc@...gle.com,
 pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, hpa@...or.com, herbert@...dor.apana.org.au
Cc: x86@...nel.org, john.allen@....com, davem@...emloft.net,
 michael.roth@....com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-crypto@...r.kernel.org
Subject: Re: [PATCH v4 4/5] KVM: SEV: Introduce new min,max sev_es and sev_snp
 asid variables


On 6/3/2025 10:52 AM, Tom Lendacky wrote:
> On 5/19/25 18:57, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@....com>
>>
>> Introduce new min, max sev_es_asid and sev_snp_asid variables.
>>
>> The new {min,max}_{sev_es,snp}_asid variables along with existing
>> {min,max}_sev_asid variable simplifies partitioning of the
>> SEV and SEV-ES+ ASID space.
>>
>> Suggested-by: Sean Christopherson <seanjc@...gle.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
>> ---
>>  arch/x86/kvm/svm/sev.c | 37 ++++++++++++++++++++++++++++---------
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index dea9480b9ff6..383db1da8699 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -85,6 +85,10 @@ static DECLARE_RWSEM(sev_deactivate_lock);
>>  static DEFINE_MUTEX(sev_bitmap_lock);
>>  unsigned int max_sev_asid;
>>  static unsigned int min_sev_asid;
>> +static unsigned int max_sev_es_asid;
>> +static unsigned int min_sev_es_asid;
>> +static unsigned int max_snp_asid;
>> +static unsigned int min_snp_asid;
>>  static unsigned long sev_me_mask;
>>  static unsigned int nr_asids;
>>  static unsigned long *sev_asid_bitmap;
>> @@ -172,20 +176,32 @@ static void sev_misc_cg_uncharge(struct kvm_sev_info *sev)
>>  	misc_cg_uncharge(type, sev->misc_cg, 1);
>>  }
>>  
>> -static int sev_asid_new(struct kvm_sev_info *sev)
>> +static int sev_asid_new(struct kvm_sev_info *sev, unsigned long vm_type)
>>  {
>>  	/*
>>  	 * SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
>>  	 * SEV-ES-enabled guest can use from 1 to min_sev_asid - 1.
>> -	 * Note: min ASID can end up larger than the max if basic SEV support is
>> -	 * effectively disabled by disallowing use of ASIDs for SEV guests.
>>  	 */
>> -	unsigned int min_asid = sev->es_active ? 1 : min_sev_asid;
>> -	unsigned int max_asid = sev->es_active ? min_sev_asid - 1 : max_sev_asid;
>> -	unsigned int asid;
>> +	unsigned int min_asid, max_asid, asid;
>>  	bool retry = true;
>>  	int ret;
>>  
>> +	if (vm_type == KVM_X86_SNP_VM) {
>> +		min_asid = min_snp_asid;
>> +		max_asid = max_snp_asid;
>> +	} else if (sev->es_active) {
>> +		min_asid = min_sev_es_asid;
>> +		max_asid = max_sev_es_asid;
>> +	} else {
>> +		min_asid = min_sev_asid;
>> +		max_asid = max_sev_asid;
>> +	}
>> +
>> +	/*
>> +	 * The min ASID can end up larger than the max if basic SEV support is
>> +	 * effectively disabled by disallowing use of ASIDs for SEV guests.
>> +	 */
>> +
> 
> Remove blank line.
> 
>>  	if (min_asid > max_asid)
>>  		return -ENOTTY;
>>  
>> @@ -439,7 +455,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>>  	if (vm_type == KVM_X86_SNP_VM)
>>  		sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>>  
>> -	ret = sev_asid_new(sev);
>> +	ret = sev_asid_new(sev, vm_type);
>>  	if (ret)
>>  		goto e_no_asid;
>>  
>> @@ -3029,6 +3045,9 @@ void __init sev_hardware_setup(void)
>>  		goto out;
>>  	}
>>  
>> +	min_sev_es_asid = min_snp_asid = 1;
>> +	max_sev_es_asid = max_snp_asid = min_sev_asid - 1;
> 
> Should these be moved to after the min_sev_asid == 1 check ...
> 
>> +
>>  	/* Has the system been allocated ASIDs for SEV-ES? */
>>  	if (min_sev_asid == 1)
>>  		goto out;
>> @@ -3048,11 +3067,11 @@ void __init sev_hardware_setup(void)
>>  	if (boot_cpu_has(X86_FEATURE_SEV_ES))
>>  		pr_info("SEV-ES %s (ASIDs %u - %u)\n",
>>  			str_enabled_disabled(sev_es_supported),
>> -			min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
>> +			min_sev_es_asid, max_sev_es_asid);
> 
> ... so that this becomes 0 and 0 if min_sev_asid == 1 ? (like before)
> 
>>  	if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>>  		pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
>>  			str_enabled_disabled(sev_snp_supported),
>> -			min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
>> +			min_snp_asid, max_snp_asid);
> 
> Ditto
>

Yes that makes sense.

Thanks,
Ashish

 
> Thanks,
> Tom
> 
>>  >  	sev_enabled = sev_supported;
>>  	sev_es_enabled = sev_es_supported;