Re: [PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Yosry Ahmed]

On Mon, Mar 17, 2025 at 02:44:54PM -0700, Jim Mattson wrote:
> On Mon, Mar 17, 2025 at 2:12 PM Yosry Ahmed <yosry.ahmed@xxxxxxxxx> wrote:
> >
> > On Thu, Mar 13, 2025 at 09:55:39PM +0000, Yosry Ahmed wrote:
> > > The ASID generation and dynamic ASID allocation logic is now only used
> > > by initialization the generation to 0 to trigger a new ASID allocation
> > > per-vCPU on the first VMRUN, so the ASID is more-or-less static
> > > per-vCPU.
> > >
> > > Moreover, having a unique ASID per-vCPU is not required. ASIDs are local
> > > to each physical CPU, and the ASID is flushed when a vCPU is migrated to
> > > a new physical CPU anyway. SEV VMs have been using a single ASID per VM
> > > already for other reasons.
> > >
> > > Use a static ASID per VM and drop the dynamic ASID allocation logic. The
> > > ASID is allocated during vCPU reset (SEV allocates its own ASID), and
> > > the ASID is always flushed on first use in case it was used by another
> > > VM previously.
> > >
> > > On VMRUN, WARN if the ASID in the VMCB does not match the VM's ASID, and
> > > update it accordingly. Also, flush the ASID on every VMRUN if the VM
> > > failed to allocate a unique ASID. This would probably wreck performance
> > > if it happens, but it should be an edge case as most AMD CPUs have over
> > > 32k ASIDs.
> > >
> > > Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > Signed-off-by: Yosry Ahmed <yosry.ahmed@xxxxxxxxx>
> > > ---
> > [..]
> > > @@ -3622,7 +3613,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
> > >
> > >  static int pre_svm_run(struct kvm_vcpu *vcpu)
> > >  {
> > > -     struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
> > > +     struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
> > >       struct vcpu_svm *svm = to_svm(vcpu);
> > >
> > >       /*
> > > @@ -3639,9 +3630,15 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > >       if (sev_guest(vcpu->kvm))
> > >               return pre_sev_run(svm, vcpu->cpu);
> > >
> > > -     /* FIXME: handle wraparound of asid_generation */
> > > -     if (svm->current_vmcb->asid_generation != sd->asid_generation)
> > > -             new_asid(svm, sd);
> > > +     /* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
> > > +     if (unlikely(!kvm_svm->asid))
> > > +             svm_vmcb_set_flush_asid(svm->vmcb);
> >
> > This is wrong. I thought we can handle ASID allocation failures by just
> > reusing ASID=0 and flushing it on every VMRUN, but using ASID=0 is
> > illegal according to the APM. Also, in this case we also need to flush
> > the ASID on every VM-exit, which I failed to do here.
> >
> > There are two ways to handle running out of ASIDs:
> >
> > (a) Failing to create the VM. This will impose a virtual limit on the
> > number of VMs that can be run concurrently. The number of ASIDs was
> > quite high on the CPUs I checked (2^15 IIRC), so it's probably not
> > an issue, but I am not sure if this is considered breaking userspace.
> 
> I'm pretty sure AMD had only 6 bits of ASID through at least Family
> 12H. At some point, VMCB ASID bits must have become decoupled from
> physical TLB tag bits. 15 TLB tag bits is inconceivable!

I checked on Rome, Milan, Genoa, and Turin. For leaf 8000000a, the value
of EBX is 00008000 for all of them. That's 32767 ASIDs for VMs (1 for
host, and ignoring SEV). I don't have access to older hardware to check,
but 6 bits would be 63 ASIDs for VMs. I imagine such a constraint may be
less acceptable, so having a single fallback ASID may be the way to go?

> 
> > (b) Designating a specific ASID value as the "fallback ASID". This value
> > would be used by any VMs created after running out of ASIDs, and we
> > flush it on every VMRUN, similar to what I am trying to do here for
> > ASID=0.
> >
> > Any thoughts on which way we should take? (a) is simpler if we can get
> > away with it and all AMD CPUs have a sufficiently large number of ASIDs.
> >