Re: [RFC PATCH v2 4/5] x86/hvm: Transition to needs_tlb_flush logic, use per-domain ASID

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.09.2026 14:51, Teddy Astie wrote:
> Le 18/09/2026 à 18:30, Ross Lagerwall a écrit :
>> On 7/31/26 3:46 PM, Teddy Astie wrote:
>>> --- a/xen/arch/x86/hvm/asid.c
>>> +++ b/xen/arch/x86/hvm/asid.c
>>> @@ -5,138 +5,115 @@
>>>    * Copyright (c) 2009, Citrix Systems, Inc.
>>>    */
>>> +#include <xen/errno.h>
>>>   #include <xen/init.h>
>>>   #include <xen/lib.h>
>>>   #include <xen/param.h>
>>> -#include <xen/sched.h>
>>> -#include <xen/smp.h>
>>> -#include <xen/percpu.h>
>>> +#include <xen/spinlock.h>
>>> +#include <xen/xvmalloc.h>
>>> +
>>> +#include <asm/bitops.h>
>>>   #include <asm/hvm/asid.h>
>>>   /* Xen command-line option to enable ASIDs */
>>>   static bool __read_mostly opt_asid_enabled = true;
>>>   boolean_param("asid", opt_asid_enabled);
>>> +bool __read_mostly asid_enabled = false;
>>> +static unsigned long __ro_after_init *asid_bitmap;
>>> +static unsigned long __ro_after_init asid_count;
>>> +static DEFINE_SPINLOCK(asid_lock);
>>> +
>>>   /*
>>> - * ASIDs partition the physical TLB.  In the current implementation 
>>> ASIDs are
>>> - * introduced to reduce the number of TLB flushes.  Each time the 
>>> guest's
>>> - * virtual address space changes (e.g. due to an INVLPG, MOV-TO-{CR3, 
>>> CR4}
>>> - * operation), instead of flushing the TLB, a new ASID is assigned.  
>>> This
>>> - * reduces the number of TLB flushes to at most 1/#ASIDs.  The biggest
>>> - * advantage is that hot parts of the hypervisor's code and data 
>>> retain in
>>> - * the TLB.
>>> - *
>>>    * Sketch of the Implementation:
>>> + * ASIDs are assigned uniquely per domain and doesn't change during 
>>> the lifecycle of the
>>> + * domain. Once vcpus are initialized and are up, we assign the same 
>>> ASID to all vcpus
>>> + * of that domain at the first VMRUN. In order to process a TLB flush 
>>> on a vcpu, we set
>>> + * needs_tlb_flush to schedule a TLB flush for the next VMRUN (e.g 
>>> using tlb control
>>> + * field of VMCB).
>>>    *
>>> - * ASIDs are a CPU-local resource.  As preemption of ASIDs is not 
>>> possible,
>>> - * ASIDs are assigned in a round-robin scheme.  To minimize the 
>>> overhead of
>>> - * ASID invalidation, at the time of a TLB flush,  ASIDs are tagged 
>>> with a
>>> - * 64-bit generation.  Only on a generation overflow the code needs to
>>> - * invalidate all ASID information stored at the VCPUs with are run 
>>> on the
>>> - * specific physical processor.  This overflow appears after about 2^80
>>> - * host processor cycles, so we do not optimize this case, but simply 
>>> disable
>>> - * ASID useage to retain correctness.
>>> + * We reserve ASID=1 as being the ASID used when none other is 
>>> available (or with asid
>>> + * use disabled). Multiples domains may use this ASID, thus we need 
>>> to systematically
>>> + * flush the TLB for this one when switching between vCPUs with ASID=1.
>>>    */
>>> -/* Per-CPU ASID management. */
>>> -struct hvm_asid_data {
>>> -   uint64_t core_asid_generation;
>>> -   uint32_t next_asid;
>>> -   uint32_t max_asid;
>>> -   bool disabled;
>>> -};
>>> -
>>> -static DEFINE_PER_CPU(struct hvm_asid_data, hvm_asid_data);
>>> -
>>> -void hvm_asid_init(unsigned int nasids)
>>> +int __init hvm_asid_init(unsigned long nasids)
>>>   {
>>> -    static int8_t __ro_after_init g_disabled = -1;
>>> -    struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
>>> +    ASSERT(nasids);
>>> -    data->max_asid = nasids - 1;
>>> -    data->disabled = !opt_asid_enabled || (nasids <= 1);
>>> +    asid_count = nasids;
>>> +    asid_enabled = opt_asid_enabled && (nasids > 1);
>>> -    if ( g_disabled < 0 )
>>> -    {
>>> -        g_disabled = data->disabled;
>>> -        printk("HVM: ASIDs %sabled\n", data->disabled ? "dis" : "en");
>>> -    }
>>> -    else if ( g_disabled != data->disabled )
>>> -        printk("HVM: CPU%u: ASIDs %sabled\n", smp_processor_id(),
>>> -               data->disabled ? "dis" : "en");
>>> +    asid_bitmap = xvzalloc_array(unsigned long, 
>>> BITS_TO_LONGS(asid_count + 1));
>>> +    if ( !asid_bitmap )
>>> +        return -ENOMEM;
>>
>> Should there be a sanity check to avoid an excessive allocation? E.g. If
>> running under another hypervisor, it might set nasids to ~0 while with 
>> one per
>> domain we need no more than ~64k.
> 
> Indeed, I didn't consider that AMD had (and enumerates) 32-bits ASIDs. 
> Only Intel uses 16-bits VPIDs.
> 
> Limiting to 64k seems wise,

Why 64k, when there can be at most 32k domains?

> the remaining question is that I'm not sure 
> if we want to make that configurable.

Unless there are significant savings to be had, I'd say stick to the smaller
of what hardware offers and 32k.

Jan