diff mbox series

[v3,09/13] pci: switch pci_conf_read32 to use pci_sbdf_t

Message ID 20190607092232.83179-10-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series pci: expand usage of pci_sbdf_t | expand

Commit Message

Roger Pau Monné June 7, 2019, 9:22 a.m. UTC
This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/cpu/amd.c                     |  7 +++--
 xen/arch/x86/mm.c                          |  2 +-
 xen/arch/x86/msi.c                         | 28 ++++++-----------
 xen/arch/x86/oprofile/op_model_athlon.c    |  6 ++--
 xen/arch/x86/x86_64/mmconf-fam10h.c        |  8 +++--
 xen/arch/x86/x86_64/mmconfig-shared.c      | 12 ++++----
 xen/arch/x86/x86_64/pci.c                  | 27 +++++++---------
 xen/drivers/char/ehci-dbgp.c               | 20 +++++++-----
 xen/drivers/char/ns16550.c                 | 18 ++++++-----
 xen/drivers/passthrough/amd/iommu_detect.c |  2 +-
 xen/drivers/passthrough/amd/iommu_init.c   |  4 +--
 xen/drivers/passthrough/pci.c              | 15 ++++-----
 xen/drivers/passthrough/vtd/quirks.c       | 36 ++++++++++++----------
 xen/drivers/pci/pci.c                      |  4 +--
 xen/drivers/vpci/header.c                  |  6 ++--
 xen/drivers/vpci/msix.c                    |  6 ++--
 xen/drivers/vpci/vpci.c                    |  5 ++-
 xen/include/xen/pci.h                      |  4 +--
 18 files changed, 101 insertions(+), 109 deletions(-)

Comments

