Re: [PATCH 4/5] pci: Parse into pci_sbdf_t directly

[dmukhin@xxxxxxxx]

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@xxxxxxxxxx>
> ---
>  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