xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t
@ 2019-07-19 14:07 Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t Roger Pau Monne
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Roger Pau Monne

Hello,

The following are the remaining patches from my 'expand usage of
pci_sbdf_t' previous series except for the one that introduces a custom
printf formatter for pci_sbdf_t.

I've also pushed them to my git tree at:

git://xenbits.xen.org/people/royger/xen.git pci_sbdf_t

Thanks, Roger.

Roger Pau Monne (6):
  pci: switch pci_conf_read8 to use pci_sbdf_t
  pci: switch pci_conf_read16 to use pci_sbdf_t
  pci: switch pci_conf_read32 to use pci_sbdf_t
  pci: switch pci_conf_write8 to use pci_sbdf_t
  pci: switch pci_conf_write16 to use pci_sbdf_t
  pci: switch pci_conf_write32 to use pci_sbdf_t

 xen/arch/x86/cpu/amd.c                     |  15 +-
 xen/arch/x86/dmi_scan.c                    |   6 +-
 xen/arch/x86/mm.c                          |   2 +-
 xen/arch/x86/msi.c                         | 170 +++++++++------------
 xen/arch/x86/oprofile/op_model_athlon.c    |  10 +-
 xen/arch/x86/x86_64/mmconf-fam10h.c        |   8 +-
 xen/arch/x86/x86_64/mmconfig-shared.c      |  14 +-
 xen/arch/x86/x86_64/pci.c                  |  98 +++++-------
 xen/drivers/acpi/reboot.c                  |   5 +-
 xen/drivers/char/ehci-dbgp.c               |  49 +++---
 xen/drivers/char/ns16550.c                 |  71 +++++----
 xen/drivers/passthrough/amd/iommu_detect.c |   2 +-
 xen/drivers/passthrough/amd/iommu_init.c   |  17 +--
 xen/drivers/passthrough/ats.h              |   4 +-
 xen/drivers/passthrough/pci.c              | 102 +++++--------
 xen/drivers/passthrough/vtd/dmar.c         |   6 +-
 xen/drivers/passthrough/vtd/quirks.c       |  61 ++++----
 xen/drivers/passthrough/x86/ats.c          |  15 +-
 xen/drivers/pci/pci.c                      |  23 ++-
 xen/drivers/video/vga.c                    |  11 +-
 xen/drivers/vpci/header.c                  |  47 +++---
 xen/drivers/vpci/msi.c                     |   7 +-
 xen/drivers/vpci/msix.c                    |  11 +-
 xen/drivers/vpci/vpci.c                    |  42 ++---
 xen/include/xen/pci.h                      |  24 +--
 25 files changed, 353 insertions(+), 467 deletions(-)

-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 2/6] pci: switch pci_conf_read16 " Roger Pau Monne
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Brian Woods,
	Roger Pau Monne

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: Jan Beulich <jbeulich@suse.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:
 - Drop stray change to one pci_conf_write8 instance.
---
 xen/arch/x86/cpu/amd.c                   |  2 +-
 xen/arch/x86/msi.c                       |  2 +-
 xen/arch/x86/x86_64/pci.c                | 25 ++++++++++++------------
 xen/drivers/char/ehci-dbgp.c             |  5 +++--
 xen/drivers/char/ns16550.c               |  6 ++++--
 xen/drivers/passthrough/amd/iommu_init.c |  2 +-
 xen/drivers/passthrough/pci.c            | 21 ++++++++------------
 xen/drivers/passthrough/vtd/dmar.c       |  6 +++---
 xen/drivers/passthrough/vtd/quirks.c     |  6 +++---
 xen/drivers/pci/pci.c                    |  9 ++++-----
 xen/drivers/video/vga.c                  |  3 +--
 xen/drivers/vpci/header.c                |  3 +--
 xen/drivers/vpci/vpci.c                  |  8 +++-----
 xen/include/xen/pci.h                    |  4 +---
 14 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 839f19292d..c6c74e75f5 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -420,7 +420,7 @@ static void disable_c1_ramping(void)
 	nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 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(0, 0, 0x18+node, 0x3, 0x87);
+		pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87);
 		/* Invalid read means we've updated every Northbridge. */
 		if (pmm7 == 0xFF)
 			break;
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 89e61160e9..ed8b89cc0f 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -800,7 +800,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
         disp = vf * pdev->vf_rlen[bir];
         limit = PCI_SRIOV_NUM_BARS;
     }
-    else switch ( pci_conf_read8(seg, bus, slot, func,
+    else switch ( pci_conf_read8(PCI_SBDF(seg, bus, slot, func),
                                  PCI_HEADER_TYPE) & 0x7f )
     {
     case PCI_HEADER_TYPE_NORMAL:
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index 6e3f5cf203..b70383fb03 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -8,27 +8,26 @@
 #include <xen/pci.h>
 #include <asm/io.h>
 
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+#define PCI_CONF_ADDRESS(sbdf, reg) \
+    (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3))
 
-uint8_t pci_conf_read8(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg)
+uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {
-    u32 value;
+    uint32_t value;
 
-    if ( seg || reg > 255 )
+    if ( sbdf.seg || reg > 255 )
     {
-        pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 1, &value);
+        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);
         return value;
     }
-    else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1);
-    }
+
+    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 uint16_t pci_conf_read16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 475dc41767..71f0aaa6ac 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -713,7 +713,7 @@ static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp,
                 cap = __find_dbgp(bus, slot, func);
                 if ( !cap || ehci_num-- )
                 {
-                    if ( !func && !(pci_conf_read8(0, bus, slot, func,
+                    if ( !func && !(pci_conf_read8(PCI_SBDF(0, bus, slot, func),
                                                    PCI_HEADER_TYPE) & 0x80) )
                         break;
                     continue;
@@ -1312,7 +1312,8 @@ static void __init ehci_dbgp_init_preirq(struct serial_port *port)
     offset = (debug_port >> 16) & 0xfff;
 
     /* double check if the mem space is enabled */
-    dbgp->pci_cr = pci_conf_read8(0, dbgp->bus, dbgp->slot, dbgp->func,
+    dbgp->pci_cr = pci_conf_read8(PCI_SBDF(0, dbgp->bus, dbgp->slot,
+                                           dbgp->func),
                                   PCI_COMMAND);
     if ( !(dbgp->pci_cr & PCI_COMMAND_MEMORY) )
     {
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 189e121b7e..547270d0e1 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1188,8 +1188,10 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 uart->bar64 = bar_64;
                 uart->io_size = max(8U << param->reg_shift,
                                     param->uart_offset);
-                uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
-                    pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
+                uart->irq = pci_conf_read8(PCI_SBDF(0, b, d, f),
+                                           PCI_INTERRUPT_PIN) ?
+                            pci_conf_read8(PCI_SBDF(0, b, d, f),
+                                           PCI_INTERRUPT_LINE) : 0;
 
                 return 0;
             }
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 4e76b2620c..943e1c393e 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1238,7 +1238,7 @@ static bool_t __init amd_sp5100_erratum28(void)
         if (vendor_id != 0x1002 || dev_id != 0x4385)
             continue;
 
-        byte = pci_conf_read8(0, bus, 0x14, 0, 0xad);
+        byte = pci_conf_read8(PCI_SBDF(0, bus, 0x14, 0), 0xad);
         if ( (byte >> 3) & 1 )
         {
             printk(XENLOG_WARNING "AMD-Vi: SP5100 erratum 28 detected, disabling IOMMU.\n"
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e88689425d..340e957954 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -260,7 +260,7 @@ static void check_pdev(const struct pci_dev *pdev)
         }
     }
 
-    switch ( pci_conf_read8(seg, bus, dev, func, PCI_HEADER_TYPE) & 0x7f )
+    switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f )
     {
     case PCI_HEADER_TYPE_BRIDGE:
         if ( !bridge_ctl_mask )
@@ -370,10 +370,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
 
         case DEV_TYPE_PCIe2PCI_BRIDGE:
         case DEV_TYPE_LEGACY_PCI_BRIDGE:
-            sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
-                                     PCI_FUNC(devfn), PCI_SECONDARY_BUS);
-            sub_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
-                                     PCI_FUNC(devfn), PCI_SUBORDINATE_BUS);
+            sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS);
+            sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS);
 
             spin_lock(&pseg->bus2bridge_lock);
             for ( ; sec_bus <= sub_bus; sec_bus++ )
@@ -436,16 +434,12 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
     /* update bus2bridge */
     switch ( pdev->type )
     {
-        u8 dev, func, sec_bus, sub_bus;
+        uint8_t sec_bus, sub_bus;
 
         case DEV_TYPE_PCIe2PCI_BRIDGE:
         case DEV_TYPE_LEGACY_PCI_BRIDGE:
-            dev = PCI_SLOT(pdev->devfn);
-            func = PCI_FUNC(pdev->devfn);
-            sec_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
-                                     PCI_SECONDARY_BUS);
-            sub_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
-                                     PCI_SUBORDINATE_BUS);
+            sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS);
+            sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS);
 
             spin_lock(&pseg->bus2bridge_lock);
             for ( ; sec_bus <= sub_bus; sec_bus++ )
@@ -1082,7 +1076,8 @@ static int __init _scan_pci_devices(struct pci_seg *pseg, void *arg)
                     return -ENOMEM;
                 }
 
