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
