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@xxxxxxxxxx>
Thanks!
>
> Best regards,
> Maxim Levitsky
>
>
>
