[v4,3/6] pci: switch pci_conf_read32 to use pci_sbdf_t
diff mbox series

Message ID 20190719140724.69596-4-roger.pau@citrix.com
State New
Headers show
Series
  • pci: expand usage of pci_sbdf_t
Related show

Commit Message

Roger Pau Monné July 19, 2019, 2:07 p.m. UTC
This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
a device number.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Brian Woods <brian.woods@amd.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.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>
---
Changes since v3:
 - Fix two usages of u32 and u64.
 - Switch {IGD/IOH}_DEV to pci_sbdf_t.
---
 xen/arch/x86/cpu/amd.c                     |  7 ++--
 xen/arch/x86/mm.c                          |  2 +-
 xen/arch/x86/msi.c                         | 30 ++++++--------
 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       | 46 +++++++++++-----------
 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, 106 insertions(+), 116 deletions(-)

Comments

Jan Beulich July 23, 2019, 10:15 a.m. UTC | #1
On 19.07.2019 16:07, Roger Pau Monne wrote:
> This reduces the number of parameters of the function to two, and
> simplifies some of the calling sites.
> 
> While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
> a device number.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Brian Woods <brian.woods@amd.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one further remark (easily addressed while committing):

> @@ -128,9 +128,9 @@ 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(IGD_DEV, 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(IGD_DEV,  PCI_BASE_ADDRESS_0);

There looks to be a stray blank in here.

Jan
Roger Pau Monné July 23, 2019, 11:04 a.m. UTC | #2
On Tue, Jul 23, 2019 at 10:15:35AM +0000, Jan Beulich wrote:
> On 19.07.2019 16:07, Roger Pau Monne wrote:
> > This reduces the number of parameters of the function to two, and
> > simplifies some of the calling sites.
> > 
> > While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
> > a device number.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Acked-by: Brian Woods <brian.woods@amd.com>
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one further remark (easily addressed while committing):
> 
> > @@ -128,9 +128,9 @@ 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(IGD_DEV, 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(IGD_DEV,  PCI_BASE_ADDRESS_0);
> 
> There looks to be a stray blank in here.

Good catch, please adjust on commit if you don't mind.

Thanks, Roger.

Patch
diff mbox series

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c6c74e75f5..8a40373ddf 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);
@@ -703,8 +704,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 334571d445..56b95e15d1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5950,7 +5950,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 7df9ddacff..52617cd843 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,8 +811,8 @@  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,
-                                     base + bir * 4) << 32);
+               ((uint64_t)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 28282e47ac..ec8811adac 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 4c7e065fe0..f60238a5b8 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -845,7 +845,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;
@@ -1228,7 +1228,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 6bc5b9682b..7f9573ceee 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);
+                uint32_t 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..40bc25d544 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -37,8 +37,8 @@ 
 #include "extern.h"
 #include "vtd.h"
 
-#define IOH_DEV      0
-#define IGD_DEV      2
+#define IOH_DEV      PCI_SBDF(0, 0, 0, 0)
+#define IGD_DEV      PCI_SBDF(0, 0, 2, 0)
 
 #define IGD_BAR_MASK 0xFFFFFFFFFFFF0000
 #define GGC 0x52
@@ -74,7 +74,7 @@  int is_igd_vt_enabled_quirk(void)
         return 1;
 
     /* integrated graphics on Intel platforms is located at 0:2.0 */
-    ggc = pci_conf_read16(PCI_SBDF(0, 0, IGD_DEV, 0), GGC);
+    ggc = pci_conf_read16(IGD_DEV, GGC);
     return ( ggc & GGC_MEMORY_VT_ENABLED ? 1 : 0 );
 }
 
@@ -88,12 +88,12 @@  static void __init cantiga_b3_errata_init(void)
     u16 vid;
     u8 did_hi, rid;
 
-    vid = pci_conf_read16(PCI_SBDF(0, 0, IGD_DEV, 0), 0);
+    vid = pci_conf_read16(IGD_DEV, 0);
     if ( vid != 0x8086 )
         return;
 
-    did_hi = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 3);
-    rid = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 8);
+    did_hi = pci_conf_read8(IGD_DEV, 3);
+    rid = pci_conf_read8(IGD_DEV, 8);
 
     if ( (did_hi == 0x2A) && (rid == 0x7) )
         is_cantiga_b3 = 1;
@@ -128,9 +128,9 @@  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(IGD_DEV, 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(IGD_DEV,  PCI_BASE_ADDRESS_0);
     igd_reg_va = ioremap(igd_mmio & IGD_BAR_MASK, 0x3000);
 }
 
@@ -280,7 +280,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 +296,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(IOH_DEV, 0);
+    igd_id = pci_conf_read32(IGD_DEV, 0);
 
     /* Mobile 4 Series Chipset neglects to set RWBF capability. */
     if ( ioh_id == 0x2a408086 )
@@ -356,15 +356,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 +384,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 +438,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 +461,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 +481,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 +497,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 +514,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 2847a8e6af..6885408687 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);