Re: [PATCH v4 4/5] KVM: SEV: Introduce new min,max sev_es and sev_snp asid variables

[Tom Lendacky <thomas.lendacky@amd.com>]

On 5/19/25 18:57, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.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@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.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

Thanks,
Tom

>  >  	sev_enabled = sev_supported;
>  	sev_es_enabled = sev_es_supported;