Re: [PATCH v2] x86/hvm: Advertise and support extended destination IDs for MSI/IO-APIC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2026 09:46, Julian Vetter wrote:
> x2APIC guests with more than 128 vCPUs have APIC IDs above 255, but MSI
> addresses and IO-APIC RTEs only provide an 8-bit destination field.
> Without extended destination ID support, Linux limits the maximum usable
> APIC ID to 255, refusing to bring up vCPUs beyond that limit. So,
> advertise XEN_HVM_CPUID_EXT_DEST_ID in the HVM hypervisor CPUID leaf,
> signalling that guests may use MSI address bits 11:5 and IO-APIC RTE
> bits 55:49 as additional high destination ID bits. This expands the
> destination ID from 8 to 15 bits.

Should MSI and IO-APIC really be covered by just a single bit?

> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -374,7 +374,16 @@ int hvm_set_pci_link_route(struct domain *d, u8 link, u8 
> isa_irq)
>  int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data)
>  {
>      uint32_t tmp = (uint32_t) addr;
> -    uint8_t  dest = (tmp & MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT;
> +    /*
> +     * Standard MSI destination address bits 19:12 (8 bits).
> +     * Extended MSI destination address bits 11:5 (7 more bits).
> +     *
> +     * As XEN_HVM_CPUID_EXT_DEST_ID is advertised, the guest may use bits 
> 11:5
> +     * for high destination ID bits, expanding to 15 bits total. Guests 
> unaware
> +     * of this feature set these bits to 0, so this is backwards-compatible.

Is it? Any of the bits being non-zero would presently be ignored, afaics.
I think this needs to be two-way negotiation: Xen advertises the capability,
and the guest has to actively opt in. (Didn't Roger say something along these
lines already?) Also don't you need to restrict use of the wider IDs to
vCPU-s actually running in x2APIC mode? (And what is going to happen if a
vCPU switches back to legacy mode?)

> +     */
> +    uint32_t dest = (MASK_EXTR(tmp, MSI_ADDR_EXT_DEST_ID_MASK) << 8) |

The literal 8 here and again ...

> @@ -411,7 +413,8 @@ static void ioapic_inj_irq(
>  
>  static void vioapic_deliver(struct hvm_vioapic *vioapic, unsigned int pin)
>  {
> -    uint16_t dest = vioapic->redirtbl[pin].fields.dest_id;
> +    uint32_t dest = ((uint32_t)vioapic->redirtbl[pin].fields.ext_dest_id << 
> 8) |

... here also would want dealing with, I guess. Use of literal numbers really
should be an exception.

> @@ -594,6 +597,49 @@ int vioapic_get_trigger_mode(const struct domain *d, 
> unsigned int gsi)
>      return vioapic->redirtbl[pin].fields.trig_mode;
>  }
>  
> +static int cf_check ioapic_check(const struct domain *d, 
> hvm_domain_context_t *h)
> +{
> +    const HVM_SAVE_TYPE(IOAPIC) *s = hvm_get_entry(IOAPIC, h);
> +    unsigned int i;
> +
> +    if ( !s )
> +        return -ENODATA;
> +
> +    for ( i = 0; i < ARRAY_SIZE(s->redirtbl); i++ )
> +    {
> +        struct cpuid_leaf res;
> +
> +        /* ext_dest_id bits not set in RTE so continue */
> +        if ( !s->redirtbl[i].fields.ext_dest_id )
> +            continue;
> +
> +        /*
> +         * An RTE in the saved state has ext_dest_id bits set, verify the
> +         * destination XEN advertises XEN_HVM_CPUID_EXT_DEST_ID to the guest.
> +         * If not interrupt routing to APIC IDs > 255 would be broken after
> +         * restore -> -EINVAL!
> +         */
> +        guest_cpuid(d->vcpu[0], 0x40000004, 0, &res);
> +        if ( !(res.a & XEN_HVM_CPUID_EXT_DEST_ID) )
> +        {
> +            printk(XENLOG_G_ERR "HVM restore: dom%d IO-APIC RTE %u has "
> +                                "extended destination ID bits set but "
> +                                "XEN_HVM_CPUID_EXT_DEST_ID is not 
> advertised\n",
> +                                d->domain_id, i);
> +            return -EINVAL;
> +        }
> +
> +        /*
> +         * Found an RTE with ext_dest bits set, but the destination XEN also
> +         * correctly announces XEN_HVM_CPUID_EXT_DEST_ID
> +         * -> All good, no need to check remaining RTEs.
> +         */
> +        break;
> +    }
> +
> +    return 0;
> +}

As indicated before, I think there wants to be a prereq patch introducing
ioapic_check() (preferably covering whatever other bits as well), with the
patch here then merely making adjustments to the function.

I also don't think you want to call guest_cpuid() here. You set the bit
unconditionally there, so no need to check here. (If it was conditionally
set there, the same condition would want using here. I think you will be
able to find examples of this for some of the other bits.)

Also in new code please no dom%d anymore. We have had %pd for quite some
time.

> --- a/xen/arch/x86/include/asm/msi.h
> +++ b/xen/arch/x86/include/asm/msi.h
> @@ -54,6 +54,8 @@
>  #define       MSI_ADDR_DEST_ID_MASK          0x00ff000
>  #define  MSI_ADDR_DEST_ID(dest)              (((dest) << 
> MSI_ADDR_DEST_ID_SHIFT) & MSI_ADDR_DEST_ID_MASK)
>  
> +#define MSI_ADDR_EXT_DEST_ID_MASK    0x0000fe0

As this isn't part of any architectural spec, it wants a comment as to
its origin.

> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -359,7 +359,9 @@ union vioapic_redir_entry
>          uint8_t trig_mode:1;
>          uint8_t mask:1;
>          uint8_t reserve:7;
> -        uint8_t reserved[4];
> +        uint8_t reserved[3];
> +        uint8_t ext_dest_rsvd:1;

Why ext_dest_ as a prefix? Does this field need naming at all?

Jan