-                if ( !func && !(pci_conf_read8(pseg->nr, bus, dev, func,
+                if ( !func && !(pci_conf_read8(PCI_SBDF(pseg->nr, bus, dev,
+                                                        func),
                                                PCI_HEADER_TYPE) & 0x80) )
                     break;
             }
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index b858fe7c80..9c94deac0b 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -348,7 +348,7 @@ static int __init acpi_parse_dev_scope(
 
         while ( --depth > 0 )
         {
-            bus = pci_conf_read8(seg, bus, path->dev, path->fn,
+            bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
                                  PCI_SECONDARY_BUS);
             path++;
         }
@@ -356,9 +356,9 @@ static int __init acpi_parse_dev_scope(
         switch ( acpi_scope->entry_type )
         {
         case ACPI_DMAR_SCOPE_TYPE_BRIDGE:
-            sec_bus = pci_conf_read8(seg, bus, path->dev, path->fn,
+            sec_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
                                      PCI_SECONDARY_BUS);
-            sub_bus = pci_conf_read8(seg, bus, path->dev, path->fn,
+            sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
                                      PCI_SUBORDINATE_BUS);
             if ( iommu_verbose )
                 printk(VTDPREFIX
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index d6db862678..ff73b0e7f4 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -92,8 +92,8 @@ static void __init cantiga_b3_errata_init(void)
     if ( vid != 0x8086 )
         return;
 
-    did_hi = pci_conf_read8(0, 0, IGD_DEV, 0, 3);
-    rid = pci_conf_read8(0, 0, IGD_DEV, 0, 8);
+    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);
 
     if ( (did_hi == 0x2A) && (rid == 0x7) )
         is_cantiga_b3 = 1;
@@ -281,7 +281,7 @@ static void __init tylersburg_intremap_quirk(void)
     {
         /* Match on System Management Registers on Device 20 Function 0 */
         device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
-        rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID);
+        rev = pci_conf_read8(PCI_SBDF(0, bus, 20, 0), PCI_REVISION_ID);
 
         if ( rev == 0x13 && device == 0x342e8086 )
         {
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index 1c808d6632..e3f883fc5c 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -21,12 +21,12 @@ int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
 
     while ( max_cap-- )
     {
-        pos = pci_conf_read8(seg, bus, dev, func, pos);
+        pos = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos);
         if ( pos < 0x40 )
             break;
 
         pos &= ~3;
-        id = pci_conf_read8(seg, bus, dev, func, pos + PCI_CAP_LIST_ID);
+        id = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos + PCI_CAP_LIST_ID);
 
         if ( id == 0xff )
             break;
@@ -46,13 +46,12 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap)
 
     while ( ttl-- )
     {
-        pos = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos);
+        pos = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos);
         if ( pos < 0x40 )
             break;
 
         pos &= ~3;
-        id = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                            pos + PCI_CAP_LIST_ID);
+        id = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos + PCI_CAP_LIST_ID);
 
         if ( id == 0xff )
             break;
diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c
index 6a64fd9013..78533ad0b1 100644
--- a/xen/drivers/video/vga.c
+++ b/xen/drivers/video/vga.c
@@ -136,8 +136,7 @@ void __init video_endboot(void)
                         b = 0;
                         break;
                     case 1:
-                        switch ( pci_conf_read8(0, b, PCI_SLOT(df),
-                                                PCI_FUNC(df),
+                        switch ( pci_conf_read8(PCI_SBDF3(0, b, df),
                                                 PCI_HEADER_TYPE) )
                         {
                         case PCI_HEADER_TYPE_BRIDGE:
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 258b91deed..564c7b6a7d 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -463,8 +463,7 @@ static int init_bars(struct pci_dev *pdev)
     struct vpci_bar *bars = header->bars;
     int rc;
 
-    switch ( pci_conf_read8(pdev->seg, pdev->bus, slot, func, PCI_HEADER_TYPE)
-             & 0x7f )
+    switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f )
     {
     case PCI_HEADER_TYPE_NORMAL:
         num_bars = PCI_HEADER_NORMAL_NR_BARS;
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index ca598675ea..c4030333a5 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -222,8 +222,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
          */
         if ( reg & 1 )
         {
-            data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
-                                  reg);
+            data = pci_conf_read8(sbdf, reg);
             data |= pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
                                     reg + 1) << 8;
         }
@@ -231,8 +230,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
         {
             data = pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
                                    reg);
-            data |= pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
-                                   reg + 2) << 16;
+            data |= pci_conf_read8(sbdf, reg + 2) << 16;
         }
         break;
 
@@ -241,7 +239,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
         break;
 
     case 1:
-        data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg);
+        data = pci_conf_read8(sbdf, reg);
         break;
 
     default:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 04a9f46cc3..408cd1cb67 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -172,9 +172,7 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg,
                                        int bus, int devfn);
 void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 
-uint8_t pci_conf_read8(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg);
+uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg);
 uint16_t pci_conf_read16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg);
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 2/6] pci: switch pci_conf_read16 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 " Roger Pau Monne
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Brian Woods,
	Roger Pau Monne

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: Jan Beulich <jbeulich@suse.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:
 - Convert one missing u16 to uint16_t.
---
 xen/arch/x86/dmi_scan.c                  |  6 +-
 xen/arch/x86/msi.c                       | 73 ++++++++++--------------
 xen/arch/x86/x86_64/mmconfig-shared.c    |  2 +-
 xen/arch/x86/x86_64/pci.c                | 27 ++++-----
 xen/drivers/char/ehci-dbgp.c             |  5 +-
 xen/drivers/char/ns16550.c               | 16 ++++--
 xen/drivers/passthrough/amd/iommu_init.c |  3 +-
 xen/drivers/passthrough/ats.h            |  4 +-
 xen/drivers/passthrough/pci.c            | 40 +++++--------
 xen/drivers/passthrough/vtd/quirks.c     |  9 ++-
 xen/drivers/passthrough/x86/ats.c        |  9 +--
 xen/drivers/pci/pci.c                    |  4 +-
 xen/drivers/video/vga.c                  |  8 +--
 xen/drivers/vpci/header.c                | 11 ++--
 xen/drivers/vpci/msi.c                   |  3 +-
 xen/drivers/vpci/msix.c                  |  3 +-
 xen/drivers/vpci/vpci.c                  | 11 ++--
 xen/include/xen/pci.h                    |  4 +-
 18 files changed, 99 insertions(+), 139 deletions(-)

diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
index fcdf2d3952..31caad133e 100644
--- a/xen/arch/x86/dmi_scan.c
+++ b/xen/arch/x86/dmi_scan.c
@@ -469,15 +469,15 @@ static int __init ich10_bios_quirk(struct dmi_system_id *d)
 {
     u32 port, smictl;
 
-    if ( pci_conf_read16(0, 0, 0x1f, 0, PCI_VENDOR_ID) != 0x8086 )
+    if ( pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), PCI_VENDOR_ID) != 0x8086 )
         return 0;
 
-    switch ( pci_conf_read16(0, 0, 0x1f, 0, PCI_DEVICE_ID) ) {
+    switch ( pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), PCI_DEVICE_ID) ) {
     case 0x3a14:
     case 0x3a16:
     case 0x3a18:
     case 0x3a1a:
-        port = (pci_conf_read16(0, 0, 0x1f, 0, 0x40) & 0xff80) + 0x30;
+        port = (pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), 0x40) & 0xff80) + 0x30;
         smictl = inl(port);
         /* turn off LEGACY_USB{,2}_EN if enabled */
         if ( smictl & 0x20008 )
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index ed8b89cc0f..7df9ddacff 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -124,29 +124,20 @@ static void msix_put_fixmap(struct arch_msix *msix, int idx)
 
 static bool memory_decoded(const struct pci_dev *dev)
 {
-    u8 bus, slot, func;
+    pci_sbdf_t sbdf = dev->sbdf;
 
-    if ( !dev->info.is_virtfn )
-    {
-        bus = dev->bus;
-        slot = PCI_SLOT(dev->devfn);
-        func = PCI_FUNC(dev->devfn);
-    }
-    else
+    if ( dev->info.is_virtfn )
     {
-        bus = dev->info.physfn.bus;
-        slot = PCI_SLOT(dev->info.physfn.devfn);
-        func = PCI_FUNC(dev->info.physfn.devfn);
+        sbdf.bus = dev->info.physfn.bus;
+        sbdf.devfn = dev->info.physfn.devfn;
     }
 
-    return !!(pci_conf_read16(dev->seg, bus, slot, func, PCI_COMMAND) &
-              PCI_COMMAND_MEMORY);
+    return pci_conf_read16(sbdf, PCI_COMMAND) & PCI_COMMAND_MEMORY;
 }
 
 static bool msix_memory_decoded(const struct pci_dev *dev, unsigned int pos)
 {
-    u16 control = pci_conf_read16(dev->seg, dev->bus, PCI_SLOT(dev->devfn),
-                                  PCI_FUNC(dev->devfn), msix_control_reg(pos));
+    uint16_t control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
 
     if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
         return false;
@@ -211,14 +202,12 @@ static bool read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
         {
             msg->address_hi = pci_conf_read32(seg, bus, slot, func,
                                               msi_upper_address_reg(pos));
-            data = pci_conf_read16(seg, bus, slot, func,
-                                   msi_data_reg(pos, 1));
+            data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 1));
         }
         else
         {
             msg->address_hi = 0;
-            data = pci_conf_read16(seg, bus, slot, func,
-                                   msi_data_reg(pos, 0));
+            data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 0));
         }
         msg->data = data;
         break;
@@ -337,7 +326,8 @@ void set_msi_affinity(struct irq_desc *desc, const cpumask_t *mask)
 
 void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable)
 {
-    u16 control = pci_conf_read16(seg, bus, slot, func, pos + PCI_MSI_FLAGS);
+    uint16_t control = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+                                       pos + PCI_MSI_FLAGS);
 
     control &= ~PCI_MSI_FLAGS_ENABLE;
     if ( enable )
