Re: [PATCH v3 3/7] x86/hvm: Support extended destination IDs in virtual MSI and IO-APIC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.03.2026 13:31, Julian Vetter wrote:
> Use the newly defined masks to extract the full 15-bit destination ID
> from guest MSI addresses and IO-APIC RTEs. In hvm_inject_msi() combine
> the standard bits [19:12] with the extended bits [11:5] of the MSI
> address into a 15-bit destination ID for LAPIC delivery. Increase the
> dest parameter of vmsi_deliver() and hvm_girq_dest_2_vcpu_id() from
> uint8_t to uint32_t. In vmsi_deliver_pirq() extract the full destination
> from gflags via XEN_DOMCTL_VMSI_X86_FULL_DEST(). In msi_gflags() pack
> the extended bits from the MSI address into the new
> XEN_DOMCTL_VMSI_X86_EXT_DEST_ID_MASK field of gflags. In
> vioapic_deliver() read the combined 15-bit destination using the
> VIOAPIC_RTE_DEST() macro. Extend ioapic_check() to check for extended
> destination bits set in a domain that does not advertise
> XEN_HVM_CPUID_EXT_DEST_ID and refuse to restore the IO-APIC state,
> preventing silent interrupt misrouting after live migration.

This is pretty hard to read without being split in a few paragraphs.

> --- 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.

How do you know? Like for the IO-APIC RTE bits, there is (and cannot be)
anything enforcing this. Hence for a guest to use this feature, it needs
to have a way to opt in.

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

Nit: This line looks too long now.

Here as well as ...

> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -411,7 +411,9 @@ 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 <<
> +                     VIOAPIC_RTE_DEST_ID_UPPER_BITS) |
> +                    vioapic->redirtbl[pin].fields.dest_id;

... e.g. here a macro or inline function doing the conversion would likely
help readability quite a bit.

> @@ -618,6 +620,21 @@ static int cf_check ioapic_check(const struct domain *d, 
> hvm_domain_context_t *h
>               e->fields.reserved[0] || e->fields.reserved[1] ||
>               e->fields.reserved[2] || e->fields.reserved2 )
>              return -EINVAL;
> +
> +        /*
> +         * An RTE in the saved state has ext_dest_id bits set. Check that
> +         * the destination Xen has extended destination ID support enabled,
> +         * otherwise interrupt routing to APIC IDs > 255 would be broken
> +         * after restore.
> +         */
> +        if ( e->fields.ext_dest_id && !d->arch.hvm.ext_dest_id_enabled )

This won't build, as the ext_dest_id_enabled field appears only in patch 6.
But yes, that looks to be the opt-in mechanism I mentioned above.

> @@ -659,7 +676,7 @@ static int cf_check ioapic_load(struct domain *d, 
> hvm_domain_context_t *h)
>      return 0;
>  }
>  
> -HVM_REGISTER_SAVE_RESTORE(IOAPIC, ioapic_save, NULL, ioapic_load, 1,
> +HVM_REGISTER_SAVE_RESTORE(IOAPIC, ioapic_save, ioapic_check, ioapic_load, 1,
>                            HVMSR_PER_DOM);

As per the comment there, this belongs in the earlier patch.

Jan