Re: [PATCH v2 02/15] vtd: Use pci_sbdf_t in acpi_parse_dev_scope()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/06/2026 3:50 pm, Teddy Astie wrote:
> diff --git a/xen/drivers/passthrough/vtd/dmar.c 
> b/xen/drivers/passthrough/vtd/dmar.c
> index 2a756831a6..71e5a51a58 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -413,9 +406,7 @@ static int __init acpi_parse_dev_scope(
>                  if ( !acpi_ioapic_unit )
>                      goto out;
>                  acpi_ioapic_unit->apic_id = acpi_scope->enumeration_id;
> -                acpi_ioapic_unit->ioapic.bdf.bus = bus;
> -                acpi_ioapic_unit->ioapic.bdf.dev = path->dev;
> -                acpi_ioapic_unit->ioapic.bdf.func = path->fn;
> +                acpi_ioapic_unit->ioapic.info = dev_sbdf.bdf;
>                  list_add(&acpi_ioapic_unit->list, &drhd->ioapic_list); 

Looking at this more closely (because of an issue in a later patch),
swapping bdf for info looks wonky.

It turns out to be correct, but would be better with:

diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index ac828e5fe8eb..dc05774629d0 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -407,7 +407,7 @@ static int __init acpi_parse_dev_scope(
                 if ( !acpi_ioapic_unit )
                     goto out;
                 acpi_ioapic_unit->apic_id = acpi_scope->enumeration_id;
-                acpi_ioapic_unit->ioapic.info = dev_sbdf.bdf;
+                acpi_ioapic_unit->bdf = dev_sbdf.bdf;
                 list_add(&acpi_ioapic_unit->list, &drhd->ioapic_list);
 
                 gfx_only = false;
diff --git a/xen/drivers/passthrough/vtd/dmar.h 
b/xen/drivers/passthrough/vtd/dmar.h
index 47e6918b45cc..5ab95c68bcf5 100644
--- a/xen/drivers/passthrough/vtd/dmar.h
+++ b/xen/drivers/passthrough/vtd/dmar.h
@@ -29,13 +29,13 @@ struct acpi_ioapic_unit {
     struct list_head list;
     int apic_id;
     union {
-        u16 info;
+        uint16_t bdf;
         struct {
             u16 func: 3,
                 dev:  5,
                 bus:  8;
-        }bdf;
-    }ioapic;
+        };
+    };
 };
 
 struct acpi_hpet_unit {
diff --git a/xen/drivers/passthrough/vtd/intremap.c 
b/xen/drivers/passthrough/vtd/intremap.c
index dd2788efd735..3aa8babdfd55 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -98,7 +98,7 @@ static u16 apicid_to_bdf(int apic_id)
 
     list_for_each_entry ( acpi_ioapic_unit, &drhd->ioapic_list, list )
         if ( acpi_ioapic_unit->apic_id == apic_id )
-            return acpi_ioapic_unit->ioapic.info;
+            return acpi_ioapic_unit->bdf;
 
     dprintk(XENLOG_ERR VTDPREFIX, "Didn't find the bdf for the apic_id!\n");
     return 0;


Personally I'd be tempted to just fold this in as well as it is relevant
to the acpi_parse_dev_scope(), and because this is brining IOAPIC in
line with HPET, but it could be a followup patch if you'd prefer not to
mix the two.

~Andrew