Re: [RFC PATCH 10/24] KVM: SVM: Use a single ASID per VM

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

On Thu, Apr 03, 2025 at 04:05:41PM -0400, Maxim Levitsky wrote:
> On Wed, 2025-03-26 at 19:36 +0000, Yosry Ahmed wrote:
[..]
> > @@ -5481,6 +5498,12 @@ static __init int svm_hardware_setup(void)
> >  			goto err;
> >  	}
> >  
> > +	fallback_asid = kvm_tlb_tags_alloc(&svm_asids);
> > +	WARN_ON_ONCE(!fallback_asid);
> 
> Nitpick: This really can't happen unless there is some very bad bug lurking somewhere.
> And if this happens, nothing will work, since likely that regular ASID allocation
> will fail too.
> 
> So why not to fail svm_hardware_setup instead?

Yeah I can do that.

> 
> 
> > +
> > +	/* Needs to be after svm_cpu_init() initializes the per-CPU xarrays */
> > +	svm_register_asid(fallback_asid);
> > +
> >  	enable_apicv = avic = avic && avic_hardware_setup();
> >  
> >  	if (!enable_apicv) {
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 4929b96d3d700..436b7e83141b9 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -117,6 +117,8 @@ struct kvm_sev_info {
> >  struct kvm_svm {
> >  	struct kvm kvm;
> >  
> > +	unsigned int asid;
> > +
> >  	/* Struct members for AVIC */
> >  	u32 avic_vm_id;
> >  	struct page *avic_logical_id_table_page;
> > @@ -132,7 +134,6 @@ struct kvm_vmcb_info {
> >  	struct vmcb *ptr;
> >  	unsigned long pa;
> >  	int cpu;
> > -	uint64_t asid_generation;
> >  };
> >  
> >  struct vmcb_save_area_cached {
> > @@ -247,7 +248,6 @@ struct vcpu_svm {
> >  	struct vmcb *vmcb;
> >  	struct kvm_vmcb_info vmcb01;
> >  	struct kvm_vmcb_info *current_vmcb;
> > -	u32 asid;
> >  	u32 sysenter_esp_hi;
> >  	u32 sysenter_eip_hi;
> >  	uint64_t tsc_aux;
> > @@ -330,11 +330,6 @@ struct vcpu_svm {
> >  };
> >  
> >  struct svm_cpu_data {
> > -	u64 asid_generation;
> > -	u32 max_asid;
> > -	u32 next_asid;
> > -	u32 min_asid;
> > -
> >  	struct vmcb *save_area;
> >  	unsigned long save_area_pa;
> >  
> > @@ -656,6 +651,7 @@ void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
> >  				     int trig_mode, int vec);
> >  bool svm_register_asid(unsigned int asid);
> >  void svm_unregister_asid(unsigned int asid);
> > +unsigned int svm_asid(struct kvm *kvm);
> >  
> >  /* nested.c */
> >  
> 
> Overall looks good to me.
> 
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Thanks!

> 
> Best regards,
> 	Maxim Levitsky
> 
> 
>