Re: [PATCH v3 03/27] KVM: VMX: Generalize VPID allocation to be vendor-neutral

[Jim Mattson]

On Mon, Oct 12, 2026 at 4:36 PM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> In preparation for sharing with SVM, generalize the VMX VPID allocation
> code and move it to common code as a TLB tags allocator. Parameterize
> the TLB tags allocator by the number of tags, and allocate the bitmap
> dynamically. Opportunisitcally use guards to acquire the lock instead of

s/Opportunisitcally/Opportunistically/

> spin_{lock/unlock}().
>
> Keep the number of allowed tags capped at VMX's hardware cap, to avoid
> allocating a huge bitmap if L0 advertises a huge number of ASIDs to an
> L1 KVM. Realistically, the number of actual hardware ASIDs wouldn't be
> that large so there is no benefit.
>
> Initialize the TLB tags allocator during hardware setup/unsetup, and
> reserve tag=0 during initialziation, similar to how VPID=0 is currently

s/initialziation/initialization/

> reserved in the VMX-specific bitmap during hardware setup.
>
> Allow nr=0 to allow allocating all ASIDs to SEV on AMD without failing
> hardware setup, if at all possible, in which case any tag allocation
> fails (SEV won't use the tag allocator).  The number of tags includes
> tag=0, which is not usable. The interface is a little confusing in that
> regard, but this will be changed soon when reserved tags are explicitly
> introduced.
>
> Keep allocate_vpid() and free_vpid() as wrapper that check enable_vpid
> to avoid checking at all callsites, and add init_vpids() and
> destroy_vpids() to wrap init/destroy calls as well.
>
> No functional change intended.

I don't think that's really true. If it is, then you failed. For
example, the new -ENOMEM when the bitmap can't be dynamically
allocated.

> Signed-off-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> ---
>  arch/x86/kvm/mmu.h     |  8 +++++
>  arch/x86/kvm/mmu/mmu.c | 78 ++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/vmx/vmx.c | 40 +++++-----------------
>  arch/x86/kvm/vmx/vmx.h | 28 ++++++++++++---
>  4 files changed, 119 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 2ae7f9ed4cf86..de79e002edf8f 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -410,4 +410,12 @@ static inline bool kvm_is_gfn_alias(struct kvm *kvm, gfn_t gfn)
>  {
>         return gfn & kvm_gfn_direct_bits(kvm);
>  }
> +
> +typedef unsigned int kvm_tlb_tag_t;
> +
> +int kvm_init_tlb_tags(unsigned int nr);
> +void kvm_destroy_tlb_tags(void);
> +kvm_tlb_tag_t kvm_alloc_tlb_tag(void);
> +void kvm_free_tlb_tag(kvm_tlb_tag_t tag);
> +
>  #endif
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index aadd5bdc2c97a..97d49247d9d82 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -8178,6 +8178,84 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)
>                 vhost_task_stop(kvm->arch.nx_huge_page_recovery_thread);
>  }
>
> +static struct {
> +       spinlock_t      lock;
> +       unsigned long   *bitmap;
> +       unsigned int    nr;
> +} tlb_tags;

Can we statically initialize this to avoid the call to spin_lock_init()?

... tlb_tags = {
        .lock = __SPIN_LOCK_UNLOCKED(tlb_tags.lock),
};

> +
> +int kvm_init_tlb_tags(unsigned int nr)
> +{
> +       /*
> +        * Limit the number of TLB tags to VMX's hardcoded maximum of 0x10000
> +        * to avoid wasting memory for the bitmap in the unlikely scenario the
> +        * CPU supports an inordinate number of ASIDs (on AMD).  If userspace
> +        * wants to concurrently run tens of thousands of vCPUs, they'll likely
> +        * need a solution that works for both Intel and AMD.
> +        */
> +       const unsigned int MAX_NR_TLB_TAGS = VMX_NR_VPIDS;
> +
> +       if (!nr)
> +               return 0;
> +
> +       if (nr > MAX_NR_TLB_TAGS) {
> +               pr_warn_once("Number of TLB tags capped (%u instead of %u)\n",
> +                            MAX_NR_TLB_TAGS, nr);
> +               nr = MAX_NR_TLB_TAGS;
> +       }
> +
> +       tlb_tags.bitmap = bitmap_zalloc(nr, GFP_KERNEL);
> +       if (!tlb_tags.bitmap)
> +               return -ENOMEM;
> +
> +       /*
> +        * 0 is the host's TLB tag for both VMX's VPID and SVM's ASID, and is
> +        * returned on failed allocations (e.g. no more tags left).
> +        */
> +       __set_bit(0, tlb_tags.bitmap);
> +
> +       tlb_tags.nr = nr;
> +       spin_lock_init(&tlb_tags.lock);
> +       return 0;
> +}
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_init_tlb_tags);
> +
> +void kvm_destroy_tlb_tags(void)
> +{
> +       bitmap_free(tlb_tags.bitmap);
> +       tlb_tags.bitmap = NULL;

For consistency and future-proofing, it might be wise to clear tlb_tags.nr here.

> +}
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_destroy_tlb_tags);