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

[Yosry Ahmed <yosry.ahmed@linux.dev>]

On Thu, Apr 03, 2025 at 04:11:47PM -0400, Maxim Levitsky wrote:
> 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@linux.dev>
> > ---
> >  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.

Oh yeah, I missed that, thanks. Will do.

> 
> 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
> 
> 
>