Re: [PATCH 1/5] pci: Introduce parse_pci_sbdf{_seg}()

[dmukhin@ford.com]

On Wed, May 20, 2026 at 12:00:07PM +0200, Teddy Astie wrote:
> Le 20/05/2026 à 04:43, dmukhin@ford.com a écrit :
> > On Mon, May 18, 2026 at 05:21:25PM +0200, Teddy Astie wrote:
> > > In many places, we're parsing a PCI string into individual parts
> > > (seg, bus, dev, fn) and then transform it into a pci_sbdf_t using PCI_SBDF
> > > macro. Rather than converting from parts to pci_sbdf_t and vice versa,
> > > introduce a new function that parses a PCI string into a pci_sbdf_t structure
> > > directly.
> > > 
> > > Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> > > ---
> > >   xen/drivers/pci/pci.c | 18 ++++++++++++++++++
> > >   xen/include/xen/pci.h |  3 +++
> > >   2 files changed, 21 insertions(+)
> > > 
> > > diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
> > > index 084be3880c..1d06cb035b 100644
> > > --- a/xen/drivers/pci/pci.c
> > > +++ b/xen/drivers/pci/pci.c
> > > @@ -202,3 +202,21 @@ const char *__init parse_pci_seg(const char *s, unsigned int *seg_p,
> > >       return s;
> > >   }
> > > +
> > > +const char *parse_pci_sbdf(const char *s, pci_sbdf_t *sbdf)
> > > +{
> > > +    unsigned int seg, bus, dev, func;
> > > +    const char *out = parse_pci(s, &seg, &bus, &dev, &func);
> > 
> > IMO, both parse_pci() and parse_pci_seg() should be merged into
> > parse_pci_sbdf() and parse_pci_sbdf_seg() at the end of the series,
> > since there will be no remaining consumers of the old APIs.
> > 
> > What do you think?
> > 
> 
> That was my plan, but parse_phantom_dev() (in xen/drivers/passthrough/pci.c)
> wants to parse the PCI string without the function part (i.e XXXX:YY:ZZ)
> which can't be expressed with a full SBDF parse function.

I see.
Perhaps adding "SBD" parsing in-place in parse_phantom_dev() is OK?
Like duplicate a small amount of code but drop old APIs.

> 
> It currently works by passing NULL to `func_p`, which has special handling
> in parse_pci.
> 
> We could eventually allow omitting PCI function and make default it to zero,
> so that we will be able migrate parse_phantom_dev to this new function (so
> it now allows parsing full SBDF, but ignore the function part of it).
>