Re: [RFC PATCH 23/24] KVM: nSVM: Allocate a new ASID for nested guests

[Maxim Levitsky]

On Wed, 2025-03-26 at 19:44 +0000, Yosry Ahmed wrote:
> Now that nested TLB flushes are properly tracked, start allocating a
> separate ASID for nested guests. This allows dropping the unconditional
> TLB flushes on nested transitions and doing finer grained TLB flushing
> when necessary.
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@xxxxxxxxx>
> ---
>  arch/x86/kvm/svm/nested.c | 11 +++++++++--
>  arch/x86/kvm/svm/svm.c    |  5 +++--
>  arch/x86/kvm/svm/svm.h    |  3 +++
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 544913461693c..0c887c91bd50d 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1204,6 +1204,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)
>  {
>  	struct kvm_svm *kvm_svm = to_kvm_svm(svm->vcpu.kvm);
>  	struct page *vmcb02_page;
> +	unsigned int asid;
>  
>  	if (svm->nested.initialized)
>  		return 0;
> @@ -1221,8 +1222,14 @@ int svm_allocate_nested(struct vcpu_svm *svm)
>  
>  	svm->nested.initialized = true;
>  
> -	if (!kvm_svm->nested_asid)
> -		kvm_svm->nested_asid = kvm_svm->asid;
> +	if (!kvm_svm->nested_asid) {
> +		asid = kvm_tlb_tags_alloc(&svm_asids);
> +		if (asid && !svm_register_asid(asid)) {
> +			kvm_tlb_tags_free(&svm_asids, asid);
> +			asid = 0;
> +		}
> +		kvm_svm->nested_asid = asid ?: fallback_asid;
> +	}

Nitpick: AFAIK at least nested KVM doesn't enable EFER.SVME,
unless it actually runs a guest thus most of the time we will waste a ASID on a VM
which once did run a VM nested and since then doesn't run anything else.

So maybe we want to free the nested ASID in the svm_free_nested?

>  
>  	return 0;
>  
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 4b95fd6b501e6..196f5bca57a0e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -249,8 +249,8 @@ static unsigned long iopm_base;
>  
>  DEFINE_PER_CPU(struct svm_cpu_data, svm_data);
>  
> -static struct kvm_tlb_tags svm_asids;
> -static unsigned int fallback_asid;
> +struct kvm_tlb_tags svm_asids;
> +unsigned int fallback_asid;
>  
>  /*
>   * Only MSR_TSC_AUX is switched via the user return hook.  EFER is switched via
> @@ -5127,6 +5127,7 @@ static void svm_vm_destroy(struct kvm *kvm)
>  	avic_vm_destroy(kvm);
>  	sev_vm_destroy(kvm);
>  	kvm_tlb_tags_free(&svm_asids, kvm_svm->asid);
> +	kvm_tlb_tags_free(&svm_asids, kvm_svm->nested_asid);
>  }
>  
>  static int svm_vm_init(struct kvm *kvm)
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 0c44133bc05ca..220d10d2b1a5c 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -630,6 +630,9 @@ static inline void svm_vmgexit_no_action(struct vcpu_svm *svm, u64 data)
>  
>  extern bool dump_invalid_vmcb;
>  
> +extern struct kvm_tlb_tags svm_asids;
> +extern unsigned int fallback_asid;
> +
>  u32 svm_msrpm_offset(u32 msr);
>  u32 *svm_vcpu_alloc_msrpm(void);
>  void svm_vcpu_init_msrpm(struct kvm_vcpu *vcpu, u32 *msrpm);


Best regards,
	Maxim Levitsky