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>,
"Jason Andryuk" <jason.andryuk@amd.com>
Subject: Re: [PATCH 4/5] pci: Parse into pci_sbdf_t directly
Date: Tue, 19 May 2026 20:31:37 -0700 [thread overview]
Message-ID: <ag0rGfmOwqMFXvi5@kraken> (raw)
In-Reply-To: <1779117763.8631fc262581453bbf619ec5b2062170.19e3baeac47000f373@vates.tech>
On Mon, May 18, 2026 at 05:21:28PM +0200, Teddy Astie wrote:
> Use the newly introduced parse_pci_sbdf() and parse_pci_sbdf_seg() in order
> to parse into a pci_sbdf_t directly instead of reconstructing it afterward.
>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> xen/drivers/char/ns16550.c | 24 +++++++++++-----------
> xen/drivers/char/xhci-dbc.c | 6 +++---
> xen/drivers/passthrough/amd/iommu_acpi.c | 26 ++++++++++++------------
> xen/drivers/passthrough/vtd/dmar.c | 7 +++----
> 4 files changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 878da27f2e..fa2d0e5991 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1572,22 +1572,22 @@ static bool __init parse_positional(struct ns16550 *uart, char **str)
> #ifdef CONFIG_HAS_PCI
> if ( *conf == ',' && *++conf != ',' )
> {
> - unsigned int b, d, f;
> + pci_sbdf_t sbdf;
>
> - conf = parse_pci(conf, NULL, &b, &d, &f);
> + conf = parse_pci_sbdf(conf, &sbdf);
Original logic considered only devices from PCI segment 0, now
all segments are allowed.
I think docs should be updated.
> if ( !conf )
> PARSE_ERR_RET("Bad port PCI coordinates");
Unrelated to the patch: I think it will be good to print the bad
string value in the error message.
> - uart->pci_device = PCI_SBDF(0, b, d, f);
> + uart->pci_device = sbdf;
> uart->ps_bdf_enable = true;
> }
>
> if ( *conf == ',' && *++conf != ',' )
> {
> - unsigned int b, d, f;
> + pci_sbdf_t sbdf;
>
> - if ( !parse_pci(conf, NULL, &b, &d, &f) )
> + if ( !parse_pci_sbdf(conf, &sbdf) )
> PARSE_ERR_RET("Bad bridge PCI coordinates");
> - uart->pci_bridge = PCI_SBDF(0, b, d, f);
> + uart->pci_bridge = sbdf;
> uart->pb_bdf_enable = true;
> }
> #endif
> @@ -1671,22 +1671,22 @@ static bool __init parse_namevalue_pairs(char *str, struct ns16550 *uart)
>
> case port_bdf:
> {
> - unsigned int b, d, f;
> + pci_sbdf_t sbdf;
>
> - if ( !parse_pci(param_value, NULL, &b, &d, &f) )
> + if ( !parse_pci_sbdf(param_value, &sbdf) )
> PARSE_ERR_RET("Bad port PCI coordinates\n");
> - uart->pci_device = PCI_SBDF(0, b, d, f);
> + uart->pci_device = sbdf;
> uart->ps_bdf_enable = true;
> break;
> }
>
> case bridge_bdf:
> {
> - unsigned int b, d, f;
> + pci_sbdf_t sbdf;
>
> - if ( !parse_pci(param_value, NULL, &b, &d, &f) )
> + if ( !parse_pci_sbdf(param_value, &sbdf) )
> PARSE_ERR_RET("Bad bridge PCI coordinates\n");
> - uart->pci_bridge = PCI_SBDF(0, b, d, f);
> + uart->pci_bridge = sbdf;
> uart->pb_bdf_enable = true;
> break;
> }
> diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
> index c1ff528de6..c7fd554be0 100644
> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -1357,9 +1357,9 @@ static int __init cf_check xhci_parse_dbgp(const char *opt_dbgp)
> }
> else if ( strncmp(opt_dbgp + 4, "@pci", 4) == 0 )
> {
> - unsigned int bus, slot, func;
> + pci_sbdf_t sbdf;
>
> - e = parse_pci(opt_dbgp + 8, NULL, &bus, &slot, &func);
> + e = parse_pci_sbdf(opt_dbgp + 8, &sbdf);
> if ( !e || (*e && *e != ',') )
> {
> printk(XENLOG_ERR
> @@ -1368,7 +1368,7 @@ static int __init cf_check xhci_parse_dbgp(const char *opt_dbgp)
> return -EINVAL;
> }
>
> - dbc->sbdf = PCI_SBDF(0, bus, slot, func);
> + dbc->sbdf = sbdf;
> }
> opt = e;
>
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 39ae637959..7b40da33ae 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -682,8 +682,8 @@ static int __init cf_check parse_ivrs_ioapic(const char *str)
> {
> const char *s = str;
> unsigned long id;
> - unsigned int seg, bus, dev, func;
> unsigned int idx;
> + pci_sbdf_t sbdf;
>
> if ( *s != '[' )
> return -EINVAL;
> @@ -692,7 +692,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str)
> if ( *s != ']' || *++s != '=' )
> return -EINVAL;
>
> - s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> + s = parse_pci_sbdf(s + 1, &sbdf);
> if ( !s || *s )
> return -EINVAL;
>
> @@ -707,7 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str)
> }
> }
>
> - ioapic_sbdf[idx].sbdf = PCI_SBDF(seg, bus, dev, func);
> + ioapic_sbdf[idx].sbdf = sbdf;
> ioapic_sbdf[idx].id = id;
> ioapic_sbdf[idx].cmdline = true;
>
> @@ -719,7 +719,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str)
> {
> const char *s = str;
> unsigned long id;
> - unsigned int seg, bus, dev, func;
> + pci_sbdf_t sbdf;
>
> if ( *s != '[' )
> return -EINVAL;
> @@ -728,12 +728,12 @@ static int __init cf_check parse_ivrs_hpet(const char *str)
> if ( id != (typeof(hpet_sbdf.id))id || *s != ']' || *++s != '=' )
> return -EINVAL;
>
> - s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> + s = parse_pci_sbdf(s + 1, &sbdf);
> if ( !s || *s )
> return -EINVAL;
>
> hpet_sbdf.id = id;
> - hpet_sbdf.sbdf = PCI_SBDF(seg, bus, dev, func);
> + hpet_sbdf.sbdf = sbdf;
> hpet_sbdf.init = HPET_CMDL;
>
> return 0;
> @@ -1399,13 +1399,13 @@ static int __init cf_check parse_ivmd_param(const char *s)
> }
>
> do {
> - unsigned int seg, bus, dev, func;
> + pci_sbdf_t sbdf;
>
> if ( nr_ivmd >= ARRAY_SIZE(user_ivmds) )
> return -E2BIG;
>
> - s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> - if ( !s || seg )
> + s = parse_pci_sbdf(s + 1, &sbdf);
> + if ( !s || sbdf.seg )
> return -EINVAL;
>
> user_ivmds[nr_ivmd].start_address = start << PAGE_SHIFT;
> @@ -1413,16 +1413,16 @@ static int __init cf_check parse_ivmd_param(const char *s)
> user_ivmds[nr_ivmd].header.flags = ACPI_IVMD_UNITY |
> ACPI_IVMD_READ | ACPI_IVMD_WRITE;
> user_ivmds[nr_ivmd].header.length = sizeof(*user_ivmds);
> - user_ivmds[nr_ivmd].header.device_id = PCI_BDF(bus, dev, func);
> + user_ivmds[nr_ivmd].header.device_id = sbdf.bdf;
> user_ivmds[nr_ivmd].header.type = ACPI_IVRS_TYPE_MEMORY_ONE;
>
> if ( *s == '-' )
> {
> - s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> - if ( !s || seg )
> + s = parse_pci_sbdf(s + 1, &sbdf);
> + if ( !s || sbdf.seg )
> return -EINVAL;
>
> - user_ivmds[nr_ivmd].aux_data = PCI_BDF(bus, dev, func);
> + user_ivmds[nr_ivmd].aux_data = sbdf.bdf;
> if ( user_ivmds[nr_ivmd].aux_data <
> user_ivmds[nr_ivmd].header.device_id )
> return -EINVAL;
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index 9f9b639eba..dafe1b62f6 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -1215,7 +1215,7 @@ static int __init cf_check parse_rmrr_param(const char *str)
> do {
> bool def_seg = false;
>
> - stmp = parse_pci_seg(s + 1, &seg, &bus, &dev, &func, &def_seg);
> + stmp = parse_pci_sbdf_seg(s + 1, &sbdf, &def_seg);
> if ( !stmp )
> return -EINVAL;
>
> @@ -1224,12 +1224,11 @@ static int __init cf_check parse_rmrr_param(const char *str)
> * Segment will be replaced with one from first device.
> */
> if ( user_rmrrs[nr_rmrr].dev_count && def_seg )
> - seg = PCI_SEG(user_rmrrs[nr_rmrr].sbdf[0]);
> + sbdf.seg = PCI_SEG(user_rmrrs[nr_rmrr].sbdf[0]);
>
> /* Keep sbdf's even if they differ and later report an error. */
> dev_count = user_rmrrs[nr_rmrr].dev_count;
> - user_rmrrs[nr_rmrr].sbdf[dev_count] =
> - PCI_SBDF(seg, bus, dev, func).sbdf;
> + user_rmrrs[nr_rmrr].sbdf[dev_count] = sbdf.sbdf;
>
> user_rmrrs[nr_rmrr].dev_count++;
> s = stmp;
> --
> 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:32 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
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 [this message]
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=ag0rGfmOwqMFXvi5@kraken \
--to=dmukhin@ford.com \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@vates.tech \
--cc=jason.andryuk@amd.com \
--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.