Re: [PATCH v3.1 02/18] pci: Use pci_sbdf_t in pci_add_device()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.07.2026 18:29, Teddy Astie wrote:
> @@ -729,14 +726,14 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>              if ( !pdev->ext_cfg )
>                  printk(XENLOG_WARNING
>                         "%pp: VF without extended config space?\n",
> -                       &pdev->sbdf);
> +                       &sbdf);
>          }
>      }
>  
>      if ( !pdev->info.is_virtfn && !pdev->physfn.vf_rlen[0] )
>      {
>          unsigned int pos = pci_find_ext_capability(pdev, 
> PCI_EXT_CAP_ID_SRIOV);
> -        uint16_t ctrl = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_CTRL);
> +        uint16_t ctrl = pci_conf_read16(sbdf, pos + PCI_SRIOV_CTRL);

There continue to be unnecessary changes here, like these two. Even if only
to limit churn, such changes imo better wouldn't be making unless they
offered a clear gain.

> @@ -818,14 +815,16 @@ out:
>      pcidevs_unlock();
>      if ( !ret )
>      {
> -        printk(XENLOG_DEBUG "PCI add %s %pp\n", type, &pdev->sbdf);
> +        unsigned int func = sbdf.fn;
> +
> +        printk(XENLOG_DEBUG "PCI add %s %pp\n", type, &sbdf);
>          while ( pdev->phantom_stride )
>          {
>              func += pdev->phantom_stride;
>              if ( PCI_SLOT(func) )
>                  break;
>              printk(XENLOG_DEBUG "PCI phantom %pp\n",
> -                   &PCI_SBDF(seg, bus, slot, func));
> +                   &PCI_SBDF(sbdf.seg, sbdf.bus, sbdf.dev, func));
>          }
>      }
>      return ret;

Is there a reason you retain "func" as a variable? As sbdf isn't used any
further, you could fiddle directly with sbfd.fn. (Then again I realize
some helper variable may be needed anyway, to be able to detect the field
having wrapped.)

Jan