Re: [PATCH 2/5] vtd: Use pci_sbdf_t in acpi_parse_dev_scope()

[dmukhin@ford.com]

On Mon, May 18, 2026 at 05:21:26PM +0200, Teddy Astie wrote:
> Use a dedicated pci_sbdf_t struct that we update instead of recreating
> one each time we need it.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
>  xen/drivers/passthrough/vtd/dmar.c | 42 ++++++++++++------------------
>  1 file changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index 2a756831a6..c36f4bbd7b 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -310,7 +310,7 @@ static int __init acpi_parse_dev_scope(
>  {
>      struct acpi_ioapic_unit *acpi_ioapic_unit;
>      const struct acpi_dmar_device_scope *acpi_scope;
> -    u16 bus, sub_bus, sec_bus;
> +    u16 sub_bus, sec_bus;
>      const struct acpi_dmar_pci_path *path;
>      struct acpi_drhd_unit *drhd = type == DMAR_TYPE ?
>          container_of(scope, struct acpi_drhd_unit, scope) : NULL;
> @@ -332,29 +332,26 @@ static int __init acpi_parse_dev_scope(
>  
>      while ( start < end )
>      {
> +        pci_sbdf_t dev_sbdf;
>          acpi_scope = start;
>          path = (const void *)(acpi_scope + 1);
>          depth = (acpi_scope->length - sizeof(*acpi_scope)) / sizeof(*path);
> -        bus = acpi_scope->bus;
> +        dev_sbdf = PCI_SBDF(seg, acpi_scope->bus, path->dev, path->fn);

`dev_sbdf` calculation depends on `path` which is updated in `while()` loop
below.

>  
>          while ( --depth > 0 )
>          {
> -            bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
> -                                 PCI_SECONDARY_BUS);
> +            dev_sbdf.bus = pci_conf_read8(dev_sbdf, PCI_SECONDARY_BUS);
>              path++;
>          }
>  
>          switch ( acpi_scope->entry_type )
>          {
>          case ACPI_DMAR_SCOPE_TYPE_BRIDGE:
> -            sec_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
> -                                     PCI_SECONDARY_BUS);
> -            sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
> -                                     PCI_SUBORDINATE_BUS);
> +            sec_bus = pci_conf_read8(dev_sbdf, PCI_SECONDARY_BUS);
> +            sub_bus = pci_conf_read8(dev_sbdf, PCI_SUBORDINATE_BUS);
>              if ( iommu_verbose )
>                  printk(VTDPREFIX " bridge: %pp start=%x sec=%x sub=%x\n",
> -                       &PCI_SBDF(seg, bus, path->dev, path->fn),
> -                       acpi_scope->bus, sec_bus, sub_bus);
> +                       &dev_sbdf, acpi_scope->bus, sec_bus, sub_bus);
>  
>              dmar_scope_add_buses(scope, sec_bus, sub_bus);
>              gfx_only = false;
> @@ -362,8 +359,7 @@ static int __init acpi_parse_dev_scope(
>  
>          case ACPI_DMAR_SCOPE_TYPE_HPET:
>              if ( iommu_verbose )
> -                printk(VTDPREFIX " MSI HPET: %pp\n",
> -                       &PCI_SBDF(seg, bus, path->dev, path->fn));
> +                printk(VTDPREFIX " MSI HPET: %pp\n", &dev_sbdf);
>  
>              if ( drhd )
>              {
> @@ -374,9 +370,7 @@ static int __init acpi_parse_dev_scope(
>                  if ( !acpi_hpet_unit )
>                      goto out;
>                  acpi_hpet_unit->id = acpi_scope->enumeration_id;
> -                acpi_hpet_unit->bus = bus;
> -                acpi_hpet_unit->dev = path->dev;
> -                acpi_hpet_unit->func = path->fn;
> +                acpi_hpet_unit->bdf = dev_sbdf.bdf;
>                  list_add(&acpi_hpet_unit->list, &drhd->hpet_list);
>  
>                  gfx_only = false;
> @@ -386,16 +380,15 @@ static int __init acpi_parse_dev_scope(
>  
>          case ACPI_DMAR_SCOPE_TYPE_ENDPOINT:
>              if ( iommu_verbose )
> -                printk(VTDPREFIX " endpoint: %pp\n",
> -                       &PCI_SBDF(seg, bus, path->dev, path->fn));
> +                printk(VTDPREFIX " endpoint: %pp\n", &dev_sbdf);
>  
> -            if ( drhd && pci_device_detect(seg, bus, path->dev, path->fn) )
> +            if ( drhd && pci_device_detect(seg, dev_sbdf.bus, dev_sbdf.dev, dev_sbdf.fn) )

Looks like `pci_device_detect()` also needs some refactoring...
(Probably out of scope for this series, though)

>              {
> -                if ( pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
> +                if ( pci_conf_read8(dev_sbdf,
>                                      PCI_CLASS_DEVICE + 1) != 0x03
>                                      /* PCI_BASE_CLASS_DISPLAY */ )
>                      gfx_only = false;
> -                else if ( !seg && !bus && path->dev == 2 && !path->fn )
> +                else if ( !seg && !dev_sbdf.bus && path->dev == 2 && !path->fn )
>                      igd_drhd_address = drhd->address;
>              }
>  
> @@ -403,8 +396,7 @@ static int __init acpi_parse_dev_scope(
>  
>          case ACPI_DMAR_SCOPE_TYPE_IOAPIC:
>              if ( iommu_verbose )
> -                printk(VTDPREFIX " IOAPIC: %pp\n",
> -                       &PCI_SBDF(seg, bus, path->dev, path->fn));
> +                printk(VTDPREFIX " IOAPIC: %pp\n", &dev_sbdf);
>  
>              if ( drhd )
>              {
> @@ -413,9 +405,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);
>  
>                  gfx_only = false;
> @@ -431,7 +421,7 @@ static int __init acpi_parse_dev_scope(
>              gfx_only = false;
>              continue;
>          }
> -        scope->devices[didx++] = PCI_BDF(bus, path->dev, path->fn);
> +        scope->devices[didx++] = dev_sbdf.bdf;
>          start += acpi_scope->length;
>      }
>  
> -- 
> 2.52.0
> 
> 
> 
> --
> Teddy Astie | Vates XCP-ng Developer
> 
> XCP-ng & Xen Orchestra - Vates solutions
> 
> web: https://vates.tech