Jan Beulich June 13, 2019, 2:36 p.m. UTC | #1
>>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote:
> @@ -817,7 +811,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
>          if ( ++bir >= limit )
>              return 0;
>          return addr + disp +
> -               ((u64)pci_conf_read32(seg, bus, slot, func,
> +               ((u64)pci_conf_read32(PCI_SBDF(seg, bus, slot, func),
>                                       base + bir * 4) << 32);

Not taking the opportunity to switch to uint64_t here, like you do
elsewhere?

> @@ -750,7 +747,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>              for ( i = 0; i < PCI_SRIOV_NUM_BARS; )
>              {
>                  unsigned int idx = pos + PCI_SRIOV_BAR + i * 4;
> -                u32 bar = pci_conf_read32(seg, bus, slot, func, idx);
> +                u32 bar = pci_conf_read32(pdev->sbdf, idx);

Similarly here.

> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -128,9 +128,11 @@ static void __init map_igd_reg(void)
>      if ( igd_reg_va )
>          return;
>  
> -    igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
> +    igd_mmio   = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),

Afaict at this point all uses of IGD_DEV are in constructs like this one.
As previously say, I think IGD_DEV itself would now better become an
invocation of PCI_SBDF(). Same for IOH_DEV then.

Jan
Roger Pau Monné June 14, 2019, 9:06 a.m. UTC | #2
On Thu, Jun 13, 2019 at 08:36:19AM -0600, Jan Beulich wrote:
> >>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote:
> > @@ -817,7 +811,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
> >          if ( ++bir >= limit )
> >              return 0;
> >          return addr + disp +
> > -               ((u64)pci_conf_read32(seg, bus, slot, func,
> > +               ((u64)pci_conf_read32(PCI_SBDF(seg, bus, slot, func),
> >                                       base + bir * 4) << 32);
> 
> Not taking the opportunity to switch to uint64_t here, like you do
> elsewhere?
> 
> > @@ -750,7 +747,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
> >              for ( i = 0; i < PCI_SRIOV_NUM_BARS; )
> >              {
> >                  unsigned int idx = pos + PCI_SRIOV_BAR + i * 4;
> > -                u32 bar = pci_conf_read32(seg, bus, slot, func, idx);
> > +                u32 bar = pci_conf_read32(pdev->sbdf, idx);
> 
> Similarly here.

Sorry, I tried to fix the surrounding issues when touching the code,
but some of those slipped. Will fix in next version.

> > --- a/xen/drivers/passthrough/vtd/quirks.c
> > +++ b/xen/drivers/passthrough/vtd/quirks.c
> > @@ -128,9 +128,11 @@ static void __init map_igd_reg(void)
> >      if ( igd_reg_va )
> >          return;
> >  
> > -    igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
> > +    igd_mmio   = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
> 
> Afaict at this point all uses of IGD_DEV are in constructs like this one.
> As previously say, I think IGD_DEV itself would now better become an
> invocation of PCI_SBDF(). Same for IOH_DEV then.

Is it fine to do this as a followup patch?

Or else I would also have to modify the pci_conf_read{8,16} calls that
use IGD_DEV or IOH_DEV in this patch.

Thanks, Roger.
Jan Beulich June 14, 2019, 9:20 a.m. UTC | #3
>>> On 14.06.19 at 11:06, <roger.pau@citrix.com> wrote:
> On Thu, Jun 13, 2019 at 08:36:19AM -0600, Jan Beulich wrote:
>> >>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote:
>> > --- a/xen/drivers/passthrough/vtd/quirks.c
>> > +++ b/xen/drivers/passthrough/vtd/quirks.c
>> > @@ -128,9 +128,11 @@ static void __init map_igd_reg(void)
>> >      if ( igd_reg_va )
>> >          return;
>> >  
>> > -    igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
>> > +    igd_mmio   = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
>> 
>> Afaict at this point all uses of IGD_DEV are in constructs like this one.
>> As previously say, I think IGD_DEV itself would now better become an
>> invocation of PCI_SBDF(). Same for IOH_DEV then.
> 
> Is it fine to do this as a followup patch?

Either way is fine by me, albeit I'd (slightly) prefer it getting folded into
here, irrespective ...

> Or else I would also have to modify the pci_conf_read{8,16} calls that
> use IGD_DEV or IOH_DEV in this patch.

... of the need for these extra adjustments.

Jan
Woods, Brian June 19, 2019, 4:01 p.m. UTC | #4
On Fri, Jun 07, 2019 at 11:22:28AM +0200, Roger Pau Monne wrote:
> This reduces the number of parameters of the function to two, and
> simplifies some of the calling sites.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

As far as AMD IOMMU

Acked-by: Brian Woods <brian.woods@amd.com

> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Brian Woods <brian.woods@amd.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
>  xen/arch/x86/cpu/amd.c                     |  7 +++--
>  xen/arch/x86/mm.c                          |  2 +-
>  xen/arch/x86/msi.c                         | 28 ++++++-----------
>  xen/arch/x86/oprofile/op_model_athlon.c    |  6 ++--
>  xen/arch/x86/x86_64/mmconf-fam10h.c        |  8 +++--
>  xen/arch/x86/x86_64/mmconfig-shared.c      | 12 ++++----
>  xen/arch/x86/x86_64/pci.c                  | 27 +++++++---------
>  xen/drivers/char/ehci-dbgp.c               | 20 +++++++-----
>  xen/drivers/char/ns16550.c                 | 18 ++++++-----
>  xen/drivers/passthrough/amd/iommu_detect.c |  2 +-
>  xen/drivers/passthrough/amd/iommu_init.c   |  4 +--
>  xen/drivers/passthrough/pci.c              | 15 ++++-----
>  xen/drivers/passthrough/vtd/quirks.c       | 36 ++++++++++++----------
>  xen/drivers/pci/pci.c                      |  4 +--
>  xen/drivers/vpci/header.c                  |  6 ++--
>  xen/drivers/vpci/msix.c                    |  6 ++--
>  xen/drivers/vpci/vpci.c                    |  5 ++-
>  xen/include/xen/pci.h                      |  4 +--
>  18 files changed, 101 insertions(+), 109 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 3c069391f4..37f60c0862 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -417,7 +417,8 @@ static void disable_c1_ramping(void)
>  	int node, nr_nodes;
>  
>  	/* Read the number of nodes from the first Northbridge. */
> -	nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1;
> +	nr_nodes = ((pci_conf_read32(PCI_SBDF(0, 0, 0x18, 0), 0x60) >> 4) &
> +		    0x07) + 1;
>  	for (node = 0; node < nr_nodes; node++) {
>  		/* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */
>  		pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87);
> @@ -696,8 +697,8 @@ static void init_amd(struct cpuinfo_x86 *c)
>  
>  	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
>  		if (c == &boot_cpu_data) {
> -			l = pci_conf_read32(0, 0, 0x18, 0x3, 0x58);
> -			h = pci_conf_read32(0, 0, 0x18, 0x3, 0x5c);
> +			l = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x58);
> +			h = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x5c);
>  			if ((l & 0x1f) | (h & 0x1))
>  				printk(KERN_WARNING
>  				       "Applying workaround for erratum 792: %s%s%s\n",
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index df2c0130f1..e67119dbe6 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5949,7 +5949,7 @@ const struct platform_bad_page *__init get_platform_badpages(unsigned int *array
>      }
>  
>      *array_size = ARRAY_SIZE(snb_bad_pages);
> -    igd_id = pci_conf_read32(0, 0, 2, 0, 0);
> +    igd_id = pci_conf_read32(PCI_SBDF(0, 0, 2, 0), 0);
>      if ( IS_SNB_GFX(igd_id) )
>          return snb_bad_pages;
>  
> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
> index ed986261c3..392cbecfe4 100644
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -191,16 +191,13 @@ static bool read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>      {
>          struct pci_dev *dev = entry->dev;
>          int pos = entry->msi_attrib.pos;
> -        u16 data, seg = dev->seg;
> -        u8 bus = dev->bus;
> -        u8 slot = PCI_SLOT(dev->devfn);
> -        u8 func = PCI_FUNC(dev->devfn);
> +        uint16_t data;
>  
> -        msg->address_lo = pci_conf_read32(seg, bus, slot, func,
> +        msg->address_lo = pci_conf_read32(dev->sbdf,
>                                            msi_lower_address_reg(pos));
>          if ( entry->msi_attrib.is_64 )
>          {
> -            msg->address_hi = pci_conf_read32(seg, bus, slot, func,
> +            msg->address_hi = pci_conf_read32(dev->sbdf,
>                                                msi_upper_address_reg(pos));
>              data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 1));
>          }
> @@ -396,7 +393,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
>          {
>              u32 mask_bits;
>  
> -            mask_bits = pci_conf_read32(seg, bus, slot, func, entry->msi.mpos);
> +            mask_bits = pci_conf_read32(pdev->sbdf, entry->msi.mpos);
>              mask_bits &= ~((u32)1 << entry->msi_attrib.entry_nr);
>              mask_bits |= (u32)flag << entry->msi_attrib.entry_nr;
>              pci_conf_write32(seg, bus, slot, func, entry->msi.mpos, mask_bits);
> @@ -465,10 +462,7 @@ static int msi_get_mask_bit(const struct msi_desc *entry)
>      case PCI_CAP_ID_MSI:
>          if ( !entry->msi_attrib.maskbit )
>              break;
> -        return (pci_conf_read32(entry->dev->seg, entry->dev->bus,
> -                                PCI_SLOT(entry->dev->devfn),
> -                                PCI_FUNC(entry->dev->devfn),
> -                                entry->msi.mpos) >>
> +        return (pci_conf_read32(entry->dev->sbdf, entry->msi.mpos) >>
>                  entry->msi_attrib.entry_nr) & 1;
>      case PCI_CAP_ID_MSIX:
>          if ( unlikely(!msix_memory_decoded(entry->dev,
> @@ -723,7 +717,7 @@ static int msi_capability_init(struct pci_dev *dev,
>          u32 maskbits;
>  
>          /* All MSIs are unmasked by default, Mask them all */
> -        maskbits = pci_conf_read32(seg, bus, slot, func, mpos);
> +        maskbits = pci_conf_read32(dev->sbdf, mpos);
>          maskbits |= ~(u32)0 >> (32 - maxvec);
>          pci_conf_write32(seg, bus, slot, func, mpos, maskbits);
>      }
> @@ -808,7 +802,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
>  
>      if ( bir >= limit )
>          return 0;
> -    addr = pci_conf_read32(seg, bus, slot, func, base + bir * 4);
> +    addr = pci_conf_read32(PCI_SBDF(seg, bus, slot, func), base + bir * 4);
>      if ( (addr & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO )
>          return 0;
>      if ( (addr & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64 )
> @@ -817,7 +811,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
>          if ( ++bir >= limit )
>              return 0;
>          return addr + disp +
> -               ((u64)pci_conf_read32(seg, bus, slot, func,
> +               ((u64)pci_conf_read32(PCI_SBDF(seg, bus, slot, func),
>                                       base + bir * 4) << 32);
>      }
>      return (addr & PCI_BASE_ADDRESS_MEM_MASK) + disp;
> @@ -886,8 +880,7 @@ static int msix_capability_init(struct pci_dev *dev,
>      }
>  
>      /* Locate MSI-X table region */
> -    table_offset = pci_conf_read32(seg, bus, slot, func,
> -                                   msix_table_offset_reg(pos));
> +    table_offset = pci_conf_read32(dev->sbdf, msix_table_offset_reg(pos));
>      bir = (u8)(table_offset & PCI_MSIX_BIRMASK);
>      table_offset &= ~PCI_MSIX_BIRMASK;
>  
> @@ -933,8 +926,7 @@ static int msix_capability_init(struct pci_dev *dev,
>          WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->table.first,
>                                          msix->table.last));
>  
> -        pba_offset = pci_conf_read32(seg, bus, slot, func,
> -                                     msix_pba_offset_reg(pos));
> +        pba_offset = pci_conf_read32(dev->sbdf, msix_pba_offset_reg(pos));
>          bir = (u8)(pba_offset & PCI_MSIX_BIRMASK);
>          pba_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf);
>          WARN_ON(!pba_paddr);
> diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
> index 3d6e26f636..3bf0b0214d 100644
> --- a/xen/arch/x86/oprofile/op_model_athlon.c
> +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> @@ -463,7 +463,8 @@ static int __init init_ibs_nmi(void)
>  	for (bus = 0; bus < 256; bus++) {
>  		for (dev = 0; dev < 32; dev++) {
>  			for (func = 0; func < 8; func++) {
> -				id = pci_conf_read32(0, bus, dev, func, PCI_VENDOR_ID);
> +				id = pci_conf_read32(PCI_SBDF(0, bus, dev, func),
> +						     PCI_VENDOR_ID);
>  
>  				vendor_id = id & 0xffff;
>  				dev_id = (id >> 16) & 0xffff;
> @@ -474,7 +475,8 @@ static int __init init_ibs_nmi(void)
>  					pci_conf_write32(0, bus, dev, func, IBSCTL,
>  						IBSCTL_LVTOFFSETVAL | APIC_EILVT_LVTOFF_IBS);
>  
> -					value = pci_conf_read32(0, bus, dev, func, IBSCTL);
> +					value = pci_conf_read32(PCI_SBDF(0, bus, dev, func),
> +								IBSCTL);
>  
>  					if (value != (IBSCTL_LVTOFFSETVAL |
>  						APIC_EILVT_LVTOFF_IBS)) {
> diff --git a/xen/arch/x86/x86_64/mmconf-fam10h.c b/xen/arch/x86/x86_64/mmconf-fam10h.c
> index ed0acb9968..f997688ad4 100644
> --- a/xen/arch/x86/x86_64/mmconf-fam10h.c
> +++ b/xen/arch/x86/x86_64/mmconf-fam10h.c
> @@ -52,7 +52,7 @@ static void __init get_fam10h_pci_mmconf_base(void)
>  
>  		bus = pci_probes[i].bus;
>  		slot = pci_probes[i].slot;
> -		id = pci_conf_read32(0, bus, slot, 0, PCI_VENDOR_ID);
> +		id = pci_conf_read32(PCI_SBDF(0, bus, slot, 0), PCI_VENDOR_ID);
>  
>  		vendor = id & 0xffff;
>  		device = (id>>16) & 0xffff;
> @@ -83,12 +83,14 @@ static void __init get_fam10h_pci_mmconf_base(void)
>  	 * above 4G
>  	 */
>  	for (hi_mmio_num = i = 0; i < 8; i++) {
> -		val = pci_conf_read32(0, bus, slot, 1, 0x80 + (i << 3));
> +		val = pci_conf_read32(PCI_SBDF(0, bus, slot, 1),
> +				      0x80 + (i << 3));
>  		if (!(val & 3))
>  			continue;
>  
>  		start = (val & 0xffffff00) << 8; /* 39:16 on 31:8*/
> -		val = pci_conf_read32(0, bus, slot, 1, 0x84 + (i << 3));
> +		val = pci_conf_read32(PCI_SBDF(0, bus, slot, 1),
> +				      0x84 + (i << 3));
>  		end = ((val & 0xffffff00) << 8) | 0xffff; /* 39:16 on 31:8*/
>  
>  		if (end < tom2)
> diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
> index 9d1db590d9..cc08b52a35 100644
> --- a/xen/arch/x86/x86_64/mmconfig-shared.c
> +++ b/xen/arch/x86/x86_64/mmconfig-shared.c
> @@ -89,7 +89,7 @@ static const char __init *pci_mmcfg_intel_945(void)
>  
>      pci_mmcfg_config_num = 1;
>  
> -    pciexbar = pci_conf_read32(0, 0, 0, 0, 0x48);
> +    pciexbar = pci_conf_read32(PCI_SBDF(0, 0, 0, 0), 0x48);
>  
>      /* Enable bit */
>      if (!(pciexbar & 1))
> @@ -213,14 +213,14 @@ static const char __init *pci_mmcfg_nvidia_mcp55(void)
>          u32 l, extcfg;
>          u16 vendor, device;
>  
> -        l = pci_conf_read32(0, bus, 0, 0, 0);
> +        l = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), 0);
>          vendor = l & 0xffff;
>          device = (l >> 16) & 0xffff;
>  
>          if (PCI_VENDOR_ID_NVIDIA != vendor || 0x0369 != device)
>              continue;
>  
> -        extcfg = pci_conf_read32(0, bus, 0, 0, extcfg_regnum);
> +        extcfg = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), extcfg_regnum);
>  
>          if (extcfg & extcfg_enable_mask)
>              i++;
> @@ -239,14 +239,14 @@ static const char __init *pci_mmcfg_nvidia_mcp55(void)
>          u16 vendor, device;
>          int size_index;
>  
> -        l = pci_conf_read32(0, bus, 0, 0, 0);
> +        l = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), 0);
>          vendor = l & 0xffff;
>          device = (l >> 16) & 0xffff;
>  
>          if (PCI_VENDOR_ID_NVIDIA != vendor || 0x0369 != device)
>              continue;
>  
> -        extcfg = pci_conf_read32(0, bus, 0, 0, extcfg_regnum);
> +        extcfg = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), extcfg_regnum);
>  
>          if (!(extcfg & extcfg_enable_mask))
>              continue;
> @@ -312,7 +312,7 @@ static int __init pci_mmcfg_check_hostbridge(void)
>      for (i = 0; !name && i < ARRAY_SIZE(pci_mmcfg_probes); i++) {
>          bus =  pci_mmcfg_probes[i].bus;
>          devfn = pci_mmcfg_probes[i].devfn;
> -        l = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
> +        l = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
>          vendor = l & 0xffff;
>          device = (l >> 16) & 0xffff;
>  
> diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
> index fe36b60c50..b8b82a6fe7 100644
> --- a/xen/arch/x86/x86_64/pci.c
> +++ b/xen/arch/x86/x86_64/pci.c
> @@ -37,28 +37,23 @@ uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg)
>      return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2);
>  }
>  
> -#undef PCI_CONF_ADDRESS
> -#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
> -    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
> -
> -uint32_t pci_conf_read32(
> -    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
> -    unsigned int reg)
> +uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
>  {
> -    u32 value;
> -
> -    if ( seg || reg > 255 )
> +    if ( sbdf.seg || reg > 255 )
>      {
> -        pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 4, &value);
> +        uint32_t value;
> +
> +        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, &value);
>          return value;
>      }
> -    else
> -    {
> -        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
> -        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4);
> -    }
> +
> +    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
>  }
>  
> +#undef PCI_CONF_ADDRESS
> +#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
> +    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
> +
>  void pci_conf_write8(
>      unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
>      unsigned int reg, uint8_t data)
> diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
> index 64258da2dc..9b9025fb33 100644
> --- a/xen/drivers/char/ehci-dbgp.c
> +++ b/xen/drivers/char/ehci-dbgp.c
> @@ -682,7 +682,8 @@ static int dbgp_control_msg(struct ehci_dbgp *dbgp, unsigned int devnum,
>  
>  static unsigned int __init __find_dbgp(u8 bus, u8 slot, u8 func)
>  {
> -    u32 class = pci_conf_read32(0, bus, slot, func, PCI_CLASS_REVISION);
> +    uint32_t class = pci_conf_read32(PCI_SBDF(0, bus, slot, func),
> +                                     PCI_CLASS_REVISION);
>  
>      if ( (class >> 8) != PCI_CLASS_SERIAL_USB_EHCI )
>          return 0;
> @@ -1006,7 +1007,8 @@ static set_debug_port_t __read_mostly set_debug_port = default_set_debug_port;
>  
>  static void nvidia_set_debug_port(struct ehci_dbgp *dbgp, unsigned int port)
>  {
> -    u32 dword = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, 0x74);
> +    uint32_t dword = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot,
> +                                              dbgp->func), 0x74);
>  
>      dword &= ~(0x0f << 12);
>      dword |= (port & 0x0f) << 12;
> @@ -1039,7 +1041,8 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
>      if ( !offset )
>          return;
>  
> -    cap = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, offset);
> +    cap = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
> +                          offset);
>      dbgp_printk("dbgp: EHCI BIOS state %08x\n", cap);
>  
>      if ( (cap & 0xff) == 1 && (cap & EHCI_USBLEGSUP_BIOS) )
> @@ -1054,7 +1057,8 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
>      {
>          mdelay(10);
>          msec -= 10;
> -        cap = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, offset);
> +        cap = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
> +                              offset);
>      }
>  
>      if ( cap & EHCI_USBLEGSUP_BIOS )
> @@ -1307,7 +1311,7 @@ static void __init ehci_dbgp_init_preirq(struct serial_port *port)
>      u32 debug_port, offset;
>      void __iomem *ehci_bar;
>  
> -    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
> +    debug_port = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
>                                   dbgp->cap);
>      offset = (debug_port >> 16) & 0xfff;
>  
> @@ -1504,7 +1508,7 @@ void __init ehci_dbgp_init(void)
>      else
>          return;
>  
> -    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
> +    debug_port = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
>                                   dbgp->cap);
>      dbgp->bar = (debug_port >> 29) & 0x7;
>      dbgp->bar = ((dbgp->bar - 1) * 4) + PCI_BASE_ADDRESS_0;
> @@ -1516,8 +1520,8 @@ void __init ehci_dbgp_init(void)
>          return;
>      }
>  
> -    dbgp->bar_val = bar_val = pci_conf_read32(0, dbgp->bus, dbgp->slot,
> -                                              dbgp->func, dbgp->bar);
> +    dbgp->bar_val = bar_val = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot,
> +                                                       dbgp->func), dbgp->bar);
>      dbgp_printk("bar_val: %08x\n", bar_val);
>      if ( bar_val & ~PCI_BASE_ADDRESS_MEM_MASK )
>      {
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 99c1254cac..fe71406cc1 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1112,27 +1112,28 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>                  }
>  
>                  uart->io_base = 0;
> -                bar = pci_conf_read32(0, b, d, f,
> -                                      PCI_BASE_ADDRESS_0 + bar_idx*4);
> +                bar = pci_conf_read32(PCI_SBDF(0, b, d, f),
> +                                      PCI_BASE_ADDRESS_0 + bar_idx * 4);
>  
>                  /* MMIO based */
>                  if ( param->mmio && !(bar & PCI_BASE_ADDRESS_SPACE_IO) )
>                  {
>                      pci_conf_write32(0, b, d, f,
>                                       PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
> -                    len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0 + bar_idx*4);
> +                    len = pci_conf_read32(PCI_SBDF(0, b, d, f),
> +                                          PCI_BASE_ADDRESS_0 + bar_idx * 4);
>                      pci_conf_write32(0, b, d, f,
>                                       PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
>  
>                      /* Handle 64 bit BAR if found */
>                      if ( bar & PCI_BASE_ADDRESS_MEM_TYPE_64 )
>                      {
> -                        bar_64 = pci_conf_read32(0, b, d, f,
> -                                      PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
> +                        bar_64 = pci_conf_read32(PCI_SBDF(0, b, d, f),
> +                                      PCI_BASE_ADDRESS_0 + (bar_idx + 1) * 4);
>                          pci_conf_write32(0, b, d, f,
>                                      PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, ~0u);
> -                        len_64 = pci_conf_read32(0, b, d, f,
> -                                    PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
> +                        len_64 = pci_conf_read32(PCI_SBDF(0, b, d, f),
> +                                    PCI_BASE_ADDRESS_0 + (bar_idx + 1) * 4);
>                          pci_conf_write32(0, b, d, f,
>                                      PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, bar_64);
>                          size  = ((u64)~0 << 32) | PCI_BASE_ADDRESS_MEM_MASK;
> @@ -1149,7 +1150,8 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>                  {
>                      pci_conf_write32(0, b, d, f,
>                                       PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
> -                    len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
> +                    len = pci_conf_read32(PCI_SBDF(0, b, d, f),
> +                                          PCI_BASE_ADDRESS_0);
>                      pci_conf_write32(0, b, d, f,
>                                       PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
>                      size = len & PCI_BASE_ADDRESS_IO_MASK;
> diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/amd/iommu_detect.c
> index 3c5d4de1a3..069df156de 100644
> --- a/xen/drivers/passthrough/amd/iommu_detect.c
> +++ b/xen/drivers/passthrough/amd/iommu_detect.c
> @@ -48,7 +48,7 @@ static int __init get_iommu_capabilities(
>  {
>      u8 type;
>  
> -    iommu->cap.header = pci_conf_read32(seg, bus, dev, func, cap_ptr);
> +    iommu->cap.header = pci_conf_read32(PCI_SBDF(seg, bus, dev, func), cap_ptr);
>      type = get_field_from_reg_u32(iommu->cap.header, PCI_CAP_TYPE_MASK,
>                                    PCI_CAP_TYPE_SHIFT);
>  
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
> index 1b3e7de10d..6083d51b91 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -844,7 +844,7 @@ static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
>          return;
>  
>      pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
> -    value = pci_conf_read32(iommu->seg, bus, dev, func, 0xf4);
> +    value = pci_conf_read32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf4);
>  
>      if ( value & (1 << 2) )
>          return;
> @@ -1231,7 +1231,7 @@ static bool_t __init amd_sp5100_erratum28(void)
>  
>      for (bus = 0; bus < 256; bus++)
>      {
> -        id = pci_conf_read32(0, bus, 0x14, 0, PCI_VENDOR_ID);
> +        id = pci_conf_read32(PCI_SBDF(0, bus, 0x14, 0), PCI_VENDOR_ID);
>  
>          vendor_id = id & 0xffff;
>          dev_id = (id >> 16) & 0xffff;
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 703056f7b9..80887af66c 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -632,8 +632,7 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
>                                uint64_t *paddr, uint64_t *psize,
>                                unsigned int flags)
>  {
> -    uint32_t hi = 0, bar = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev,
> -                                           sbdf.fn, pos);
> +    uint32_t hi = 0, bar = pci_conf_read32(sbdf, pos);
>      uint64_t size;
>      bool is64bits = !(flags & PCI_BAR_ROM) &&
>          (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64;
> @@ -655,15 +654,13 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
>              *psize = 0;
>              return 1;
>          }
> -        hi = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4);
> +        hi = pci_conf_read32(sbdf, pos + 4);
>          pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, ~0);
>      }
> -    size = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
> -                           pos) & mask;
> +    size = pci_conf_read32(sbdf, pos) & mask;
>      if ( is64bits )
>      {
> -        size |= (uint64_t)pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev,
> -                                          sbdf.fn, pos + 4) << 32;
> +        size |= (uint64_t)pci_conf_read32(sbdf, pos + 4) << 32;
>          pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, hi);
>      }
>      else if ( size )
> @@ -750,7 +747,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>              for ( i = 0; i < PCI_SRIOV_NUM_BARS; )
>              {
>                  unsigned int idx = pos + PCI_SRIOV_BAR + i * 4;
> -                u32 bar = pci_conf_read32(seg, bus, slot, func, idx);
> +                u32 bar = pci_conf_read32(pdev->sbdf, idx);
>                  pci_sbdf_t sbdf = PCI_SBDF3(seg, bus, devfn);
>  
>                  if ( (bar & PCI_BASE_ADDRESS_SPACE) ==
> @@ -1002,7 +999,7 @@ bool_t __init pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func)
>  {
>      u32 vendor;
>  
> -    vendor = pci_conf_read32(seg, bus, dev, func, PCI_VENDOR_ID);
> +    vendor = pci_conf_read32(PCI_SBDF(seg, bus, dev, func), PCI_VENDOR_ID);
>      /* some broken boards return 0 or ~0 if a slot is empty: */
>      if ( (vendor == 0xffffffff) || (vendor == 0x00000000) ||
>           (vendor == 0x0000ffff) || (vendor == 0xffff0000) )
> diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
> index 47597c9600..28e9597014 100644
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -128,9 +128,11 @@ static void __init map_igd_reg(void)
>      if ( igd_reg_va )
>          return;
>  
> -    igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
> +    igd_mmio   = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
> +                                 PCI_BASE_ADDRESS_1);
>      igd_mmio <<= 32;
> -    igd_mmio  += pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_0);
> +    igd_mmio  += pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
> +                                 PCI_BASE_ADDRESS_0);
>      igd_reg_va = ioremap(igd_mmio & IGD_BAR_MASK, 0x3000);
>  }
>  
> @@ -280,7 +282,7 @@ static void __init tylersburg_intremap_quirk(void)
>      for ( bus = 0; bus < 0x100; bus++ )
>      {
>          /* Match on System Management Registers on Device 20 Function 0 */
> -        device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
> +        device = pci_conf_read32(PCI_SBDF(0, bus, 20, 0), PCI_VENDOR_ID);
>          rev = pci_conf_read8(PCI_SBDF(0, bus, 20, 0), PCI_REVISION_ID);
>  
>          if ( rev == 0x13 && device == 0x342e8086 )
> @@ -296,8 +298,8 @@ static void __init tylersburg_intremap_quirk(void)
>  /* initialize platform identification flags */
>  void __init platform_quirks_init(void)
>  {
> -    ioh_id = pci_conf_read32(0, 0, IOH_DEV, 0, 0);
> -    igd_id = pci_conf_read32(0, 0, IGD_DEV, 0, 0);
> +    ioh_id = pci_conf_read32(PCI_SBDF(0, 0, IOH_DEV, 0), 0);
> +    igd_id = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0), 0);
>  
>      /* Mobile 4 Series Chipset neglects to set RWBF capability. */
>      if ( ioh_id == 0x2a408086 )
> @@ -356,15 +358,15 @@ int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
>      u32 id;
>      int rc = 0;
>  
> -    id = pci_conf_read32(0, 0, 0, 0, 0);
> +    id = pci_conf_read32(PCI_SBDF(0, 0, 0, 0), 0);
>      if ( IS_CTG(id) )
>      {
>          /* quit if ME does not exist */
> -        if ( pci_conf_read32(0, 0, 3, 0, 0) == 0xffffffff )
> +        if ( pci_conf_read32(PCI_SBDF(0, 0, 3, 0), 0) == 0xffffffff )
>              return 0;
>  
>          /* if device is WLAN device, map ME phantom device 0:3.7 */
> -        id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
> +        id = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
>          switch (id)
>          {
>              case 0x42328086:
> @@ -384,11 +386,11 @@ int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
>      else if ( IS_ILK(id) || IS_CPT(id) )
>      {
>          /* quit if ME does not exist */
> -        if ( pci_conf_read32(0, 0, 22, 0, 0) == 0xffffffff )
> +        if ( pci_conf_read32(PCI_SBDF(0, 0, 22, 0), 0) == 0xffffffff )
>              return 0;
>  
>          /* if device is WLAN device, map ME phantom device 0:22.7 */
> -        id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
> +        id = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
>          switch (id)
>          {
>              case 0x00878086:        /* Kilmer Peak */
> @@ -438,7 +440,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>      case 0x342e: /* Tylersburg chipset (Nehalem / Westmere systems) */
>      case 0x3728: /* Xeon C5500/C3500 (JasperForest) */
>      case 0x3c28: /* Sandybridge */
> -        val = pci_conf_read32(seg, bus, dev, func, 0x1AC);
> +        val = pci_conf_read32(pdev->sbdf, 0x1AC);
>          pci_conf_write32(seg, bus, dev, func, 0x1AC, val | (1 << 31));
>          printk(XENLOG_INFO "Masked VT-d error signaling on %04x:%02x:%02x.%u\n",
>                 seg, bus, dev, func);
> @@ -461,7 +463,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>                                            PCI_EXT_CAP_ID_VNDR);
>              while ( pos )
>              {
> -                val = pci_conf_read32(seg, bus, dev, func, pos + PCI_VNDR_HEADER);
> +                val = pci_conf_read32(pdev->sbdf, pos + PCI_VNDR_HEADER);
>                  if ( PCI_VNDR_HEADER_ID(val) == 4 && PCI_VNDR_HEADER_REV(val) == 1 )
>                  {
>                      pos += PCI_VNDR_HEADER;
> @@ -481,8 +483,8 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>              break;
>          }
>  
> -        val = pci_conf_read32(seg, bus, dev, func, pos + PCI_ERR_UNCOR_MASK);
> -        val2 = pci_conf_read32(seg, bus, dev, func, pos + PCI_ERR_COR_MASK);
> +        val = pci_conf_read32(pdev->sbdf, pos + PCI_ERR_UNCOR_MASK);
> +        val2 = pci_conf_read32(pdev->sbdf, pos + PCI_ERR_COR_MASK);
>          if ( (val & PCI_ERR_UNC_UNSUP) && (val2 & PCI_ERR_COR_ADV_NFAT) )
>              action = "Found masked";
>          else if ( !ff )
> @@ -497,7 +499,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>              action = "Must not mask";
>  
>          /* XPUNCERRMSK Send Completion with Unsupported Request */
> -        val = pci_conf_read32(seg, bus, dev, func, 0x20c);
> +        val = pci_conf_read32(pdev->sbdf, 0x20c);
>          pci_conf_write32(seg, bus, dev, func, 0x20c, val | (1 << 4));
>  
>          printk(XENLOG_INFO "%s UR signaling on %04x:%02x:%02x.%u\n",
> @@ -514,8 +516,8 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>      case 0x1610: case 0x1614: case 0x1618: /* Broadwell */
>      case 0x1900: case 0x1904: case 0x1908: case 0x190c: case 0x190f: /* Skylake */
>      case 0x1910: case 0x1918: case 0x191f: /* Skylake */
> -        bar = pci_conf_read32(seg, bus, dev, func, 0x6c);
> -        bar = (bar << 32) | pci_conf_read32(seg, bus, dev, func, 0x68);
> +        bar = pci_conf_read32(pdev->sbdf, 0x6c);
> +        bar = (bar << 32) | pci_conf_read32(pdev->sbdf, 0x68);
>          pa = bar & 0x7ffffff000UL; /* bits 12...38 */
>          if ( (bar & 1) && pa &&
>               page_is_ram_type(paddr_to_pfn(pa), RAM_TYPE_RESERVED) )
> diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
> index 5e5e0f0538..b24702e0c3 100644
> --- a/xen/drivers/pci/pci.c
> +++ b/xen/drivers/pci/pci.c
> @@ -93,7 +93,7 @@ int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap
>      int ttl = 480; /* 3840 bytes, minimum 8 bytes per capability */
>      int pos = max(start, 0x100);
>  
> -    header = pci_conf_read32(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
> +    header = pci_conf_read32(PCI_SBDF3(seg, bus, devfn), pos);
>  
>      /*
>       * If we have no capabilities, this is indicated by cap ID,
> @@ -109,7 +109,7 @@ int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap
>          pos = PCI_EXT_CAP_NEXT(header);
>          if ( pos < 0x100 )
>              break;
> -        header = pci_conf_read32(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
> +        header = pci_conf_read32(PCI_SBDF3(seg, bus, devfn), pos);
>      }
>      return 0;
>  }
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 0b176b490a..7476634982 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -511,7 +511,7 @@ static int init_bars(struct pci_dev *pdev)
>              continue;
>          }
>  
> -        val = pci_conf_read32(pdev->seg, pdev->bus, slot, func, reg);
> +        val = pci_conf_read32(pdev->sbdf, reg);
>          if ( (val & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO )
>          {
>              bars[i].type = VPCI_BAR_IO;
> @@ -561,8 +561,8 @@ static int init_bars(struct pci_dev *pdev)
>          rom->type = VPCI_BAR_ROM;
>          rom->size = size;
>          rom->addr = addr;
> -        header->rom_enabled = pci_conf_read32(pdev->seg, pdev->bus, slot, func,
> -                                              rom_reg) & PCI_ROM_ADDRESS_ENABLE;
> +        header->rom_enabled = pci_conf_read32(pdev->sbdf, rom_reg) &
> +                              PCI_ROM_ADDRESS_ENABLE;
>  
>          rc = vpci_add_register(pdev->vpci, vpci_hw_read32, rom_write, rom_reg,
>                                 4, rom);
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 8e6cd070d0..c60cba0137 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -469,11 +469,9 @@ static int init_msix(struct pci_dev *pdev)
>      pdev->vpci->msix->pdev = pdev;
>  
>      pdev->vpci->msix->tables[VPCI_MSIX_TABLE] =
> -        pci_conf_read32(pdev->seg, pdev->bus, slot, func,
> -                        msix_table_offset_reg(msix_offset));
> +        pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
>      pdev->vpci->msix->tables[VPCI_MSIX_PBA] =
> -        pci_conf_read32(pdev->seg, pdev->bus, slot, func,
> -                        msix_pba_offset_reg(msix_offset));
> +        pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
>  
>      for ( i = 0; i < pdev->vpci->msix->max_entries; i++)
>      {
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 1a4c2ee4f1..2106255863 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -120,8 +120,7 @@ uint32_t vpci_hw_read16(const struct pci_dev *pdev, unsigned int reg,
>  uint32_t vpci_hw_read32(const struct pci_dev *pdev, unsigned int reg,
>                          void *data)
>  {
> -    return pci_conf_read32(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> -                           PCI_FUNC(pdev->devfn), reg);
> +    return pci_conf_read32(pdev->sbdf, reg);
>  }
>  
>  int vpci_add_register(struct vpci *vpci, vpci_read_t *read_handler,
> @@ -211,7 +210,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
>      switch ( size )
>      {
>      case 4:
> -        data = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg);
> +        data = pci_conf_read32(sbdf, reg);
>          break;
>  
>      case 3:
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index cf4c223f7c..2a5705560e 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -174,9 +174,7 @@ void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
>  
>  uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg);
>  uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg);
> -uint32_t pci_conf_read32(
> -    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
> -    unsigned int reg);
> +uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg);
>  void pci_conf_write8(
>      unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
>      unsigned int reg, uint8_t data);
> -- 
> 2.20.1 (Apple Git-117)
>
Tian, Kevin June 28, 2019, 2:03 a.m. UTC | #5
> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> Sent: Friday, June 7, 2019 5:22 PM
> 
> This reduces the number of parameters of the function to two, and
> simplifies some of the calling sites.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>


Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 3c069391f4..37f60c0862 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -417,7 +417,8 @@  static void disable_c1_ramping(void)
 	int node, nr_nodes;
 
 	/* Read the number of nodes from the first Northbridge. */
-	nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1;
+	nr_nodes = ((pci_conf_read32(PCI_SBDF(0, 0, 0x18, 0), 0x60) >> 4) &
+		    0x07) + 1;
 	for (node = 0; node < nr_nodes; node++) {
 		/* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */
 		pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87);
@@ -696,8 +697,8 @@  static void init_amd(struct cpuinfo_x86 *c)
 
 	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
 		if (c == &boot_cpu_data) {
-			l = pci_conf_read32(0, 0, 0x18, 0x3, 0x58);
-			h = pci_conf_read32(0, 0, 0x18, 0x3, 0x5c);
+			l = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x58);
+			h = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x5c);
 			if ((l & 0x1f) | (h & 0x1))
 				printk(KERN_WARNING
 				       "Applying workaround for erratum 792: %s%s%s\n",
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index df2c0130f1..e67119dbe6 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5949,7 +5949,7 @@  const struct platform_bad_page *__init get_platform_badpages(unsigned int *array
     }
 
     *array_size = ARRAY_SIZE(snb_bad_pages);
-    igd_id = pci_conf_read32(0, 0, 2, 0, 0);
+    igd_id = pci_conf_read32(PCI_SBDF(0, 0, 2, 0), 0);
     if ( IS_SNB_GFX(igd_id) )
         return snb_bad_pages;
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index ed986261c3..392cbecfe4 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -191,16 +191,13 @@  static bool read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
     {
         struct pci_dev *dev = entry->dev;
         int pos = entry->msi_attrib.pos;
-        u16 data, seg = dev->seg;
-        u8 bus = dev->bus;
-        u8 slot = PCI_SLOT(dev->devfn);
-        u8 func = PCI_FUNC(dev->devfn);
+        uint16_t data;
 
-        msg->address_lo = pci_conf_read32(seg, bus, slot, func,
+        msg->address_lo = pci_conf_read32(dev->sbdf,
                                           msi_lower_address_reg(pos));
         if ( entry->msi_attrib.is_64 )
         {
-            msg->address_hi = pci_conf_read32(seg, bus, slot, func,
+            msg->address_hi = pci_conf_read32(dev->sbdf,
                                               msi_upper_address_reg(pos));
             data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 1));
         }
@@ -396,7 +393,7 @@  static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
         {
             u32 mask_bits;
 
-            mask_bits = pci_conf_read32(seg, bus, slot, func, entry->msi.mpos);
+            mask_bits = pci_conf_read32(pdev->sbdf, entry->msi.mpos);
             mask_bits &= ~((u32)1 << entry->msi_attrib.entry_nr);
             mask_bits |= (u32)flag << entry->msi_attrib.entry_nr;
             pci_conf_write32(seg, bus, slot, func, entry->msi.mpos, mask_bits);
@@ -465,10 +462,7 @@  static int msi_get_mask_bit(const struct msi_desc *entry)
     case PCI_CAP_ID_MSI:
         if ( !entry->msi_attrib.maskbit )
             break;
-        return (pci_conf_read32(entry->dev->seg, entry->dev->bus,
-                                PCI_SLOT(entry->dev->devfn),
-                                PCI_FUNC(entry->dev->devfn),
-                                entry->msi.mpos) >>
+        return (pci_conf_read32(entry->dev->sbdf, entry->msi.mpos) >>
                 entry->msi_attrib.entry_nr) & 1;
     case PCI_CAP_ID_MSIX:
         if ( unlikely(!msix_memory_decoded(entry->dev,
@@ -723,7 +717,7 @@  static int msi_capability_init(struct pci_dev *dev,
         u32 maskbits;
 
         /* All MSIs are unmasked by default, Mask them all */
-        maskbits = pci_conf_read32(seg, bus, slot, func, mpos);
+        maskbits = pci_conf_read32(dev->sbdf, mpos);
         maskbits |= ~(u32)0 >> (32 - maxvec);
         pci_conf_write32(seg, bus, slot, func, mpos, maskbits);
     }
@@ -808,7 +802,7 @@  static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
 
     if ( bir >= limit )
         return 0;
-    addr = pci_conf_read32(seg, bus, slot, func, base + bir * 4);
+    addr = pci_conf_read32(PCI_SBDF(seg, bus, slot, func), base + bir * 4);
     if ( (addr & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO )
         return 0;
     if ( (addr & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64 )
@@ -817,7 +811,7 @@  static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
         if ( ++bir >= limit )
             return 0;
         return addr + disp +
-               ((u64)pci_conf_read32(seg, bus, slot, func,
+               ((u64)pci_conf_read32(PCI_SBDF(seg, bus, slot, func),
                                      base + bir * 4) << 32);
     }
     return (addr & PCI_BASE_ADDRESS_MEM_MASK) + disp;
@@ -886,8 +880,7 @@  static int msix_capability_init(struct pci_dev *dev,
     }
 
     /* Locate MSI-X table region */
-    table_offset = pci_conf_read32(seg, bus, slot, func,
-                                   msix_table_offset_reg(pos));
+    table_offset = pci_conf_read32(dev->sbdf, msix_table_offset_reg(pos));
     bir = (u8)(table_offset & PCI_MSIX_BIRMASK);
     table_offset &= ~PCI_MSIX_BIRMASK;
 
@@ -933,8 +926,7 @@  static int msix_capability_init(struct pci_dev *dev,
         WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->table.first,
                                         msix->table.last));
 
-        pba_offset = pci_conf_read32(seg, bus, slot, func,
-                                     msix_pba_offset_reg(pos));
+        pba_offset = pci_conf_read32(dev->sbdf, msix_pba_offset_reg(pos));
         bir = (u8)(pba_offset & PCI_MSIX_BIRMASK);
         pba_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf);
         WARN_ON(!pba_paddr);
diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
index 3d6e26f636..3bf0b0214d 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -463,7 +463,8 @@  static int __init init_ibs_nmi(void)
 	for (bus = 0; bus < 256; bus++) {
 		for (dev = 0; dev < 32; dev++) {
 			for (func = 0; func < 8; func++) {
-				id = pci_conf_read32(0, bus, dev, func, PCI_VENDOR_ID);
+				id = pci_conf_read32(PCI_SBDF(0, bus, dev, func),
+						     PCI_VENDOR_ID);
 
 				vendor_id = id & 0xffff;
 				dev_id = (id >> 16) & 0xffff;
@@ -474,7 +475,8 @@  static int __init init_ibs_nmi(void)
 					pci_conf_write32(0, bus, dev, func, IBSCTL,
 						IBSCTL_LVTOFFSETVAL | APIC_EILVT_LVTOFF_IBS);
 
-					value = pci_conf_read32(0, bus, dev, func, IBSCTL);
+					value = pci_conf_read32(PCI_SBDF(0, bus, dev, func),
+								IBSCTL);
 
 					if (value != (IBSCTL_LVTOFFSETVAL |
 						APIC_EILVT_LVTOFF_IBS)) {
diff --git a/xen/arch/x86/x86_64/mmconf-fam10h.c b/xen/arch/x86/x86_64/mmconf-fam10h.c
index ed0acb9968..f997688ad4 100644
--- a/xen/arch/x86/x86_64/mmconf-fam10h.c
+++ b/xen/arch/x86/x86_64/mmconf-fam10h.c
@@ -52,7 +52,7 @@  static void __init get_fam10h_pci_mmconf_base(void)
 
 		bus = pci_probes[i].bus;
 		slot = pci_probes[i].slot;
-		id = pci_conf_read32(0, bus, slot, 0, PCI_VENDOR_ID);
+		id = pci_conf_read32(PCI_SBDF(0, bus, slot, 0), PCI_VENDOR_ID);
 
 		vendor = id & 0xffff;
 		device = (id>>16) & 0xffff;
@@ -83,12 +83,14 @@  static void __init get_fam10h_pci_mmconf_base(void)
 	 * above 4G
 	 */
 	for (hi_mmio_num = i = 0; i < 8; i++) {
-		val = pci_conf_read32(0, bus, slot, 1, 0x80 + (i << 3));
+		val = pci_conf_read32(PCI_SBDF(0, bus, slot, 1),
+				      0x80 + (i << 3));
 		if (!(val & 3))
 			continue;
 
 		start = (val & 0xffffff00) << 8; /* 39:16 on 31:8*/
-		val = pci_conf_read32(0, bus, slot, 1, 0x84 + (i << 3));
+		val = pci_conf_read32(PCI_SBDF(0, bus, slot, 1),
+				      0x84 + (i << 3));
 		end = ((val & 0xffffff00) << 8) | 0xffff; /* 39:16 on 31:8*/
 
 		if (end < tom2)
diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
index 9d1db590d9..cc08b52a35 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -89,7 +89,7 @@  static const char __init *pci_mmcfg_intel_945(void)
 
     pci_mmcfg_config_num = 1;
 
-    pciexbar = pci_conf_read32(0, 0, 0, 0, 0x48);
+    pciexbar = pci_conf_read32(PCI_SBDF(0, 0, 0, 0), 0x48);
 
     /* Enable bit */
     if (!(pciexbar & 1))
@@ -213,14 +213,14 @@  static const char __init *pci_mmcfg_nvidia_mcp55(void)
         u32 l, extcfg;
         u16 vendor, device;
 
-        l = pci_conf_read32(0, bus, 0, 0, 0);
+        l = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), 0);
         vendor = l & 0xffff;
         device = (l >> 16) & 0xffff;
 
         if (PCI_VENDOR_ID_NVIDIA != vendor || 0x0369 != device)
             continue;
 
-        extcfg = pci_conf_read32(0, bus, 0, 0, extcfg_regnum);
+        extcfg = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), extcfg_regnum);
 
         if (extcfg & extcfg_enable_mask)
             i++;
@@ -239,14 +239,14 @@  static const char __init *pci_mmcfg_nvidia_mcp55(void)
         u16 vendor, device;
         int size_index;
 
-        l = pci_conf_read32(0, bus, 0, 0, 0);
+        l = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), 0);
         vendor = l & 0xffff;
         device = (l >> 16) & 0xffff;
 
         if (PCI_VENDOR_ID_NVIDIA != vendor || 0x0369 != device)
             continue;
 
-        extcfg = pci_conf_read32(0, bus, 0, 0, extcfg_regnum);
+        extcfg = pci_conf_read32(PCI_SBDF(0, bus, 0, 0), extcfg_regnum);
 
         if (!(extcfg & extcfg_enable_mask))
             continue;
@@ -312,7 +312,7 @@  static int __init pci_mmcfg_check_hostbridge(void)
     for (i = 0; !name && i < ARRAY_SIZE(pci_mmcfg_probes); i++) {
         bus =  pci_mmcfg_probes[i].bus;
         devfn = pci_mmcfg_probes[i].devfn;
-        l = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
+        l = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
         vendor = l & 0xffff;
         device = (l >> 16) & 0xffff;
 
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index fe36b60c50..b8b82a6fe7 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -37,28 +37,23 @@  uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg)
     return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-uint32_t pci_conf_read32(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg)
+uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
 {
-    u32 value;
-
-    if ( seg || reg > 255 )
+    if ( sbdf.seg || reg > 255 )
     {
-        pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 4, &value);
+        uint32_t value;
+
+        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, &value);
         return value;
     }
-    else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4);
-    }
+
+    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 void pci_conf_write8(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint8_t data)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 64258da2dc..9b9025fb33 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -682,7 +682,8 @@  static int dbgp_control_msg(struct ehci_dbgp *dbgp, unsigned int devnum,
 
 static unsigned int __init __find_dbgp(u8 bus, u8 slot, u8 func)
 {
-    u32 class = pci_conf_read32(0, bus, slot, func, PCI_CLASS_REVISION);
+    uint32_t class = pci_conf_read32(PCI_SBDF(0, bus, slot, func),
+                                     PCI_CLASS_REVISION);
 
     if ( (class >> 8) != PCI_CLASS_SERIAL_USB_EHCI )
         return 0;
@@ -1006,7 +1007,8 @@  static set_debug_port_t __read_mostly set_debug_port = default_set_debug_port;
 
 static void nvidia_set_debug_port(struct ehci_dbgp *dbgp, unsigned int port)
 {
-    u32 dword = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, 0x74);
+    uint32_t dword = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot,
+                                              dbgp->func), 0x74);
 
     dword &= ~(0x0f << 12);
     dword |= (port & 0x0f) << 12;
@@ -1039,7 +1041,8 @@  static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
     if ( !offset )
         return;
 
-    cap = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, offset);
+    cap = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                          offset);
     dbgp_printk("dbgp: EHCI BIOS state %08x\n", cap);
 
     if ( (cap & 0xff) == 1 && (cap & EHCI_USBLEGSUP_BIOS) )
@@ -1054,7 +1057,8 @@  static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
     {
         mdelay(10);
         msec -= 10;
-        cap = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func, offset);
+        cap = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                              offset);
     }
 
     if ( cap & EHCI_USBLEGSUP_BIOS )
@@ -1307,7 +1311,7 @@  static void __init ehci_dbgp_init_preirq(struct serial_port *port)
     u32 debug_port, offset;
     void __iomem *ehci_bar;
 
-    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
+    debug_port = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                                  dbgp->cap);
     offset = (debug_port >> 16) & 0xfff;
 
@@ -1504,7 +1508,7 @@  void __init ehci_dbgp_init(void)
     else
         return;
 
-    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
+    debug_port = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                                  dbgp->cap);
     dbgp->bar = (debug_port >> 29) & 0x7;
     dbgp->bar = ((dbgp->bar - 1) * 4) + PCI_BASE_ADDRESS_0;
@@ -1516,8 +1520,8 @@  void __init ehci_dbgp_init(void)
         return;
     }
 
-    dbgp->bar_val = bar_val = pci_conf_read32(0, dbgp->bus, dbgp->slot,
-                                              dbgp->func, dbgp->bar);
+    dbgp->bar_val = bar_val = pci_conf_read32(PCI_SBDF(0, dbgp->bus, dbgp->slot,
+                                                       dbgp->func), dbgp->bar);
     dbgp_printk("bar_val: %08x\n", bar_val);
     if ( bar_val & ~PCI_BASE_ADDRESS_MEM_MASK )
     {
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 99c1254cac..fe71406cc1 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1112,27 +1112,28 @@  pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 }
 
                 uart->io_base = 0;
-                bar = pci_conf_read32(0, b, d, f,
-                                      PCI_BASE_ADDRESS_0 + bar_idx*4);
+                bar = pci_conf_read32(PCI_SBDF(0, b, d, f),
+                                      PCI_BASE_ADDRESS_0 + bar_idx * 4);
 
                 /* MMIO based */
                 if ( param->mmio && !(bar & PCI_BASE_ADDRESS_SPACE_IO) )
                 {
                     pci_conf_write32(0, b, d, f,
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
-                    len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0 + bar_idx*4);
+                    len = pci_conf_read32(PCI_SBDF(0, b, d, f),
+                                          PCI_BASE_ADDRESS_0 + bar_idx * 4);
                     pci_conf_write32(0, b, d, f,
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
 
                     /* Handle 64 bit BAR if found */
                     if ( bar & PCI_BASE_ADDRESS_MEM_TYPE_64 )
                     {
-                        bar_64 = pci_conf_read32(0, b, d, f,
-                                      PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
+                        bar_64 = pci_conf_read32(PCI_SBDF(0, b, d, f),
+                                      PCI_BASE_ADDRESS_0 + (bar_idx + 1) * 4);
                         pci_conf_write32(0, b, d, f,
                                     PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, ~0u);
-                        len_64 = pci_conf_read32(0, b, d, f,
-                                    PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
+                        len_64 = pci_conf_read32(PCI_SBDF(0, b, d, f),
+                                    PCI_BASE_ADDRESS_0 + (bar_idx + 1) * 4);
                         pci_conf_write32(0, b, d, f,
                                     PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, bar_64);
                         size  = ((u64)~0 << 32) | PCI_BASE_ADDRESS_MEM_MASK;
@@ -1149,7 +1150,8 @@  pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 {
                     pci_conf_write32(0, b, d, f,
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
-                    len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
+                    len = pci_conf_read32(PCI_SBDF(0, b, d, f),
+                                          PCI_BASE_ADDRESS_0);
                     pci_conf_write32(0, b, d, f,
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
                     size = len & PCI_BASE_ADDRESS_IO_MASK;
diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/amd/iommu_detect.c
index 3c5d4de1a3..069df156de 100644
--- a/xen/drivers/passthrough/amd/iommu_detect.c
+++ b/xen/drivers/passthrough/amd/iommu_detect.c
@@ -48,7 +48,7 @@  static int __init get_iommu_capabilities(
 {
     u8 type;
 
-    iommu->cap.header = pci_conf_read32(seg, bus, dev, func, cap_ptr);
+    iommu->cap.header = pci_conf_read32(PCI_SBDF(seg, bus, dev, func), cap_ptr);
     type = get_field_from_reg_u32(iommu->cap.header, PCI_CAP_TYPE_MASK,
                                   PCI_CAP_TYPE_SHIFT);
 
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 1b3e7de10d..6083d51b91 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -844,7 +844,7 @@  static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
         return;
 
     pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
-    value = pci_conf_read32(iommu->seg, bus, dev, func, 0xf4);
+    value = pci_conf_read32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf4);
 
     if ( value & (1 << 2) )
         return;
@@ -1231,7 +1231,7 @@  static bool_t __init amd_sp5100_erratum28(void)
 
     for (bus = 0; bus < 256; bus++)
     {
-        id = pci_conf_read32(0, bus, 0x14, 0, PCI_VENDOR_ID);
+        id = pci_conf_read32(PCI_SBDF(0, bus, 0x14, 0), PCI_VENDOR_ID);
 
         vendor_id = id & 0xffff;
         dev_id = (id >> 16) & 0xffff;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 703056f7b9..80887af66c 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -632,8 +632,7 @@  unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
                               uint64_t *paddr, uint64_t *psize,
                               unsigned int flags)
 {
-    uint32_t hi = 0, bar = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev,
-                                           sbdf.fn, pos);
+    uint32_t hi = 0, bar = pci_conf_read32(sbdf, pos);
     uint64_t size;
     bool is64bits = !(flags & PCI_BAR_ROM) &&
         (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64;
@@ -655,15 +654,13 @@  unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
             *psize = 0;
             return 1;
         }
-        hi = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4);
+        hi = pci_conf_read32(sbdf, pos + 4);
         pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, ~0);
     }
-    size = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
-                           pos) & mask;
+    size = pci_conf_read32(sbdf, pos) & mask;
     if ( is64bits )
     {
-        size |= (uint64_t)pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev,
-                                          sbdf.fn, pos + 4) << 32;
+        size |= (uint64_t)pci_conf_read32(sbdf, pos + 4) << 32;
         pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, hi);
     }
     else if ( size )
@@ -750,7 +747,7 @@  int pci_add_device(u16 seg, u8 bus, u8 devfn,
             for ( i = 0; i < PCI_SRIOV_NUM_BARS; )
             {
                 unsigned int idx = pos + PCI_SRIOV_BAR + i * 4;
-                u32 bar = pci_conf_read32(seg, bus, slot, func, idx);
+                u32 bar = pci_conf_read32(pdev->sbdf, idx);
                 pci_sbdf_t sbdf = PCI_SBDF3(seg, bus, devfn);
 
                 if ( (bar & PCI_BASE_ADDRESS_SPACE) ==
@@ -1002,7 +999,7 @@  bool_t __init pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func)
 {
     u32 vendor;
 
-    vendor = pci_conf_read32(seg, bus, dev, func, PCI_VENDOR_ID);
+    vendor = pci_conf_read32(PCI_SBDF(seg, bus, dev, func), PCI_VENDOR_ID);
     /* some broken boards return 0 or ~0 if a slot is empty: */
     if ( (vendor == 0xffffffff) || (vendor == 0x00000000) ||
          (vendor == 0x0000ffff) || (vendor == 0xffff0000) )
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 47597c9600..28e9597014 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -128,9 +128,11 @@  static void __init map_igd_reg(void)
     if ( igd_reg_va )
         return;
 
-    igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
+    igd_mmio   = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
+                                 PCI_BASE_ADDRESS_1);
     igd_mmio <<= 32;
-    igd_mmio  += pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_0);
+    igd_mmio  += pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0),
+                                 PCI_BASE_ADDRESS_0);
     igd_reg_va = ioremap(igd_mmio & IGD_BAR_MASK, 0x3000);
 }
 
@@ -280,7 +282,7 @@  static void __init tylersburg_intremap_quirk(void)
     for ( bus = 0; bus < 0x100; bus++ )
     {
         /* Match on System Management Registers on Device 20 Function 0 */
-        device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
+        device = pci_conf_read32(PCI_SBDF(0, bus, 20, 0), PCI_VENDOR_ID);
         rev = pci_conf_read8(PCI_SBDF(0, bus, 20, 0), PCI_REVISION_ID);
 
         if ( rev == 0x13 && device == 0x342e8086 )
@@ -296,8 +298,8 @@  static void __init tylersburg_intremap_quirk(void)
 /* initialize platform identification flags */
 void __init platform_quirks_init(void)
 {
-    ioh_id = pci_conf_read32(0, 0, IOH_DEV, 0, 0);
-    igd_id = pci_conf_read32(0, 0, IGD_DEV, 0, 0);
+    ioh_id = pci_conf_read32(PCI_SBDF(0, 0, IOH_DEV, 0), 0);
+    igd_id = pci_conf_read32(PCI_SBDF(0, 0, IGD_DEV, 0), 0);
 
     /* Mobile 4 Series Chipset neglects to set RWBF capability. */
     if ( ioh_id == 0x2a408086 )
@@ -356,15 +358,15 @@  int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
     u32 id;
     int rc = 0;
 
-    id = pci_conf_read32(0, 0, 0, 0, 0);
+    id = pci_conf_read32(PCI_SBDF(0, 0, 0, 0), 0);
     if ( IS_CTG(id) )
     {
         /* quit if ME does not exist */
-        if ( pci_conf_read32(0, 0, 3, 0, 0) == 0xffffffff )
+        if ( pci_conf_read32(PCI_SBDF(0, 0, 3, 0), 0) == 0xffffffff )
             return 0;
 
         /* if device is WLAN device, map ME phantom device 0:3.7 */
-        id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
+        id = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
         switch (id)
         {
             case 0x42328086:
@@ -384,11 +386,11 @@  int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
     else if ( IS_ILK(id) || IS_CPT(id) )
     {
         /* quit if ME does not exist */
-        if ( pci_conf_read32(0, 0, 22, 0, 0) == 0xffffffff )
+        if ( pci_conf_read32(PCI_SBDF(0, 0, 22, 0), 0) == 0xffffffff )
             return 0;
 
         /* if device is WLAN device, map ME phantom device 0:22.7 */
-        id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
+        id = pci_conf_read32(PCI_SBDF3(0, bus, devfn), 0);
         switch (id)
         {
             case 0x00878086:        /* Kilmer Peak */
@@ -438,7 +440,7 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
     case 0x342e: /* Tylersburg chipset (Nehalem / Westmere systems) */
     case 0x3728: /* Xeon C5500/C3500 (JasperForest) */
     case 0x3c28: /* Sandybridge */
-        val = pci_conf_read32(seg, bus, dev, func, 0x1AC);
+        val = pci_conf_read32(pdev->sbdf, 0x1AC);
         pci_conf_write32(seg, bus, dev, func, 0x1AC, val | (1 << 31));
         printk(XENLOG_INFO "Masked VT-d error signaling on %04x:%02x:%02x.%u\n",
                seg, bus, dev, func);
@@ -461,7 +463,7 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
                                           PCI_EXT_CAP_ID_VNDR);
             while ( pos )
             {
-                val = pci_conf_read32(seg, bus, dev, func, pos + PCI_VNDR_HEADER);
+                val = pci_conf_read32(pdev->sbdf, pos + PCI_VNDR_HEADER);
                 if ( PCI_VNDR_HEADER_ID(val) == 4 && PCI_VNDR_HEADER_REV(val) == 1 )
                 {
                     pos += PCI_VNDR_HEADER;
@@ -481,8 +483,8 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
             break;
         }
 
-        val = pci_conf_read32(seg, bus, dev, func, pos + PCI_ERR_UNCOR_MASK);
-        val2 = pci_conf_read32(seg, bus, dev, func, pos + PCI_ERR_COR_MASK);
+        val = pci_conf_read32(pdev->sbdf, pos + PCI_ERR_UNCOR_MASK);
+        val2 = pci_conf_read32(pdev->sbdf, pos + PCI_ERR_COR_MASK);
         if ( (val & PCI_ERR_UNC_UNSUP) && (val2 & PCI_ERR_COR_ADV_NFAT) )
             action = "Found masked";
         else if ( !ff )
@@ -497,7 +499,7 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
             action = "Must not mask";
 
         /* XPUNCERRMSK Send Completion with Unsupported Request */
-        val = pci_conf_read32(seg, bus, dev, func, 0x20c);
+        val = pci_conf_read32(pdev->sbdf, 0x20c);
         pci_conf_write32(seg, bus, dev, func, 0x20c, val | (1 << 4));
 
         printk(XENLOG_INFO "%s UR signaling on %04x:%02x:%02x.%u\n",
@@ -514,8 +516,8 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
     case 0x1610: case 0x1614: case 0x1618: /* Broadwell */
     case 0x1900: case 0x1904: case 0x1908: case 0x190c: case 0x190f: /* Skylake */
     case 0x1910: case 0x1918: case 0x191f: /* Skylake */
-        bar = pci_conf_read32(seg, bus, dev, func, 0x6c);
-        bar = (bar << 32) | pci_conf_read32(seg, bus, dev, func, 0x68);
+        bar = pci_conf_read32(pdev->sbdf, 0x6c);
+        bar = (bar << 32) | pci_conf_read32(pdev->sbdf, 0x68);
         pa = bar & 0x7ffffff000UL; /* bits 12...38 */
         if ( (bar & 1) && pa &&
              page_is_ram_type(paddr_to_pfn(pa), RAM_TYPE_RESERVED) )
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index 5e5e0f0538..b24702e0c3 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -93,7 +93,7 @@  int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap
     int ttl = 480; /* 3840 bytes, minimum 8 bytes per capability */
     int pos = max(start, 0x100);
 
-    header = pci_conf_read32(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+    header = pci_conf_read32(PCI_SBDF3(seg, bus, devfn), pos);
 
     /*
      * If we have no capabilities, this is indicated by cap ID,
@@ -109,7 +109,7 @@  int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap
         pos = PCI_EXT_CAP_NEXT(header);
         if ( pos < 0x100 )
             break;
-        header = pci_conf_read32(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+        header = pci_conf_read32(PCI_SBDF3(seg, bus, devfn), pos);
     }
     return 0;
 }
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 0b176b490a..7476634982 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -511,7 +511,7 @@  static int init_bars(struct pci_dev *pdev)
             continue;
         }
 
-        val = pci_conf_read32(pdev->seg, pdev->bus, slot, func, reg);
+        val = pci_conf_read32(pdev->sbdf, reg);
         if ( (val & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO )
         {
             bars[i].type = VPCI_BAR_IO;
@@ -561,8 +561,8 @@  static int init_bars(struct pci_dev *pdev)
         rom->type = VPCI_BAR_ROM;
         rom->size = size;
         rom->addr = addr;
-        header->rom_enabled = pci_conf_read32(pdev->seg, pdev->bus, slot, func,
-                                              rom_reg) & PCI_ROM_ADDRESS_ENABLE;
+        header->rom_enabled = pci_conf_read32(pdev->sbdf, rom_reg) &
+                              PCI_ROM_ADDRESS_ENABLE;
 
         rc = vpci_add_register(pdev->vpci, vpci_hw_read32, rom_write, rom_reg,
                                4, rom);
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 8e6cd070d0..c60cba0137 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -469,11 +469,9 @@  static int init_msix(struct pci_dev *pdev)
     pdev->vpci->msix->pdev = pdev;
 
     pdev->vpci->msix->tables[VPCI_MSIX_TABLE] =
-        pci_conf_read32(pdev->seg, pdev->bus, slot, func,
-                        msix_table_offset_reg(msix_offset));
+        pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
     pdev->vpci->msix->tables[VPCI_MSIX_PBA] =
-        pci_conf_read32(pdev->seg, pdev->bus, slot, func,
-                        msix_pba_offset_reg(msix_offset));
+        pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
 
     for ( i = 0; i < pdev->vpci->msix->max_entries; i++)
     {
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 1a4c2ee4f1..2106255863 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -120,8 +120,7 @@  uint32_t vpci_hw_read16(const struct pci_dev *pdev, unsigned int reg,
 uint32_t vpci_hw_read32(const struct pci_dev *pdev, unsigned int reg,
                         void *data)
 {
-    return pci_conf_read32(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                           PCI_FUNC(pdev->devfn), reg);
+    return pci_conf_read32(pdev->sbdf, reg);
 }
 
 int vpci_add_register(struct vpci *vpci, vpci_read_t *read_handler,
@@ -211,7 +210,7 @@  static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
     switch ( size )
     {
     case 4:
-        data = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg);
+        data = pci_conf_read32(sbdf, reg);
         break;
 
     case 3:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index cf4c223f7c..2a5705560e 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -174,9 +174,7 @@  void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg);
 uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg);
-uint32_t pci_conf_read32(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg);
+uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg);
 void pci_conf_write8(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint8_t data);