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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aAdmrsQcmEQeNnXb@Asmaa.>
Date: Tue, 22 Apr 2025 02:51:42 -0700
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Jim Mattson <jmattson@...gle.com>,
	Vitaly Kuznetsov <vkuznets@...hat.com>,
	Rik van Riel <riel@...riel.com>,
	Tom Lendacky <thomas.lendacky@....com>, x86@...nel.org,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 10/24] KVM: SVM: Use a single ASID per VM

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@...hat.com>

Thanks!

> 
> Best regards,
> 	Maxim Levitsky
> 
> 
>