@@ -369,7 +359,7 @@ static void msix_set_enable(struct pci_dev *dev, int enable)
     pos = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSIX);
     if ( pos )
     {
-        control = pci_conf_read16(seg, bus, slot, func, msix_control_reg(pos));
+        control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
         control &= ~PCI_MSIX_FLAGS_ENABLE;
         if ( enable )
             control |= PCI_MSIX_FLAGS_ENABLE;
@@ -414,7 +404,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
         break;
     case PCI_CAP_ID_MSIX:
         maskall = pdev->msix->host_maskall;
-        control = pci_conf_read16(seg, bus, slot, func,
+        control = pci_conf_read16(pdev->sbdf,
                                   msix_control_reg(entry->msi_attrib.pos));
         if ( unlikely(!(control & PCI_MSIX_FLAGS_ENABLE)) )
         {
@@ -594,8 +584,7 @@ int setup_msi_irq(struct irq_desc *desc, struct msi_desc *msidesc)
 
     if ( msidesc->msi_attrib.type == PCI_CAP_ID_MSIX )
     {
-        control = pci_conf_read16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                                  PCI_FUNC(pdev->devfn), cpos);
+        control = pci_conf_read16(pdev->sbdf, cpos);
         if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
             pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
                              PCI_FUNC(pdev->devfn), cpos,
@@ -698,7 +687,7 @@ static int msi_capability_init(struct pci_dev *dev,
     pos = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSI);
     if ( !pos )
         return -ENODEV;
-    control = pci_conf_read16(seg, bus, slot, func, msi_control_reg(pos));
+    control = pci_conf_read16(dev->sbdf, msi_control_reg(pos));
     maxvec = multi_msi_capable(control);
     if ( nvec > maxvec )
         return maxvec;
@@ -769,13 +758,14 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
         unsigned int pos = pci_find_ext_capability(seg, bus,
                                                    PCI_DEVFN(slot, func),
                                                    PCI_EXT_CAP_ID_SRIOV);
-        u16 ctrl = pci_conf_read16(seg, bus, slot, func, pos + PCI_SRIOV_CTRL);
-        u16 num_vf = pci_conf_read16(seg, bus, slot, func,
-                                     pos + PCI_SRIOV_NUM_VF);
-        u16 offset = pci_conf_read16(seg, bus, slot, func,
-                                     pos + PCI_SRIOV_VF_OFFSET);
-        u16 stride = pci_conf_read16(seg, bus, slot, func,
-                                     pos + PCI_SRIOV_VF_STRIDE);
+        uint16_t ctrl = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+                                        pos + PCI_SRIOV_CTRL);
+        uint16_t num_vf = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+                                          pos + PCI_SRIOV_NUM_VF);
+        uint16_t offset = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+                                          pos + PCI_SRIOV_VF_OFFSET);
+        uint16_t stride = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+                                          pos + PCI_SRIOV_VF_STRIDE);
 
         if ( !pdev || !pos ||
              !(ctrl & PCI_SRIOV_CTRL_VFE) ||
@@ -864,7 +854,7 @@ static int msix_capability_init(struct pci_dev *dev,
 
     ASSERT(pcidevs_locked());
 
-    control = pci_conf_read16(seg, bus, slot, func, msix_control_reg(pos));
+    control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
     /*
      * Ensure MSI-X interrupts are masked during setup. Some devices require
      * MSI-X to be enabled before we can touch the MSI-X registers. We need
@@ -1131,8 +1121,7 @@ static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc)
     if ( !pdev || !pos )
         return -ENODEV;
 
-    control = pci_conf_read16(msi->seg, msi->bus, slot, func,
-                              msix_control_reg(pos));
+    control = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
     nr_entries = multi_msix_capable(control);
     if ( msi->entry_nr >= nr_entries )
         return -EINVAL;
@@ -1178,7 +1167,7 @@ static void __pci_disable_msix(struct msi_desc *entry)
     u8 func = PCI_FUNC(dev->devfn);
     unsigned int pos = pci_find_cap_offset(seg, bus, slot, func,
                                            PCI_CAP_ID_MSIX);
-    u16 control = pci_conf_read16(seg, bus, slot, func,
+    u16 control = pci_conf_read16(dev->sbdf,
                                   msix_control_reg(entry->msi_attrib.pos));
     bool maskall = dev->msix->host_maskall;
 
@@ -1236,8 +1225,8 @@ int pci_prepare_msix(u16 seg, u8 bus, u8 devfn, bool off)
     }
     else
     {
-        u16 control = pci_conf_read16(seg, bus, slot, func,
-                                      msix_control_reg(pos));
+        uint16_t control = pci_conf_read16(PCI_SBDF3(seg, bus, devfn),
+                                           msix_control_reg(pos));
 
         rc = msix_capability_init(pdev, pos, NULL, NULL,
                                   multi_msix_capable(control));
@@ -1339,7 +1328,7 @@ int pci_msi_conf_write_intercept(struct pci_dev *pdev, unsigned int reg,
         if ( reg < entry->msi.mpos || reg >= entry->msi.mpos + 4 || size != 4 )
             return -EACCES;
 
-        cntl = pci_conf_read16(seg, bus, slot, func, msi_control_reg(pos));
+        cntl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
         unused = ~(uint32_t)0 >> (32 - multi_msi_capable(cntl));
         for ( pos = 0; pos < nvec; ++pos, ++entry )
         {
@@ -1415,8 +1404,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
         }
         else if ( !type && entry->msi_attrib.type == PCI_CAP_ID_MSIX )
         {
-            control = pci_conf_read16(pdev->seg, pdev->bus, slot, func,
-                                      msix_control_reg(pos));
+            control = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
             pci_conf_write16(pdev->seg, pdev->bus, slot, func,
                              msix_control_reg(pos),
                              control | (PCI_MSIX_FLAGS_ENABLE |
@@ -1458,8 +1446,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
         {
             unsigned int cpos = msi_control_reg(pos);
 
-            control = pci_conf_read16(pdev->seg, pdev->bus, slot, func, cpos) &
-                      ~PCI_MSI_FLAGS_QSIZE;
+            control = pci_conf_read16(pdev->sbdf, cpos) & ~PCI_MSI_FLAGS_QSIZE;
             multi_msi_enable(control, entry->msi.nvec);
             pci_conf_write16(pdev->seg, pdev->bus, slot, func, cpos, control);
 
diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
index 9e1c81dcd2..9d1db590d9 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -64,7 +64,7 @@ custom_param("mmcfg", parse_mmcfg);
 static const char __init *pci_mmcfg_e7520(void)
 {
     u32 win;
-    win = pci_conf_read16(0, 0, 0, 0, 0xce);
+    win = pci_conf_read16(PCI_SBDF(0, 0, 0, 0), 0xce);
 
     win = win & 0xf000;
     if(win == 0x0000 || win == 0xf000)
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index b70383fb03..fe36b60c50 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -24,28 +24,23 @@ uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
     return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-uint16_t pci_conf_read16(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg)
+uint16_t pci_conf_read16(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, 2, &value);
+        uint32_t value;
+
+        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, &value);
         return value;
     }
-    else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2);
-    }
+
+    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)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 71f0aaa6ac..64258da2dc 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1016,7 +1016,7 @@ static void nvidia_set_debug_port(struct ehci_dbgp *dbgp, unsigned int port)
 
 static void __init detect_set_debug_port(struct ehci_dbgp *dbgp)
 {
-    if ( pci_conf_read16(0, dbgp->bus, dbgp->slot, dbgp->func,
+    if ( pci_conf_read16(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                          PCI_VENDOR_ID) == 0x10de )
     {
         dbgp_printk("using nvidia set_debug_port\n");
@@ -1416,7 +1416,8 @@ static void ehci_dbgp_suspend(struct serial_port *port)
     stop_timer(&dbgp->timer);
     dbgp->timer.expires = 0;
 
-    dbgp->pci_cr = pci_conf_read16(0, dbgp->bus, dbgp->slot, dbgp->func,
+    dbgp->pci_cr = pci_conf_read16(PCI_SBDF(0, dbgp->bus, dbgp->slot,
+                                            dbgp->func),
                                    PCI_COMMAND);
 
     dbgp->state = dbgp_unsafe;
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 547270d0e1..99c1254cac 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -846,8 +846,8 @@ static void ns16550_suspend(struct serial_port *port)
 
 #ifdef CONFIG_HAS_PCI
     if ( uart->bar )
-       uart->cr = pci_conf_read16(0, uart->ps_bdf[0], uart->ps_bdf[1],
-                                  uart->ps_bdf[2], PCI_COMMAND);
+       uart->cr = pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
+                                  uart->ps_bdf[2]), PCI_COMMAND);
 #endif
 }
 
@@ -1064,10 +1064,12 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 u64 size = 0;
                 const struct ns16550_config_param *param = uart_param;
 
-                nextf = (f || (pci_conf_read16(0, b, d, f, PCI_HEADER_TYPE) &
+                nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
+                                               PCI_HEADER_TYPE) &
                                0x80)) ? f + 1 : 8;
 
-                switch ( pci_conf_read16(0, b, d, f, PCI_CLASS_DEVICE) )
+                switch ( pci_conf_read16(PCI_SBDF(0, b, d, f),
+                                         PCI_CLASS_DEVICE) )
                 {
                 case 0x0700: /* single port serial */
                 case 0x0702: /* multi port serial */
@@ -1084,8 +1086,10 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 /* Check for params in uart_config lookup table */
                 for ( i = 0; i < ARRAY_SIZE(uart_config); i++ )
                 {
-                    u16 vendor = pci_conf_read16(0, b, d, f, PCI_VENDOR_ID);
-                    u16 device = pci_conf_read16(0, b, d, f, PCI_DEVICE_ID);
+                    u16 vendor = pci_conf_read16(PCI_SBDF(0, b, d, f),
+                                                 PCI_VENDOR_ID);
+                    u16 device = pci_conf_read16(PCI_SBDF(0, b, d, f),
+                                                 PCI_DEVICE_ID);
 
                     if ( uart_config[i].vendor_id == vendor &&
                          uart_config[i].dev_id == device )
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 943e1c393e..4c7e065fe0 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -799,8 +799,7 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
                         PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf));
         return 0;
     }
-    control = pci_conf_read16(iommu->seg, PCI_BUS(iommu->bdf),
-                              PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf),
+    control = pci_conf_read16(PCI_SBDF2(iommu->seg, iommu->bdf),
                               iommu->msi.msi_attrib.pos + PCI_MSI_FLAGS);
     iommu->msi.msi.nvec = 1;
     if ( is_mask_bit_support(control) )
diff --git a/xen/drivers/passthrough/ats.h b/xen/drivers/passthrough/ats.h
index bee13911c0..22ae209b37 100644
--- a/xen/drivers/passthrough/ats.h
+++ b/xen/drivers/passthrough/ats.h
@@ -35,8 +35,8 @@ static inline int pci_ats_enabled(int seg, int bus, int devfn)
     pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
     BUG_ON(!pos);
 
