Re: [PATCH v3 08/18] pci: Introduce parse_pci_sbdf{_seg}()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.06.2026 19:21, 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@xxxxxxxxxx>

Having looked at patch 17 (undoing the transient pieces added here), I can't 
help
the impression that none of the style issues introduced here actually make that
patch any easier / smaller. Therefore I'm having a hard time ack-ing this 
change,
which otherwise I would now that (among the committers) we've sorted the issue 
of
transient introduction of unreachable code (permitting it when sorted within a
series).

Jan

> --- a/xen/drivers/pci/pci.c
> +++ b/xen/drivers/pci/pci.c
> @@ -151,7 +151,7 @@ void pci_intx(const struct pci_dev *pdev, bool enable)
>      pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
>  }
>  
> -const char *__init parse_pci(const char *s, unsigned int *seg_p,
> +const char *__init parse_pci_split(const char *s, unsigned int *seg_p,
>                               unsigned int *bus_p, unsigned int *dev_p,
>                               unsigned int *func_p)
>  {
> @@ -160,7 +160,7 @@ const char *__init parse_pci(const char *s, unsigned int 
> *seg_p,
>      return parse_pci_seg(s, seg_p, bus_p, dev_p, func_p, &def_seg);
>  }
>  
> -const char *__init parse_pci_seg(const char *s, unsigned int *seg_p,
> +const char *__init parse_pci_split_seg(const char *s, unsigned int *seg_p,
>                                   unsigned int *bus_p, unsigned int *dev_p,
>                                   unsigned int *func_p, bool *def_seg)
>  {
> @@ -202,3 +202,21 @@ const char *__init parse_pci_seg(const char *s, unsigned 
> int *seg_p,
>  
>      return s;
>  }
> +
> +const char *__init 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);
> +
> +    *sbdf = PCI_SBDF(seg, bus, dev, func);
> +    return out;
> +}
> +
> +const char *__init parse_pci_sbdf_seg(const char *s, pci_sbdf_t *sbdf, bool 
> *def_seg)
> +{
> +    unsigned int seg, bus, dev, func;
> +    const char *out = parse_pci_seg(s, &seg, &bus, &dev, &func, def_seg);
> +
> +    *sbdf = PCI_SBDF(seg, bus, dev, func);
> +    return out;
> +}
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index ed6c95452f..8e2f8a1ab7 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -274,12 +274,28 @@ unsigned int pci_find_ext_capability(const struct 
> pci_dev *pdev,
>  unsigned int pci_find_next_ext_capability(const struct pci_dev *pdev,
>                                            unsigned int start,
>                                            unsigned int cap);
> -const char *parse_pci(const char *s, unsigned int *seg_p, unsigned int 
> *bus_p,
> +const char *parse_pci_split(const char *s, unsigned int *seg_p, unsigned int 
> *bus_p,
>                        unsigned int *dev_p, unsigned int *func_p);
> -const char *parse_pci_seg(const char *s, unsigned int *seg_p,
> +const char *parse_pci_split_seg(const char *s, unsigned int *seg_p,
>                            unsigned int *bus_p, unsigned int *dev_p,
>                            unsigned int *func_p, bool *def_seg);
>  
> +const char *parse_pci_sbdf(const char *s, pci_sbdf_t *sbdf);
> +const char *parse_pci_sbdf_seg(const char *s, pci_sbdf_t *sbdf, bool 
> *def_seg);
> +
> +#define _parse_pci_split(a, b, c, d, e, ...) parse_pci_split(a, b, c, d, e)
> +#define _parse_pci_split_seg(a, b, c, d, e, f, ...) parse_pci_split_seg(a, 
> b, c, d, e, f)
> +
> +#define parse_pci(a, b, ...)                                            \
> +    (count_args(__VA_ARGS__) > 0                                        \
> +     ? _parse_pci_split(a, (void *)b, ##__VA_ARGS__, NULL, NULL, NULL)  \
> +     :  parse_pci_sbdf(a, (void *)b))
> +
> +#define parse_pci_seg(a, b, c, ...)                                     \
> +    (count_args(__VA_ARGS__) > 0                                        \
> +     ? _parse_pci_split_seg(a, (void *)b, (void *)c, ##__VA_ARGS__, NULL, 
> NULL, NULL, NULL) \
> +     :  parse_pci_sbdf_seg(a, (void *)b, (void *)c))
> +
>  #define PCI_BAR_VF      (1u << 0)
>  #define PCI_BAR_LAST    (1u << 1)
>  #define PCI_BAR_ROM     (1u << 2)