Re: [PATCH 3/5] pci: Use pci_sbdf_t in pci_device_detect()

[dmukhin@xxxxxxxx]

On Mon, May 18, 2026 at 05:21:27PM +0200, Teddy Astie wrote:
> Use a single pci_sbdf_t instead of each of its part as individual parameters.
> 
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
>  xen/drivers/char/ehci-dbgp.c       | 35 ++++++++++++++----------------
>  xen/drivers/passthrough/pci.c      | 16 +++++++-------
>  xen/drivers/passthrough/vtd/dmar.c | 33 +++++++++++-----------------
>  xen/include/xen/pci.h              |  2 +-
>  4 files changed, 38 insertions(+), 48 deletions(-)
> 
> diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
> index a5c79f56fc..39a047eb3f 100644
> --- a/xen/drivers/char/ehci-dbgp.c
> +++ b/xen/drivers/char/ehci-dbgp.c
> @@ -681,16 +681,14 @@ static int dbgp_control_msg(struct ehci_dbgp *dbgp, 
> unsigned int devnum,
>      return ret;
>  }
>  
> -static unsigned int __init __find_dbgp(u8 bus, u8 slot, u8 func)
> +static unsigned int __init __find_dbgp(pci_sbdf_t sbdf)
>  {
> -    uint32_t class = pci_conf_read32(PCI_SBDF(0, bus, slot, func),
> -                                     PCI_CLASS_REVISION);
> +    uint32_t class = pci_conf_read32(sbdf, PCI_CLASS_REVISION);
>  
>      if ( (class >> 8) != PCI_CLASS_SERIAL_USB_EHCI )
>          return 0;
>  
> -    return pci_find_cap_offset(PCI_SBDF(0, bus, slot, func),
> -                               PCI_CAP_ID_EHCI_DEBUG);
> +    return pci_find_cap_offset(sbdf, PCI_CAP_ID_EHCI_DEBUG);
>  }
>  
>  static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp,
> @@ -704,20 +702,20 @@ static unsigned int __init find_dbgp(struct ehci_dbgp 
> *dbgp,
>          {
>              for ( func = 0; func < 8; func++ )
>              {
> +                pci_sbdf_t sbdf = PCI_SBDF(0, bus, slot, func);
>                  unsigned int cap;
>  
> -                if ( !pci_device_detect(0, bus, slot, func) )
> +                if ( !pci_device_detect(sbdf) )
>                  {
>                      if ( !func )
>                          break;
>                      continue;
>                  }
>  
> -                cap = __find_dbgp(bus, slot, func);
> +                cap = __find_dbgp(sbdf);
>                  if ( !cap || ehci_num-- )
>                  {
> -                    if ( !func && !(pci_conf_read8(PCI_SBDF(0, bus, slot, 
> func),
> -                                                   PCI_HEADER_TYPE) & 0x80) )
> +                    if ( !func && !(pci_conf_read8(sbdf, PCI_HEADER_TYPE) & 
> 0x80) )
>                          break;
>                      continue;
>                  }
> @@ -1510,25 +1508,24 @@ void __init ehci_dbgp_init(void)
>      }
>      else if ( strncmp(opt_dbgp + 4, "@pci", 4) == 0 )
>      {
> -        unsigned int bus, slot, func;
> -
> -        e = parse_pci(opt_dbgp + 8, NULL, &bus, &slot, &func);
> +        pci_sbdf_t sbdf = PCI_SBDF(0, 0, 0, 0);
> +        
> +        e = parse_pci_sbdf(opt_dbgp + 8, &sbdf);

The original logic was ignoring PCI segment, now the full PCI address
is allowed.

Looks like
  docs/misc/xen-command-line.pandoc
needs update; but the documentation also says that debug port should be
in PCI segment 0...

>          if ( !e || *e )
>              return;
>  
> -        dbgp->bus = bus;
> -        dbgp->slot = slot;
> -        dbgp->func = func;
> +        dbgp->bus = sbdf.bus;
> +        dbgp->slot = sbdf.dev;
> +        dbgp->func = sbdf.fn;
>  
> -        if ( !pci_device_detect(0, bus, slot, func) )
> +        if ( !pci_device_detect(sbdf) )
>              return;
>  
> -        dbgp->cap = __find_dbgp(bus, slot, func);
> +        dbgp->cap = __find_dbgp(sbdf);
>          if ( !dbgp->cap )
>              return;
>  
> -        dbgp_printk("Using EHCI debug port on %02x:%02x.%u\n",
> -                    bus, slot, func);
> +        dbgp_printk("Using EHCI debug port on %pp\n", &sbdf);
>      }
>      else
>          return;
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 464bb0fee4..7b2898bd5a 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1166,11 +1166,11 @@ out:
>      return ret;
>  }
>  
> -bool __init pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func)
> +bool __init pci_device_detect(pci_sbdf_t sbdf)
>  {
>      u32 vendor;
>  
> -    vendor = pci_conf_read32(PCI_SBDF(seg, bus, dev, func), PCI_VENDOR_ID);
> +    vendor = pci_conf_read32(sbdf, PCI_VENDOR_ID);
>      /* some broken boards return 0 or ~0 if a slot is empty: */
>      if ( (vendor == 0xffffffffU) || (vendor == 0x00000000U) ||
>           (vendor == 0x0000ffffU) || (vendor == 0xffff0000U) )
> @@ -1221,24 +1221,24 @@ static int __init cf_check _scan_pci_devices(struct 
> pci_seg *pseg, void *arg)
>          {
>              for ( func = 0; func < 8; func++ )
>              {
> -                if ( !pci_device_detect(pseg->nr, bus, dev, func) )
> +                pci_sbdf_t sbdf = PCI_SBDF(pseg->nr, bus, dev, func);
> +
> +                if ( !pci_device_detect(sbdf) )
>                  {
>                      if ( !func )
>                          break;
>                      continue;
>                  }
>  
> -                pdev = alloc_pdev(pseg, bus, PCI_DEVFN(dev, func));
> +                pdev = alloc_pdev(pseg, bus, sbdf.devfn);
>                  if ( !pdev )
>                  {
>                      printk(XENLOG_WARNING "%pp: alloc_pdev failed\n",
> -                           &PCI_SBDF(pseg->nr, bus, dev, func));
> +                           &sbdf);
>                      return -ENOMEM;
>                  }
>  
> -                if ( !func && !(pci_conf_read8(PCI_SBDF(pseg->nr, bus, dev,
> -                                                        func),
> -                                               PCI_HEADER_TYPE) & 0x80) )
> +                if ( !func && !(pci_conf_read8(sbdf, PCI_HEADER_TYPE) & 
> 0x80) )
>                      break;
>              }
>          }
> diff --git a/xen/drivers/passthrough/vtd/dmar.c 
> b/xen/drivers/passthrough/vtd/dmar.c
> index c36f4bbd7b..9f9b639eba 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -382,7 +382,7 @@ static int __init acpi_parse_dev_scope(
>              if ( iommu_verbose )
>                  printk(VTDPREFIX " endpoint: %pp\n", &dev_sbdf);
>  
> -            if ( drhd && pci_device_detect(seg, dev_sbdf.bus, dev_sbdf.dev, 
> dev_sbdf.fn) )
> +            if ( drhd && pci_device_detect(dev_sbdf) )
>              {
>                  if ( pci_conf_read8(dev_sbdf,
>                                      PCI_CLASS_DEVICE + 1) != 0x03
> @@ -505,7 +505,6 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>          acpi_register_drhd_unit(dmaru);
>      else
>      {
> -        u8 b, d, f;
>          unsigned int i = 0;
>          union {
>              const void *raw;
> @@ -519,18 +518,16 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>          for ( p.raw = dev_scope_start; i < dmaru->scope.devices_cnt;
>                i++, p.raw += p.scope->length )
>          {
> +            pci_sbdf_t sbdf = PCI_SBDF(drhd->segment, 
> dmaru->scope.devices[i]);
> +
>              if ( p.scope->entry_type == ACPI_DMAR_SCOPE_TYPE_IOAPIC ||
>                   p.scope->entry_type == ACPI_DMAR_SCOPE_TYPE_HPET )
>                  continue;
>  
> -            b = PCI_BUS(dmaru->scope.devices[i]);
> -            d = PCI_SLOT(dmaru->scope.devices[i]);
> -            f = PCI_FUNC(dmaru->scope.devices[i]);
> -
> -            if ( !pci_device_detect(drhd->segment, b, d, f) )
> +            if ( !pci_device_detect(sbdf) )
>                  printk(XENLOG_WARNING VTDPREFIX
>                         " Non-existent device (%pp) in this DRHD's scope!\n",
> -                       &PCI_SBDF(drhd->segment, b, d, f));
> +                       &sbdf);
>          }
>  
>          acpi_register_drhd_unit(dmaru);
> @@ -559,17 +556,14 @@ static int __init register_one_rmrr(struct 
> acpi_rmrr_unit *rmrru)
>  
>      for ( ; i < rmrru->scope.devices_cnt; i++ )
>      {
> -        u8 b = PCI_BUS(rmrru->scope.devices[i]);
> -        u8 d = PCI_SLOT(rmrru->scope.devices[i]);
> -        u8 f = PCI_FUNC(rmrru->scope.devices[i]);
> +        pci_sbdf_t sbdf = PCI_SBDF(rmrru->segment, rmrru->scope.devices[i]);
>  
> -        if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
> +        if ( pci_device_detect(sbdf) == 0 )
>          {
>              dprintk(XENLOG_WARNING VTDPREFIX,
>                      " Non-existent device (%pp) is reported"
>                      " in RMRR [%"PRIx64", %"PRIx64"]'s scope!\n",
> -                    &PCI_SBDF(rmrru->segment, b, d, f),
> -                    rmrru->base_address, rmrru->end_address);
> +                    &sbdf, rmrru->base_address, rmrru->end_address);
>              ignore = true;
>          }
>          else
> @@ -753,15 +747,13 @@ static int __init register_one_satc(struct 
> acpi_satc_unit *satcu)
>  
>      for ( ; i < satcu->scope.devices_cnt; i++ )
>      {
> -        uint8_t b = PCI_BUS(satcu->scope.devices[i]);
> -        uint8_t d = PCI_SLOT(satcu->scope.devices[i]);
> -        uint8_t f = PCI_FUNC(satcu->scope.devices[i]);
> +        pci_sbdf_t sbdf = PCI_SBDF(satcu->segment, satcu->scope.devices[i]);
>  
> -        if ( !pci_device_detect(satcu->segment, b, d, f) )
> +        if ( !pci_device_detect(sbdf) )
>          {
>              dprintk(XENLOG_WARNING VTDPREFIX,
>                      " Non-existent device (%pp) is reported in SATC 
> scope!\n",
> -                    &PCI_SBDF(satcu->segment, b, d, f));
> +                    &sbdf);
>              ignore = true;
>          }
>          else
> @@ -1184,8 +1176,9 @@ int cf_check intel_iommu_get_reserved_device_memory(
>  static int __init cf_check parse_rmrr_param(const char *str)
>  {
>      const char *s = str, *cur, *stmp;
> -    unsigned int seg, bus, dev, func, dev_count;
> +    unsigned int dev_count;
>      unsigned long start, end;
> +    pci_sbdf_t sbdf;
>  
>      do {
>          if ( nr_rmrr >= MAX_USER_RMRR )
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 7bfc59cd75..d816dcad05 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -218,7 +218,7 @@ static always_inline bool pcidevs_trylock(void)
>  #endif
>  
>  bool pci_known_segment(u16 seg);
> -bool pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
> +bool pci_device_detect(pci_sbdf_t sbdf);
>  int scan_pci_devices(void);
>  enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn);
>  int find_upstream_bridge(u16 seg, u8 *bus, u8 *devfn, u8 *secbus);
> -- 
> 2.52.0
> 
> 
> 
> --
> Teddy Astie | Vates XCP-ng Developer
> 
> XCP-ng & Xen Orchestra - Vates solutions
> 
> web: https://vates.tech