-    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                            pos + ATS_REG_CTL);
+    value = pci_conf_read16(PCI_SBDF3(seg, bus, devfn), pos + ATS_REG_CTL);
+
     return value & ATS_ENABLE;
 }
 
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 340e957954..6bc5b9682b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -246,11 +246,11 @@ static void check_pdev(const struct pci_dev *pdev)
 
     if ( command_mask )
     {
-        val = pci_conf_read16(seg, bus, dev, func, PCI_COMMAND);
+        val = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
         if ( val & command_mask )
             pci_conf_write16(seg, bus, dev, func, PCI_COMMAND,
                              val & ~command_mask);
-        val = pci_conf_read16(seg, bus, dev, func, PCI_STATUS);
+        val = pci_conf_read16(pdev->sbdf, PCI_STATUS);
         if ( val & PCI_STATUS_CHECK )
         {
             printk(XENLOG_INFO "%04x:%02x:%02x.%u status %04x -> %04x\n",
@@ -265,11 +265,11 @@ static void check_pdev(const struct pci_dev *pdev)
     case PCI_HEADER_TYPE_BRIDGE:
         if ( !bridge_ctl_mask )
             break;
-        val = pci_conf_read16(seg, bus, dev, func, PCI_BRIDGE_CONTROL);
+        val = pci_conf_read16(pdev->sbdf, PCI_BRIDGE_CONTROL);
         if ( val & bridge_ctl_mask )
             pci_conf_write16(seg, bus, dev, func, PCI_BRIDGE_CONTROL,
                              val & ~bridge_ctl_mask);
-        val = pci_conf_read16(seg, bus, dev, func, PCI_SEC_STATUS);
+        val = pci_conf_read16(pdev->sbdf, PCI_SEC_STATUS);
         if ( val & PCI_STATUS_CHECK )
         {
             printk(XENLOG_INFO
@@ -289,12 +289,8 @@ static void check_pdev(const struct pci_dev *pdev)
 
 static void apply_quirks(struct pci_dev *pdev)
 {
-    uint16_t vendor = pci_conf_read16(pdev->seg, pdev->bus,
-                                      PCI_SLOT(pdev->devfn),
-                                      PCI_FUNC(pdev->devfn), PCI_VENDOR_ID);
-    uint16_t device = pci_conf_read16(pdev->seg, pdev->bus,
-                                      PCI_SLOT(pdev->devfn),
-                                      PCI_FUNC(pdev->devfn), PCI_DEVICE_ID);
+    uint16_t vendor = pci_conf_read16(pdev->sbdf, PCI_VENDOR_ID);
+    uint16_t device = pci_conf_read16(pdev->sbdf, PCI_DEVICE_ID);
     static const struct {
         uint16_t vendor, device;
     } ignore_bars[] = {
@@ -387,8 +383,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
             pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn),
                                       PCI_FUNC(devfn), PCI_CAP_ID_EXP);
             BUG_ON(!pos);
-            cap = pci_conf_read16(pseg->nr, bus, PCI_SLOT(devfn),
-                                  PCI_FUNC(devfn), pos + PCI_EXP_DEVCAP);
+            cap = pci_conf_read16(pdev->sbdf, pos + PCI_EXP_DEVCAP);
             if ( cap & PCI_EXP_DEVCAP_PHANTOM )
             {
                 pdev->phantom_stride = 8 >> MASK_EXTR(cap,
@@ -611,8 +606,8 @@ static void pci_enable_acs(struct pci_dev *pdev)
     if (!pos)
         return;
 
-    cap = pci_conf_read16(seg, bus, dev, func, pos + PCI_ACS_CAP);
-    ctrl = pci_conf_read16(seg, bus, dev, func, pos + PCI_ACS_CTRL);
+    cap = pci_conf_read16(pdev->sbdf, pos + PCI_ACS_CAP);
+    ctrl = pci_conf_read16(pdev->sbdf, pos + PCI_ACS_CTRL);
 
     /* Source Validation */
     ctrl |= (cap & PCI_ACS_SV);
@@ -743,7 +738,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
     {
         unsigned int pos = pci_find_ext_capability(seg, bus, devfn,
                                                    PCI_EXT_CAP_ID_SRIOV);
-        u16 ctrl = pci_conf_read16(seg, bus, slot, func, pos + PCI_SRIOV_CTRL);
+        uint16_t ctrl = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_CTRL);
 
         if ( !pos )
             /* Nothing */;
@@ -937,13 +932,13 @@ enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
     u8 d = PCI_SLOT(devfn), f = PCI_FUNC(devfn);
     int pos = pci_find_cap_offset(seg, bus, d, f, PCI_CAP_ID_EXP);
 
-    class_device = pci_conf_read16(seg, bus, d, f, PCI_CLASS_DEVICE);
+    class_device = pci_conf_read16(PCI_SBDF(seg, bus, d, f), PCI_CLASS_DEVICE);
     switch ( class_device )
     {
     case PCI_CLASS_BRIDGE_PCI:
         if ( !pos )
             return DEV_TYPE_LEGACY_PCI_BRIDGE;
-        creg = pci_conf_read16(seg, bus, d, f, pos + PCI_EXP_FLAGS);
+        creg = pci_conf_read16(PCI_SBDF(seg, bus, d, f), pos + PCI_EXP_FLAGS);
         switch ( (creg & PCI_EXP_FLAGS_TYPE) >> 4 )
         {
         case PCI_EXP_TYPE_PCI_BRIDGE:
@@ -1040,8 +1035,7 @@ void pci_check_disable_device(u16 seg, u8 bus, u8 devfn)
     /* Tell the device to stop DMAing; we can't rely on the guest to
      * control it for us. */
     devfn = pdev->devfn;
-    cword = pci_conf_read16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                            PCI_COMMAND);
+    cword = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
     pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                      PCI_COMMAND, cword & ~PCI_COMMAND_MASTER);
 }
@@ -1209,10 +1203,7 @@ static bool_t hest_match_type(const struct acpi_hest_header *hest_hdr,
                                            PCI_SLOT(pdev->devfn),
                                            PCI_FUNC(pdev->devfn),
                                            PCI_CAP_ID_EXP);
-    u8 pcie = MASK_EXTR(pci_conf_read16(pdev->seg, pdev->bus,
-                                        PCI_SLOT(pdev->devfn),
-                                        PCI_FUNC(pdev->devfn),
-                                        pos + PCI_EXP_FLAGS),
+    u8 pcie = MASK_EXTR(pci_conf_read16(pdev->sbdf, pos + PCI_EXP_FLAGS),
                         PCI_EXP_FLAGS_TYPE);
 
     switch ( hest_hdr->type )
@@ -1222,8 +1213,7 @@ static bool_t hest_match_type(const struct acpi_hest_header *hest_hdr,
     case ACPI_HEST_TYPE_AER_ENDPOINT:
         return pcie == PCI_EXP_TYPE_ENDPOINT;
     case ACPI_HEST_TYPE_AER_BRIDGE:
-        return pci_conf_read16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                               PCI_FUNC(pdev->devfn), PCI_CLASS_DEVICE) ==
+        return pci_conf_read16(pdev->sbdf, PCI_CLASS_DEVICE) ==
                PCI_CLASS_BRIDGE_PCI;
     }
 
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index ff73b0e7f4..47597c9600 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -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(0, 0, IGD_DEV, 0, GGC);
+    ggc = pci_conf_read16(PCI_SBDF(0, 0, IGD_DEV, 0), GGC);
     return ( ggc & GGC_MEMORY_VT_ENABLED ? 1 : 0 );
 }
 
@@ -88,7 +88,7 @@ static void __init cantiga_b3_errata_init(void)
     u16 vid;
     u8 did_hi, rid;
 
-    vid = pci_conf_read16(0, 0, IGD_DEV, 0, 0);
+    vid = pci_conf_read16(PCI_SBDF(0, 0, IGD_DEV, 0), 0);
     if ( vid != 0x8086 )
         return;
 
@@ -424,11 +424,10 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
     paddr_t pa;
     const char *action;
 
-    if ( pci_conf_read16(seg, bus, dev, func, PCI_VENDOR_ID) !=
-         PCI_VENDOR_ID_INTEL )
+    if ( pci_conf_read16(pdev->sbdf, PCI_VENDOR_ID) != PCI_VENDOR_ID_INTEL )
         return;
 
-    switch ( pci_conf_read16(seg, bus, dev, func, PCI_DEVICE_ID) )
+    switch ( pci_conf_read16(pdev->sbdf, PCI_DEVICE_ID) )
     {
     /*
      * Mask reporting Intel VT-d faults to IOH core logic:
diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index 59c163459a..cb022c598a 100644
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -34,8 +34,7 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
         dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS capability found\n",
                 seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
 
-    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
-                            PCI_FUNC(devfn), pos + ATS_REG_CTL);
+    value = pci_conf_read16(pdev->sbdf, pos + ATS_REG_CTL);
     if ( value & ATS_ENABLE )
     {
         struct pci_dev *other;
@@ -58,8 +57,7 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
     if ( pos )
     {
         pdev->ats.cap_pos = pos;
-        value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
-                                PCI_FUNC(devfn), pos + ATS_REG_CAP);
+        value = pci_conf_read16(pdev->sbdf, pos + ATS_REG_CAP);
         pdev->ats.queue_depth = value & ATS_QUEUE_DEPTH_MASK ?:
                                 ATS_QUEUE_DEPTH_MASK + 1;
         list_add(&pdev->ats.list, ats_list);
@@ -81,8 +79,7 @@ void disable_ats_device(struct pci_dev *pdev)
 
     BUG_ON(!pdev->ats.cap_pos);
 
-    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                            pdev->ats.cap_pos + ATS_REG_CTL);
+    value = pci_conf_read16(pdev->sbdf, pdev->ats.cap_pos + ATS_REG_CTL);
     value &= ~ATS_ENABLE;
     pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                      pdev->ats.cap_pos + ATS_REG_CTL, value);
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index e3f883fc5c..5e5e0f0538 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -15,7 +15,7 @@ int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
     u8 pos = PCI_CAPABILITY_LIST;
     u16 status;
 
-    status = pci_conf_read16(seg, bus, dev, func, PCI_STATUS);
+    status = pci_conf_read16(PCI_SBDF(seg, bus, dev, func), PCI_STATUS);
     if ( (status & PCI_STATUS_CAP_LIST) == 0 )
         return 0;
 
@@ -120,7 +120,7 @@ void pci_intx(const struct pci_dev *pdev, bool enable)
     uint8_t bus = pdev->bus;
     uint8_t slot = PCI_SLOT(pdev->devfn);
     uint8_t func = PCI_FUNC(pdev->devfn);
-    uint16_t cmd = pci_conf_read16(seg, bus, slot, func, PCI_COMMAND);
+    uint16_t cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
 
     if ( enable )
         cmd &= ~PCI_COMMAND_INTX_DISABLE;
diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c
index 78533ad0b1..454457ade8 100644
--- a/xen/drivers/video/vga.c
+++ b/xen/drivers/video/vga.c
@@ -121,10 +121,9 @@ void __init video_endboot(void)
                 pcidevs_unlock();
 
                 if ( !pdev ||
-                     pci_conf_read16(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                     pci_conf_read16(PCI_SBDF3(0, bus, devfn),
                                      PCI_CLASS_DEVICE) != 0x0300 ||
-                     !(pci_conf_read16(0, bus, PCI_SLOT(devfn),
-                                       PCI_FUNC(devfn), PCI_COMMAND) &
+                     !(pci_conf_read16(PCI_SBDF3(0, bus, devfn), PCI_COMMAND) &
                        (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) )
                     continue;
 
@@ -141,8 +140,7 @@ void __init video_endboot(void)
                         {
                         case PCI_HEADER_TYPE_BRIDGE:
                         case PCI_HEADER_TYPE_CARDBUS:
-                            if ( pci_conf_read16(0, b, PCI_SLOT(df),
-                                                 PCI_FUNC(df),
+                            if ( pci_conf_read16(PCI_SBDF3(0, b, df),
                                                  PCI_BRIDGE_CONTROL) &
                                  PCI_BRIDGE_CTL_VGA )
                                 continue;
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 564c7b6a7d..0b176b490a 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -336,8 +336,7 @@ static void cmd_write(const struct pci_dev *pdev, unsigned int reg,
                       uint32_t cmd, void *data)
 {
     uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
-    uint16_t current_cmd = pci_conf_read16(pdev->seg, pdev->bus, slot, func,
-                                           reg);
+    uint16_t current_cmd = pci_conf_read16(pdev->sbdf, reg);
 
     /*
      * Let Dom0 play with all the bits directly except for the memory
@@ -371,8 +370,7 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
     else
         val &= PCI_BASE_ADDRESS_MEM_MASK;
 
-    if ( pci_conf_read16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND) &
-         PCI_COMMAND_MEMORY )
+    if ( pci_conf_read16(pdev->sbdf, PCI_COMMAND) & PCI_COMMAND_MEMORY )
     {
         /* If the value written is the current one avoid printing a warning. */
         if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
@@ -409,8 +407,7 @@ static void rom_write(const struct pci_dev *pdev, unsigned int reg,
     struct vpci_header *header = &pdev->vpci->header;
     struct vpci_bar *rom = data;
     uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
-    uint16_t cmd = pci_conf_read16(pdev->seg, pdev->bus, slot, func,
-                                   PCI_COMMAND);
+    uint16_t cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
     bool new_enabled = val & PCI_ROM_ADDRESS_ENABLE;
 
     if ( (cmd & PCI_COMMAND_MEMORY) && header->rom_enabled && new_enabled )
@@ -489,7 +486,7 @@ static int init_bars(struct pci_dev *pdev)
         return 0;
 
     /* Disable memory decoding before sizing. */
-    cmd = pci_conf_read16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND);
+    cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
     if ( cmd & PCI_COMMAND_MEMORY )
         pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
                          cmd & ~PCI_COMMAND_MEMORY);
diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
index c4e1d2a411..8fe89fc912 100644
--- a/xen/drivers/vpci/msi.c
+++ b/xen/drivers/vpci/msi.c
@@ -211,8 +211,7 @@ static int init_msi(struct pci_dev *pdev)
         return ret;
 
     /* Get the maximum number of vectors the device supports. */
-    control = pci_conf_read16(pdev->seg, pdev->bus, slot, func,
-                              msi_control_reg(pos));
+    control = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
 
     /*
      * FIXME: I've only been able to test this code with devices using a single
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index af3ffa087d..8e6cd070d0 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -457,8 +457,7 @@ static int init_msix(struct pci_dev *pdev)
     if ( !msix_offset )
         return 0;
 
-    control = pci_conf_read16(pdev->seg, pdev->bus, slot, func,
-                              msix_control_reg(msix_offset));
+    control = pci_conf_read16(pdev->sbdf, msix_control_reg(msix_offset));
 
     max_entries = msix_table_size(control);
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index c4030333a5..1a4c2ee4f1 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -114,8 +114,7 @@ static void vpci_ignored_write(const struct pci_dev *pdev, unsigned int reg,
 uint32_t vpci_hw_read16(const struct pci_dev *pdev, unsigned int reg,
                         void *data)
 {
-    return pci_conf_read16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                           PCI_FUNC(pdev->devfn), reg);
+    return pci_conf_read16(pdev->sbdf, reg);
 }
 
 uint32_t vpci_hw_read32(const struct pci_dev *pdev, unsigned int reg,
@@ -223,19 +222,17 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
         if ( reg & 1 )
         {
             data = pci_conf_read8(sbdf, reg);
-            data |= pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
-                                    reg + 1) << 8;
+            data |= pci_conf_read16(sbdf, reg + 1) << 8;
         }
         else
         {
-            data = pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn,
-                                   reg);
+            data = pci_conf_read16(sbdf, reg);
             data |= pci_conf_read8(sbdf, reg + 2) << 16;
         }
         break;
 
     case 2:
-        data = pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg);
+        data = pci_conf_read16(sbdf, reg);
         break;
 
     case 1:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 408cd1cb67..2847a8e6af 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -173,9 +173,7 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg,
 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(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    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);
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 2/6] pci: switch pci_conf_read16 " Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  2019-07-23 10:15   ` Jan Beulich
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 " Roger Pau Monne
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Brian Woods,
	Roger Pau Monne

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(-)

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);
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
                   ` (2 preceding siblings ...)
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 " Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  2019-07-23 10:20   ` Jan Beulich
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 5/6] pci: switch pci_conf_write16 " Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 6/6] pci: switch pci_conf_write32 " Roger Pau Monne
  5 siblings, 1 reply; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, Roger Pau Monne

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: "Roger Pau Monné" <roger.pau@citrix.com>
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>
---
Changes since v3:
 - Drop pointless AND-ing by constants.
 - Remove insertion of stray newline.
---
 xen/arch/x86/cpu/amd.c       |  2 +-
 xen/arch/x86/x86_64/pci.c    | 21 ++++++++-------------
 xen/drivers/acpi/reboot.c    |  5 ++---
 xen/drivers/char/ehci-dbgp.c |  8 +++++---
 xen/drivers/vpci/vpci.c      |  8 +++-----
 xen/include/xen/pci.h        |  4 +---
 6 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 8a40373ddf..f66de362cb 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -426,7 +426,7 @@ static void disable_c1_ramping(void)
 		if (pmm7 == 0xFF)
 			break;
 		pmm7 &= 0xFC; /* clear pmm7[1:0] */
-		pci_conf_write8(0, 0, 0x18+node, 0x3, 0x87, pmm7);
+		pci_conf_write8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87, pmm7);
 		printk ("AMD: Disabling C1 Clock Ramping Node #%x\n", node);
 	}
 }
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index b8b82a6fe7..eaa67b04f2 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -50,23 +50,18 @@ uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
     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)
+void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data)
 {
-    if ( seg || reg > 255 )
-        pci_mmcfg_write(seg, bus, PCI_DEVFN(dev, func), reg, 1, data);
+    if ( sbdf.seg || reg > 255 )
+        pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, data);
     else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1, data);
