diff mbox series

[v3,11/13] pci: switch pci_conf_write16 to use pci_sbdf_t

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

Commit Message

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

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/cpu/amd.c            |  4 +--
 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 +--
 13 files changed, 64 insertions(+), 99 deletions(-)

Comments

Jan Beulich June 17, 2019, 10:05 a.m. UTC | #1
>>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -707,11 +707,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(0, 0, 0x18, 3, 0x58,
>  						 l & ~0x1f);
>  
>  			if (h & 0x1)
> -				pci_conf_write32(0, 0, 0x18, 0x3, 0x5c,
> +				pci_conf_write32(0, 0, 0x18, 3, 0x5c,
>  						 h & ~0x1);
>  		}

These changes don't seem to belong here. With this taken care of
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 489d45fd25..2e6529fba3 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -707,11 +707,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(0, 0, 0x18, 3, 0x58,
 						 l & ~0x1f);
 
 			if (h & 0x1)
-				pci_conf_write32(0, 0, 0x18, 0x3, 0x5c,
+				pci_conf_write32(0, 0, 0x18, 3, 0x5c,
 						 h & ~0x1);
 		}
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 392cbecfe4..cbc1e3b3f0 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);
 }
@@ -1380,8 +1377,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;
         }
@@ -1396,15 +1392,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;
             }
@@ -1439,15 +1433,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 80887af66c..ff400e9a31 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 53c5821c20..b61672f295 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -265,19 +265,17 @@  static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         {
 
             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 3faa2a22d3..cb90d2f785 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);