From: dmukhin@ford.com
To: Teddy Astie <teddy.astie@vates.tech>
Cc: xen-devel@lists.xenproject.org,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"Anthony PERARD" <anthony.perard@vates.tech>,
"Michal Orzel" <michal.orzel@amd.com>,
"Jan Beulich" <jbeulich@suse.com>,
"Julien Grall" <julien@xen.org>,
"Roger Pau Monné" <roger.pau@citrix.com>,
"Stefano Stabellini" <sstabellini@kernel.org>
Subject: Re: [PATCH 3/5] pci: Use pci_sbdf_t in pci_device_detect()
Date: Tue, 19 May 2026 20:21:06 -0700 [thread overview]
Message-ID: <ag0oovFrGircUaut@kraken> (raw)
In-Reply-To: <1779117762.8631fc262581453bbf619ec5b2062170.19e3baea7d1000f373@vates.tech>
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@vates.tech>
> ---
> 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
next prev parent reply other threads:[~2026-05-20 3:21 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1779116255.git.teddy.astie@vates.tech>
2026-05-18 15:21 ` [PATCH 1/5] pci: Introduce parse_pci_sbdf{_seg}() Teddy Astie
2026-05-20 2:39 ` dmukhin
2026-05-20 10:00 ` Teddy Astie
2026-05-21 1:32 ` dmukhin
2026-05-18 15:21 ` [PATCH 2/5] vtd: Use pci_sbdf_t in acpi_parse_dev_scope() Teddy Astie
2026-05-20 3:00 ` dmukhin
2026-05-20 3:23 ` dmukhin
2026-05-20 6:32 ` Jan Beulich
2026-05-20 10:08 ` Teddy Astie
2026-05-18 15:21 ` [PATCH 3/5] pci: Use pci_sbdf_t in pci_device_detect() Teddy Astie
2026-05-20 3:21 ` dmukhin [this message]
2026-05-20 6:37 ` Jan Beulich
2026-05-18 15:21 ` [PATCH 4/5] pci: Parse into pci_sbdf_t directly Teddy Astie
2026-05-20 3:31 ` dmukhin
2026-05-18 15:21 ` [PATCH 5/5] RFC: pci: Migrate pci_mmcfg_{read,write} to pci.c Teddy Astie
2026-05-18 17:35 ` Andrew Cooper
2026-05-19 6:02 ` Jan Beulich
2026-05-19 17:15 ` Andrew Cooper
2026-05-19 13:42 ` Teddy Astie
2026-05-18 17:20 ` [PATCH 0/5] Small PCI refactoring Teddy Astie
2026-05-19 17:41 ` Andrew Cooper
2026-05-20 6:30 ` Jan Beulich
2026-05-20 3:34 ` dmukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ag0oovFrGircUaut@kraken \
--to=dmukhin@ford.com \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@vates.tech \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=teddy.astie@vates.tech \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.