-    }
+        pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, data);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 void pci_conf_write16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint16_t data)
diff --git a/xen/drivers/acpi/reboot.c b/xen/drivers/acpi/reboot.c
index 72d06fd8e5..f6345be874 100644
--- a/xen/drivers/acpi/reboot.c
+++ b/xen/drivers/acpi/reboot.c
@@ -23,9 +23,8 @@ void acpi_reboot(void)
 	case ACPI_ADR_SPACE_PCI_CONFIG:
 		printk("Resetting with ACPI PCI RESET_REG.\n");
 		/* Write the value that resets us. */
-		pci_conf_write8(0, 0,
-				(rr->address >> 32) & 31,
-				(rr->address >> 16) & 7,
+		pci_conf_write8(PCI_SBDF(0, 0, rr->address >> 32,
+					 rr->address >> 16),
 				(rr->address & 255),
 				reset_value);
 		break;
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 9b9025fb33..010fc3c5bc 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1048,7 +1048,8 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
     if ( (cap & 0xff) == 1 && (cap & EHCI_USBLEGSUP_BIOS) )
     {
         dbgp_printk("dbgp: BIOS handoff\n");
-        pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 3, 1);
+        pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                        offset + 3, 1);
     }
 
     /* if boot firmware now owns EHCI, spin till it hands it over. */
@@ -1066,11 +1067,12 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
         /* well, possibly buggy BIOS... try to shut it down,
          * and hope nothing goes too wrong */
         dbgp_printk("dbgp: BIOS handoff failed: %08x\n", cap);
-        pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 2, 0);
+        pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                        offset + 2, 0);
     }
 
     /* just in case, always disable EHCI SMIs */
-    pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func,
+    pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                     offset + EHCI_USBLEGCTLSTS, 0);
 }
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 2106255863..b141e57883 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -263,8 +263,7 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
          */
         if ( reg & 1 )
         {
-            pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
-                            data);
+            pci_conf_write8(sbdf, reg, data);
             pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 1,
                              data >> 8);
         }
@@ -272,8 +271,7 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         {
             pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
                              data);
-            pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 2,
-                            data >> 16);
+            pci_conf_write8(sbdf, reg + 2, data >> 16);
         }
         break;
 
@@ -282,7 +280,7 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         break;
 
     case 1:
-        pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg, data);
+        pci_conf_write8(sbdf, reg, data);
         break;
 
     default:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 6885408687..144cfb2e62 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -175,9 +175,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(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);
+void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data);
 void pci_conf_write16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint16_t data);
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 5/6] pci: switch pci_conf_write16 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
                   ` (3 preceding siblings ...)
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 " Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 6/6] pci: switch pci_conf_write32 " Roger Pau Monne
  5 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, Roger Pau Monne

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: Jan Beulich <jbeulich@suse.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>
---
Changes since v3:
 - Remove stray change of pci_conf_write32 call.
---
 xen/arch/x86/msi.c                | 53 ++++++++++++++-----------------
 xen/arch/x86/x86_64/pci.c         | 21 +++++-------
 xen/drivers/char/ehci-dbgp.c      |  6 ++--
 xen/drivers/char/ns16550.c        |  9 ++++--
 xen/drivers/passthrough/pci.c     | 18 ++++-------
 xen/drivers/passthrough/x86/ats.c |  6 ++--
 xen/drivers/pci/pci.c             |  6 +---
 xen/drivers/vpci/header.c         | 22 ++++---------
 xen/drivers/vpci/msi.c            |  4 +--
 xen/drivers/vpci/msix.c           |  2 +-
 xen/drivers/vpci/vpci.c           |  8 ++---
 xen/include/xen/pci.h             |  4 +--
 12 files changed, 62 insertions(+), 97 deletions(-)

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 52617cd843..f3e5c5cb03 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -267,12 +267,10 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
         {
             pci_conf_write32(seg, bus, slot, func, msi_upper_address_reg(pos),
                              msg->address_hi);
-            pci_conf_write16(seg, bus, slot, func, msi_data_reg(pos, 1),
-                             msg->data);
+            pci_conf_write16(dev->sbdf, msi_data_reg(pos, 1), msg->data);
         }
         else
-            pci_conf_write16(seg, bus, slot, func, msi_data_reg(pos, 0),
-                             msg->data);
+            pci_conf_write16(dev->sbdf, msi_data_reg(pos, 0), msg->data);
         break;
     }
     case PCI_CAP_ID_MSIX:
@@ -329,7 +327,8 @@ void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable)
     control &= ~PCI_MSI_FLAGS_ENABLE;
     if ( enable )
         control |= PCI_MSI_FLAGS_ENABLE;
-    pci_conf_write16(seg, bus, slot, func, pos + PCI_MSI_FLAGS, control);
+    pci_conf_write16(PCI_SBDF(seg, bus, slot, func),
+                     pos + PCI_MSI_FLAGS, control);
 }
 
 static void msi_set_enable(struct pci_dev *dev, int enable)
@@ -360,7 +359,7 @@ static void msix_set_enable(struct pci_dev *dev, int enable)
         control &= ~PCI_MSIX_FLAGS_ENABLE;
         if ( enable )
             control |= PCI_MSIX_FLAGS_ENABLE;
-        pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
+        pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
     }
 }
 
@@ -406,7 +405,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
         if ( unlikely(!(control & PCI_MSIX_FLAGS_ENABLE)) )
         {
             pdev->msix->host_maskall = 1;
-            pci_conf_write16(seg, bus, slot, func,
+            pci_conf_write16(pdev->sbdf,
                              msix_control_reg(entry->msi_attrib.pos),
                              control | (PCI_MSIX_FLAGS_ENABLE |
                                         PCI_MSIX_FLAGS_MASKALL));
@@ -440,7 +439,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
         pdev->msix->host_maskall = maskall;
         if ( maskall || pdev->msix->guest_maskall )
             control |= PCI_MSIX_FLAGS_MASKALL;
-        pci_conf_write16(seg, bus, slot, func,
+        pci_conf_write16(pdev->sbdf,
                          msix_control_reg(entry->msi_attrib.pos), control);
         return flag;
     default:
@@ -580,8 +579,7 @@ int setup_msi_irq(struct irq_desc *desc, struct msi_desc *msidesc)
     {
         control = pci_conf_read16(pdev->sbdf, cpos);
         if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
-            pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                             PCI_FUNC(pdev->devfn), cpos,
+            pci_conf_write16(pdev->sbdf, cpos,
                              control | (PCI_MSIX_FLAGS_ENABLE |
                                         PCI_MSIX_FLAGS_MASKALL));
     }
@@ -591,8 +589,7 @@ int setup_msi_irq(struct irq_desc *desc, struct msi_desc *msidesc)
                                                    : &pci_msi_nonmaskable);
 
     if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
-        pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                         PCI_FUNC(pdev->devfn), cpos, control);
+        pci_conf_write16(pdev->sbdf, cpos, control);
 
     return rc;
 }
@@ -735,7 +732,7 @@ static int msi_capability_init(struct pci_dev *dev,
         pci_intx(dev, false);
         control |= PCI_MSI_FLAGS_ENABLE;
     }
-    pci_conf_write16(seg, bus, slot, func, msi_control_reg(pos), control);
+    pci_conf_write16(dev->sbdf, msi_control_reg(pos), control);
 
     return 0;
 }
@@ -856,13 +853,13 @@ static int msix_capability_init(struct pci_dev *dev,
      * fully set up.
      */
     msix->host_maskall = 1;
-    pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+    pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                      control | (PCI_MSIX_FLAGS_ENABLE |
                                 PCI_MSIX_FLAGS_MASKALL));
 
     if ( unlikely(!memory_decoded(dev)) )
     {
-        pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+        pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                          control & ~PCI_MSIX_FLAGS_ENABLE);
         return -ENXIO;
     }
@@ -872,7 +869,7 @@ static int msix_capability_init(struct pci_dev *dev,
         entry = alloc_msi_entry(1);
         if ( !entry )
         {
-            pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+            pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                              control & ~PCI_MSIX_FLAGS_ENABLE);
             return -ENOMEM;
         }
@@ -905,7 +902,7 @@ static int msix_capability_init(struct pci_dev *dev,
     {
         if ( !msi || !msi->table_base )
         {
-            pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+            pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                              control & ~PCI_MSIX_FLAGS_ENABLE);
             xfree(entry);
             return -ENXIO;
@@ -948,7 +945,7 @@ static int msix_capability_init(struct pci_dev *dev,
 
         if ( idx < 0 )
         {
-            pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+            pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                              control & ~PCI_MSIX_FLAGS_ENABLE);
             xfree(entry);
             return idx;
@@ -1024,7 +1021,7 @@ static int msix_capability_init(struct pci_dev *dev,
         maskall = 0;
     }
     msix->host_maskall = maskall;
-    pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
+    pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
 
     return 0;
 }
@@ -1166,7 +1163,7 @@ static void __pci_disable_msix(struct msi_desc *entry)
     if ( unlikely(!(control & PCI_MSIX_FLAGS_ENABLE)) )
     {
         dev->msix->host_maskall = 1;
-        pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos),
+        pci_conf_write16(dev->sbdf, msix_control_reg(pos),
                          control | (PCI_MSIX_FLAGS_ENABLE |
                                     PCI_MSIX_FLAGS_MASKALL));
     }
@@ -1185,7 +1182,7 @@ static void __pci_disable_msix(struct msi_desc *entry)
     dev->msix->host_maskall = maskall;
     if ( maskall || dev->msix->guest_maskall )
         control |= PCI_MSIX_FLAGS_MASKALL;
-    pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
+    pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
 
     _pci_cleanup_msix(dev->msix);
 }
@@ -1381,8 +1378,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
                     pdev->seg, pdev->bus, slot, func, i);
             spin_unlock_irqrestore(&desc->lock, flags);
             if ( type == PCI_CAP_ID_MSIX )
-                pci_conf_write16(pdev->seg, pdev->bus, slot, func,
-                                 msix_control_reg(pos),
+                pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
                                  control & ~PCI_MSIX_FLAGS_ENABLE);
             return -EINVAL;
         }
@@ -1397,15 +1393,13 @@ int pci_restore_msi_state(struct pci_dev *pdev)
         else if ( !type && entry->msi_attrib.type == PCI_CAP_ID_MSIX )
         {
             control = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
-            pci_conf_write16(pdev->seg, pdev->bus, slot, func,
-                             msix_control_reg(pos),
+            pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
                              control | (PCI_MSIX_FLAGS_ENABLE |
                                         PCI_MSIX_FLAGS_MASKALL));
             if ( unlikely(!memory_decoded(pdev)) )
             {
                 spin_unlock_irqrestore(&desc->lock, flags);
-                pci_conf_write16(pdev->seg, pdev->bus, slot, func,
-                                 msix_control_reg(pos),
+                pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
                                  control & ~PCI_MSIX_FLAGS_ENABLE);
                 return -ENXIO;
             }
@@ -1440,15 +1434,14 @@ int pci_restore_msi_state(struct pci_dev *pdev)
 
             control = pci_conf_read16(pdev->sbdf, cpos) & ~PCI_MSI_FLAGS_QSIZE;
             multi_msi_enable(control, entry->msi.nvec);
-            pci_conf_write16(pdev->seg, pdev->bus, slot, func, cpos, control);
+            pci_conf_write16(pdev->sbdf, cpos, control);
 
             msi_set_enable(pdev, 1);
         }
     }
 
     if ( type == PCI_CAP_ID_MSIX )
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func,
-                         msix_control_reg(pos),
+        pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
                          control | PCI_MSIX_FLAGS_ENABLE);
 
     return 0;
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index eaa67b04f2..f014fe0fc5 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -58,23 +58,18 @@ void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data)
         pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, data);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-void pci_conf_write16(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint16_t data)
+void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg, uint16_t data)
 {
-    if ( seg || reg > 255 )
-        pci_mmcfg_write(seg, bus, PCI_DEVFN(dev, func), reg, 2, data);
+    if ( sbdf.seg || reg > 255 )
+        pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, data);
     else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 2, 2, data);
-    }
+        pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2, data);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 void pci_conf_write32(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint32_t data)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 010fc3c5bc..b780334953 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1324,8 +1324,8 @@ static void __init ehci_dbgp_init_preirq(struct serial_port *port)
     if ( !(dbgp->pci_cr & PCI_COMMAND_MEMORY) )
     {
         dbgp->pci_cr |= PCI_COMMAND_MEMORY;
-        pci_conf_write16(0, dbgp->bus, dbgp->slot, dbgp->func, PCI_COMMAND,
-                         dbgp->pci_cr);
+        pci_conf_write16(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                         PCI_COMMAND, dbgp->pci_cr);
         dbgp_printk("MMIO for EHCI enabled\n");
     }
 
@@ -1438,7 +1438,7 @@ static void ehci_dbgp_resume(struct serial_port *port)
 
     pci_conf_write32(0, dbgp->bus, dbgp->slot, dbgp->func, dbgp->bar,
                      dbgp->bar_val);
-    pci_conf_write16(0, dbgp->bus, dbgp->slot, dbgp->func,
+    pci_conf_write16(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                      PCI_COMMAND, dbgp->pci_cr);
 
     ehci_dbgp_setup_preirq(dbgp);
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index fe71406cc1..20eaecee59 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -624,7 +624,8 @@ static void pci_serial_early_init(struct ns16550 *uart)
         return;
 
     if ( uart->pb_bdf_enable )
-        pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
+        pci_conf_write16(PCI_SBDF(0, uart->pb_bdf[0], uart->pb_bdf[1],
+                                  uart->pb_bdf[2]),
                          PCI_IO_BASE,
                          (uart->io_base & 0xF000) |
                          ((uart->io_base & 0xF000) >> 8));
@@ -632,7 +633,8 @@ static void pci_serial_early_init(struct ns16550 *uart)
     pci_conf_write32(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
                      PCI_BASE_ADDRESS_0,
                      uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
-    pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
+    pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
+                              uart->ps_bdf[2]),
                      PCI_COMMAND, PCI_COMMAND_IO);
 #endif
 }
@@ -867,7 +869,8 @@ static void _ns16550_resume(struct serial_port *port)
                         uart->ps_bdf[1], uart->ps_bdf[2],
                         PCI_BASE_ADDRESS_0 + (uart->bar_idx+1)*4, uart->bar64);
 
-       pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
+       pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
+                                 uart->ps_bdf[2]),
                         PCI_COMMAND, uart->cr);
     }
 #endif
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 7f9573ceee..ee2b9cc01c 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -248,15 +248,13 @@ static void check_pdev(const struct pci_dev *pdev)
     {
         val = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
         if ( val & command_mask )
-            pci_conf_write16(seg, bus, dev, func, PCI_COMMAND,
-                             val & ~command_mask);
+            pci_conf_write16(pdev->sbdf, PCI_COMMAND, val & ~command_mask);
         val = pci_conf_read16(pdev->sbdf, PCI_STATUS);
         if ( val & PCI_STATUS_CHECK )
         {
             printk(XENLOG_INFO "%04x:%02x:%02x.%u status %04x -> %04x\n",
                    seg, bus, dev, func, val, val & ~PCI_STATUS_CHECK);
-            pci_conf_write16(seg, bus, dev, func, PCI_STATUS,
-                             val & PCI_STATUS_CHECK);
+            pci_conf_write16(pdev->sbdf, PCI_STATUS, val & PCI_STATUS_CHECK);
         }
     }
 
@@ -267,7 +265,7 @@ static void check_pdev(const struct pci_dev *pdev)
             break;
         val = pci_conf_read16(pdev->sbdf, PCI_BRIDGE_CONTROL);
         if ( val & bridge_ctl_mask )
-            pci_conf_write16(seg, bus, dev, func, PCI_BRIDGE_CONTROL,
+            pci_conf_write16(pdev->sbdf, PCI_BRIDGE_CONTROL,
                              val & ~bridge_ctl_mask);
         val = pci_conf_read16(pdev->sbdf, PCI_SEC_STATUS);
         if ( val & PCI_STATUS_CHECK )
@@ -275,7 +273,7 @@ static void check_pdev(const struct pci_dev *pdev)
             printk(XENLOG_INFO
                    "%04x:%02x:%02x.%u secondary status %04x -> %04x\n",
                    seg, bus, dev, func, val, val & ~PCI_STATUS_CHECK);
-            pci_conf_write16(seg, bus, dev, func, PCI_SEC_STATUS,
+            pci_conf_write16(pdev->sbdf, PCI_SEC_STATUS,
                              val & PCI_STATUS_CHECK);
         }
         break;
@@ -596,8 +594,6 @@ static void pci_enable_acs(struct pci_dev *pdev)
     int pos;
     u16 cap, ctrl, seg = pdev->seg;
     u8 bus = pdev->bus;
-    u8 dev = PCI_SLOT(pdev->devfn);
-    u8 func = PCI_FUNC(pdev->devfn);
 
     if ( !iommu_enabled )
         return;
@@ -621,7 +617,7 @@ static void pci_enable_acs(struct pci_dev *pdev)
     /* Upstream Forwarding */
     ctrl |= (cap & PCI_ACS_UF);
 
-    pci_conf_write16(seg, bus, dev, func, pos + PCI_ACS_CTRL, ctrl);
+    pci_conf_write16(pdev->sbdf, pos + PCI_ACS_CTRL, ctrl);
 }
 
 static int iommu_add_device(struct pci_dev *pdev);
@@ -1031,10 +1027,8 @@ void pci_check_disable_device(u16 seg, u8 bus, u8 devfn)
 
     /* Tell the device to stop DMAing; we can't rely on the guest to
      * control it for us. */
-    devfn = pdev->devfn;
     cword = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
-    pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                     PCI_COMMAND, cword & ~PCI_COMMAND_MASTER);
+    pci_conf_write16(pdev->sbdf, PCI_COMMAND, cword & ~PCI_COMMAND_MASTER);
 }
 
 /*
diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index cb022c598a..3eea7f89fc 100644
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -50,8 +50,7 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
     if ( !(value & ATS_ENABLE) )
     {
         value |= ATS_ENABLE;
-        pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                         pos + ATS_REG_CTL, value);
+        pci_conf_write16(pdev->sbdf, pos + ATS_REG_CTL, value);
     }
 
     if ( pos )
@@ -81,8 +80,7 @@ void disable_ats_device(struct pci_dev *pdev)
 
     value = pci_conf_read16(pdev->sbdf, pdev->ats.cap_pos + ATS_REG_CTL);
     value &= ~ATS_ENABLE;
-    pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                     pdev->ats.cap_pos + ATS_REG_CTL, value);
+    pci_conf_write16(pdev->sbdf, pdev->ats.cap_pos + ATS_REG_CTL, value);
 
     list_del(&pdev->ats.list);
 
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index b24702e0c3..4de5fdf679 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -116,17 +116,13 @@ int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap
 
 void pci_intx(const struct pci_dev *pdev, bool enable)
 {
-    uint16_t seg = pdev->seg;
-    uint8_t bus = pdev->bus;
-    uint8_t slot = PCI_SLOT(pdev->devfn);
-    uint8_t func = PCI_FUNC(pdev->devfn);
     uint16_t cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
 
     if ( enable )
         cmd &= ~PCI_COMMAND_INTX_DISABLE;
     else
         cmd |= PCI_COMMAND_INTX_DISABLE;
-    pci_conf_write16(seg, bus, slot, func, PCI_COMMAND, cmd);
+    pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
 }
 
 const char *__init parse_pci(const char *s, unsigned int *seg_p,
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 7476634982..f377e6abdf 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -123,8 +123,7 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
     }
 
     if ( !rom_only )
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
-                         cmd);
+        pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
     else
         ASSERT_UNREACHABLE();
 }
@@ -335,7 +334,6 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only)
 static void cmd_write(const struct pci_dev *pdev, unsigned int reg,
                       uint32_t cmd, void *data)
 {
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     uint16_t current_cmd = pci_conf_read16(pdev->sbdf, reg);
 
     /*
@@ -351,7 +349,7 @@ static void cmd_write(const struct pci_dev *pdev, unsigned int reg,
          */
         modify_bars(pdev, cmd, false);
     else
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd);
+        pci_conf_write16(pdev->sbdf, reg, cmd);
 }
 
 static void bar_write(const struct pci_dev *pdev, unsigned int reg,
@@ -397,8 +395,7 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
         val |= bar->prefetchable ? PCI_BASE_ADDRESS_MEM_PREFETCH : 0;
     }
 
-    pci_conf_write32(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                     PCI_FUNC(pdev->devfn), reg, val);
+    pci_conf_write32(pdev->seg, pdev->bus, slot, func, reg, val);
 }
 
 static void rom_write(const struct pci_dev *pdev, unsigned int reg,
@@ -452,7 +449,6 @@ static void rom_write(const struct pci_dev *pdev, unsigned int reg,
 
 static int init_bars(struct pci_dev *pdev)
 {
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     uint16_t cmd;
     uint64_t addr, size;
     unsigned int i, num_bars, rom_reg;
@@ -488,8 +484,7 @@ static int init_bars(struct pci_dev *pdev)
     /* Disable memory decoding before sizing. */
     cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
     if ( cmd & PCI_COMMAND_MEMORY )
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
-                         cmd & ~PCI_COMMAND_MEMORY);
+        pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
 
     for ( i = 0; i < num_bars; i++ )
     {
@@ -503,8 +498,7 @@ static int init_bars(struct pci_dev *pdev)
                                    4, &bars[i]);
             if ( rc )
             {
-                pci_conf_write16(pdev->seg, pdev->bus, slot, func,
-                                 PCI_COMMAND, cmd);
+                pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
                 return rc;
             }
 
@@ -527,8 +521,7 @@ static int init_bars(struct pci_dev *pdev)
                               (i == num_bars - 1) ? PCI_BAR_LAST : 0);
         if ( rc < 0 )
         {
-            pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
-                             cmd);
+            pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
             return rc;
         }
 
@@ -546,8 +539,7 @@ static int init_bars(struct pci_dev *pdev)
                                &bars[i]);
         if ( rc )
         {
-            pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
-                             cmd);
+            pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
             return rc;
         }
     }
diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
index 8fe89fc912..5b6602f3c2 100644
--- a/xen/drivers/vpci/msi.c
+++ b/xen/drivers/vpci/msi.c
@@ -77,9 +77,7 @@ static void control_write(const struct pci_dev *pdev, unsigned int reg,
     msi->vectors = vectors;
     msi->enabled = new_enabled;
 
-    pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                     PCI_FUNC(pdev->devfn), reg,
-                     control_read(pdev, reg, data));
+    pci_conf_write16(pdev->sbdf, reg, control_read(pdev, reg, data));
 }
 
 static void update_msi(const struct pci_dev *pdev, struct vpci_msi *msi)
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index c60cba0137..38c1e7e5dd 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -146,7 +146,7 @@ static void control_write(const struct pci_dev *pdev, unsigned int reg,
 
     val = control_read(pdev, reg, data);
     if ( pci_msi_conf_write_intercept(msix->pdev, reg, 2, &val) >= 0 )
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, val);
+        pci_conf_write16(pdev->sbdf, reg, val);
 }
 
 static struct vpci_msix *msix_find(const struct domain *d, unsigned long addr)
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index b141e57883..6cef5e420a 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -264,19 +264,17 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         if ( reg & 1 )
         {
             pci_conf_write8(sbdf, reg, data);
-            pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 1,
-                             data >> 8);
+            pci_conf_write16(sbdf, reg + 1, data >> 8);
         }
         else
         {
-            pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
-                             data);
+            pci_conf_write16(sbdf, reg, data);
             pci_conf_write8(sbdf, reg + 2, data >> 16);
         }
         break;
 
     case 2:
-        pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg, data);
+        pci_conf_write16(sbdf, reg, data);
         break;
 
     case 1:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 144cfb2e62..bfdfca057b 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -176,9 +176,7 @@ 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(pci_sbdf_t sbdf, unsigned int reg);
 void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data);
-void pci_conf_write16(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint16_t data);
+void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg, uint16_t data);
 void pci_conf_write32(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint32_t data);
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH v4 6/6] pci: switch pci_conf_write32 to use pci_sbdf_t
  2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
                   ` (4 preceding siblings ...)
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 5/6] pci: switch pci_conf_write16 " Roger Pau Monne
@ 2019-07-19 14:07 ` Roger Pau Monne
  5 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monne @ 2019-07-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Brian Woods,
	Roger Pau Monne

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: Jan Beulich <jbeulich@suse.com>
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                   |  4 ++--
 xen/arch/x86/msi.c                       | 12 ++++--------
 xen/arch/x86/oprofile/op_model_athlon.c  |  4 +++-
 xen/arch/x86/x86_64/pci.c                | 17 ++++-------------
 xen/drivers/char/ehci-dbgp.c             |  5 +++--
 xen/drivers/char/ns16550.c               | 22 ++++++++++++----------
 xen/drivers/passthrough/amd/iommu_init.c |  8 ++++----
 xen/drivers/passthrough/pci.c            |  8 ++++----
 xen/drivers/passthrough/vtd/quirks.c     |  8 ++++----
 xen/drivers/vpci/header.c                |  7 +++----
 xen/drivers/vpci/vpci.c                  |  2 +-
 xen/include/xen/pci.h                    |  4 +---
 12 files changed, 45 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index f66de362cb..a2f83c79a5 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -714,11 +714,11 @@ static void init_amd(struct cpuinfo_x86 *c)
 				       (h & 0x1) ? "clearing D18F3x5C[0]" : "");
 
 			if (l & 0x1f)
-				pci_conf_write32(0, 0, 0x18, 0x3, 0x58,
+				pci_conf_write32(PCI_SBDF(0, 0, 0x18, 3), 0x58,
 						 l & ~0x1f);
 
 			if (h & 0x1)
-				pci_conf_write32(0, 0, 0x18, 0x3, 0x5c,
+				pci_conf_write32(PCI_SBDF(0, 0, 0x18, 3), 0x5c,
 						 h & ~0x1);
 		}
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index f3e5c5cb03..d6306005a9 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -251,21 +251,17 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
     {
         struct pci_dev *dev = entry->dev;
         int pos = entry->msi_attrib.pos;
-        u16 seg = dev->seg;
-        u8 bus = dev->bus;
-        u8 slot = PCI_SLOT(dev->devfn);
-        u8 func = PCI_FUNC(dev->devfn);
         int nr = entry->msi_attrib.entry_nr;
 
         ASSERT((msg->data & (entry[-nr].msi.nvec - 1)) == nr);
         if ( nr )
             return 0;
 
-        pci_conf_write32(seg, bus, slot, func, msi_lower_address_reg(pos),
+        pci_conf_write32(dev->sbdf, msi_lower_address_reg(pos),
                          msg->address_lo);
         if ( entry->msi_attrib.is_64 )
         {
-            pci_conf_write32(seg, bus, slot, func, msi_upper_address_reg(pos),
+            pci_conf_write32(dev->sbdf, msi_upper_address_reg(pos),
                              msg->address_hi);
             pci_conf_write16(dev->sbdf, msi_data_reg(pos, 1), msg->data);
         }
@@ -395,7 +391,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
             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);
+            pci_conf_write32(pdev->sbdf, entry->msi.mpos, mask_bits);
         }
         break;
     case PCI_CAP_ID_MSIX:
@@ -716,7 +712,7 @@ static int msi_capability_init(struct pci_dev *dev,
         /* All MSIs are unmasked by default, Mask them all */
         maskbits = pci_conf_read32(dev->sbdf, mpos);
         maskbits |= ~(u32)0 >> (32 - maxvec);
-        pci_conf_write32(seg, bus, slot, func, mpos, maskbits);
+        pci_conf_write32(dev->sbdf, mpos, maskbits);
     }
     list_add_tail(&entry->list, &dev->msi_list);
 
diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
index 3bf0b0214d..5c48f868ae 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -472,7 +472,9 @@ static int __init init_ibs_nmi(void)
 				if ((vendor_id == PCI_VENDOR_ID_AMD) &&
 					(dev_id == PCI_DEVICE_ID_AMD_10H_NB_MISC)) {
 
-					pci_conf_write32(0, bus, dev, func, IBSCTL,
+					pci_conf_write32(
+						PCI_SBDF(0, bus, dev, func),
+						IBSCTL,
 						IBSCTL_LVTOFFSETVAL | APIC_EILVT_LVTOFF_IBS);
 
 					value = pci_conf_read32(PCI_SBDF(0, bus, dev, func),
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index f014fe0fc5..aad1c3f7cf 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -66,19 +66,10 @@ void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg, uint16_t data)
         pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2, data);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-void pci_conf_write32(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint32_t data)
+void pci_conf_write32(pci_sbdf_t sbdf, unsigned int reg, uint32_t data)
 {
-    if ( seg || reg > 255 )
-        pci_mmcfg_write(seg, bus, PCI_DEVFN(dev, func), reg, 4, data);
+    if ( sbdf.seg || reg > 255 )
+        pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, data);
     else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), 0, 4, data);
-    }
+        pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), 0, 4, data);
 }
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index b780334953..b6e155d17b 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1012,7 +1012,8 @@ static void nvidia_set_debug_port(struct ehci_dbgp *dbgp, unsigned int port)
 
     dword &= ~(0x0f << 12);
     dword |= (port & 0x0f) << 12;
-    pci_conf_write32(0, dbgp->bus, dbgp->slot, dbgp->func, 0x74, dword);
+    pci_conf_write32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func), 0x74,
+                     dword);
     dbgp_printk("set debug port to %u\n", port);
 }
 
@@ -1436,7 +1437,7 @@ static void ehci_dbgp_resume(struct serial_port *port)
     if ( !dbgp->ehci_debug )
         return;
 
-    pci_conf_write32(0, dbgp->bus, dbgp->slot, dbgp->func, dbgp->bar,
+    pci_conf_write32(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func), dbgp->bar,
                      dbgp->bar_val);
     pci_conf_write16(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                      PCI_COMMAND, dbgp->pci_cr);
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 20eaecee59..e518f2d790 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -630,7 +630,8 @@ static void pci_serial_early_init(struct ns16550 *uart)
                          (uart->io_base & 0xF000) |
                          ((uart->io_base & 0xF000) >> 8));
 
-    pci_conf_write32(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
+    pci_conf_write32(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
+                              uart->ps_bdf[2]),
                      PCI_BASE_ADDRESS_0,
                      uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
     pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
@@ -860,13 +861,14 @@ static void _ns16550_resume(struct serial_port *port)
 
     if ( uart->bar )
     {
-       pci_conf_write32(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
+       pci_conf_write32(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
+                                 uart->ps_bdf[2]),
                         PCI_BASE_ADDRESS_0 + uart->bar_idx*4, uart->bar);
 
         /* If 64 bit BAR, write higher 32 bits to BAR+4 */
         if ( uart->bar & PCI_BASE_ADDRESS_MEM_TYPE_64 )
-            pci_conf_write32(0, uart->ps_bdf[0],
-                        uart->ps_bdf[1], uart->ps_bdf[2],
+            pci_conf_write32(PCI_SBDF(0, uart->ps_bdf[0],  uart->ps_bdf[1],
+                                      uart->ps_bdf[2]),
                         PCI_BASE_ADDRESS_0 + (uart->bar_idx+1)*4, uart->bar64);
 
        pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
@@ -1121,11 +1123,11 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 /* MMIO based */
                 if ( param->mmio && !(bar & PCI_BASE_ADDRESS_SPACE_IO) )
                 {
-                    pci_conf_write32(0, b, d, f,
+                    pci_conf_write32(PCI_SBDF(0, b, d, f),
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
                     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_conf_write32(PCI_SBDF(0, b, d, f),
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
 
                     /* Handle 64 bit BAR if found */
@@ -1133,11 +1135,11 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                     {
                         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_conf_write32(PCI_SBDF(0, b, d, f),
                                     PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, ~0u);
                         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_conf_write32(PCI_SBDF(0, b, d, f),
                                     PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, bar_64);
                         size  = ((u64)~0 << 32) | PCI_BASE_ADDRESS_MEM_MASK;
                         size &= ((u64)len_64 << 32) | len;
@@ -1151,11 +1153,11 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
                 /* IO based */
                 else if ( !param->mmio && (bar & PCI_BASE_ADDRESS_SPACE_IO) )
                 {
-                    pci_conf_write32(0, b, d, f,
+                    pci_conf_write32(PCI_SBDF(0, b, d, f),
                                      PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
                     len = pci_conf_read32(PCI_SBDF(0, b, d, f),
                                           PCI_BASE_ADDRESS_0);
-                    pci_conf_write32(0, b, d, f,
+                    pci_conf_write32(PCI_SBDF(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_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index f60238a5b8..d970f88a27 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -844,22 +844,22 @@ static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
          (boot_cpu_data.x86_model > 0x1f) )
         return;
 
-    pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
+    pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf0, 0x90);
     value = pci_conf_read32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf4);
 
     if ( value & (1 << 2) )
         return;
 
     /* Select NB indirect register 0x90 and enable writing */
-    pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90 | (1 << 8));
+    pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf0, 0x90 | (1 << 8));
 
-    pci_conf_write32(iommu->seg, bus, dev, func, 0xf4, value | (1 << 2));
+    pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf4, value | (1 << 2));
     printk(XENLOG_INFO
            "AMD-Vi: Applying erratum 746 workaround for IOMMU at %04x:%02x:%02x.%u\n",
            iommu->seg, bus, dev, func);
 
     /* Clear the enable writing bit */
-    pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
+    pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf0, 0x90);
 }
 
 static void enable_iommu(struct amd_iommu *iommu)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index ee2b9cc01c..7c196ba58b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -638,7 +638,7 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
     ASSERT(!((flags & PCI_BAR_VF) && (flags & PCI_BAR_ROM)));
     ASSERT((flags & PCI_BAR_ROM) ||
            (bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY);
-    pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos, ~0);
+    pci_conf_write32(sbdf, pos, ~0);
     if ( is64bits )
     {
         if ( flags & PCI_BAR_LAST )
@@ -651,17 +651,17 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
             return 1;
         }
         hi = pci_conf_read32(sbdf, pos + 4);
-        pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, ~0);
+        pci_conf_write32(sbdf, pos + 4, ~0);
     }
     size = pci_conf_read32(sbdf, pos) & mask;
     if ( is64bits )
     {
         size |= (uint64_t)pci_conf_read32(sbdf, pos + 4) << 32;
-        pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos + 4, hi);
+        pci_conf_write32(sbdf, pos + 4, hi);
     }
     else if ( size )
         size |= (uint64_t)~0 << 32;
-    pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, pos, bar);
+    pci_conf_write32(sbdf, pos, bar);
     size = -size;
 
     if ( paddr )
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 40bc25d544..d1512b9347 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -439,7 +439,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
     case 0x3728: /* Xeon C5500/C3500 (JasperForest) */
     case 0x3c28: /* Sandybridge */
         val = pci_conf_read32(pdev->sbdf, 0x1AC);
-        pci_conf_write32(seg, bus, dev, func, 0x1AC, val | (1 << 31));
+        pci_conf_write32(pdev->sbdf, 0x1AC, val | (1 << 31));
         printk(XENLOG_INFO "Masked VT-d error signaling on %04x:%02x:%02x.%u\n",
                seg, bus, dev, func);
         break;
@@ -487,9 +487,9 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
             action = "Found masked";
         else if ( !ff )
         {
-            pci_conf_write32(seg, bus, dev, func, pos + PCI_ERR_UNCOR_MASK,
+            pci_conf_write32(pdev->sbdf, pos + PCI_ERR_UNCOR_MASK,
                              val | PCI_ERR_UNC_UNSUP);
-            pci_conf_write32(seg, bus, dev, func, pos + PCI_ERR_COR_MASK,
+            pci_conf_write32(pdev->sbdf, pos + PCI_ERR_COR_MASK,
                              val2 | PCI_ERR_COR_ADV_NFAT);
             action = "Masked";
         }
@@ -498,7 +498,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
 
         /* XPUNCERRMSK Send Completion with Unsupported Request */
         val = pci_conf_read32(pdev->sbdf, 0x20c);
-        pci_conf_write32(seg, bus, dev, func, 0x20c, val | (1 << 4));
+        pci_conf_write32(pdev->sbdf, 0x20c, val | (1 << 4));
 
         printk(XENLOG_INFO "%s UR signaling on %04x:%02x:%02x.%u\n",
                action, seg, bus, dev, func);
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index f377e6abdf..3c794f486d 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -85,7 +85,6 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
                             bool rom_only)
 {
     struct vpci_header *header = &pdev->vpci->header;
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     bool map = cmd & PCI_COMMAND_MEMORY;
     unsigned int i;
 
@@ -113,7 +112,7 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
                            (map ? PCI_ROM_ADDRESS_ENABLE : 0);
 
             header->bars[i].enabled = header->rom_enabled = map;
-            pci_conf_write32(pdev->seg, pdev->bus, slot, func, rom_pos, val);
+            pci_conf_write32(pdev->sbdf, rom_pos, val);
             return;
         }
 
@@ -395,7 +394,7 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
         val |= bar->prefetchable ? PCI_BASE_ADDRESS_MEM_PREFETCH : 0;
     }
 
-    pci_conf_write32(pdev->seg, pdev->bus, slot, func, reg, val);
+    pci_conf_write32(pdev->sbdf, reg, val);
 }
 
 static void rom_write(const struct pci_dev *pdev, unsigned int reg,
@@ -426,7 +425,7 @@ static void rom_write(const struct pci_dev *pdev, unsigned int reg,
     {
         /* Just update the ROM BAR field. */
         header->rom_enabled = new_enabled;
-        pci_conf_write32(pdev->seg, pdev->bus, slot, func, reg, val);
+        pci_conf_write32(pdev->sbdf, reg, val);
     }
     /*
      * Pass PCI_COMMAND_MEMORY or 0 to signal a map/unmap request, note that
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 6cef5e420a..758d9420e7 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -253,7 +253,7 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
     switch ( size )
     {
     case 4:
-        pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg, data);
+        pci_conf_write32(sbdf, reg, data);
         break;
 
     case 3:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index bfdfca057b..8148f5b2e7 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -177,9 +177,7 @@ uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg);
 uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg);
 void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data);
 void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg, uint16_t data);
-void pci_conf_write32(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint32_t data);
+void pci_conf_write32(pci_sbdf_t sbdf, unsigned int reg, uint32_t data);
 uint32_t pci_conf_read(uint32_t cf8, uint8_t offset, uint8_t bytes);
 void pci_conf_write(uint32_t cf8, uint8_t offset, uint8_t bytes, uint32_t data);
 int pci_mmcfg_read(unsigned int seg, unsigned int bus,
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 " Roger Pau Monne
@ 2019-07-23 10:15   ` Jan Beulich
  2019-07-23 11:04     ` Roger Pau Monné
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-07-23 10:15 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, AndrewCooper, Ian Jackson, TimDeegan,
	Julien Grall, Suravee Suthikulpanit, xen-devel, Brian Woods

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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 to use pci_sbdf_t
  2019-07-19 14:07 ` [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 " Roger Pau Monne
@ 2019-07-23 10:20   ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2019-07-23 10:20 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, GeorgeDunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel

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.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t
  2019-07-23 10:15   ` Jan Beulich
@ 2019-07-23 11:04     ` Roger Pau Monné
  0 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monné @ 2019-07-23 11:04 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, AndrewCooper, Ian Jackson, TimDeegan,
	Julien Grall, Suravee Suthikulpanit, xen-devel, Brian Woods

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-07-23 11:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 14:07 [Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t Roger Pau Monne
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t Roger Pau Monne
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 2/6] pci: switch pci_conf_read16 " Roger Pau Monne
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 " Roger Pau Monne
2019-07-23 10:15   ` Jan Beulich
2019-07-23 11:04     ` Roger Pau Monné
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 " Roger Pau Monne
2019-07-23 10:20   ` Jan Beulich
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 5/6] pci: switch pci_conf_write16 " Roger Pau Monne
2019-07-19 14:07 ` [Xen-devel] [PATCH v4 6/6] pci: switch pci_conf_write32 " Roger Pau Monne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).