diff mbox series

[5/5] pci: switch PCI capabilities related functions to use pci_sbdf_t

Message ID 20190510161056.48648-6-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é May 10, 2019, 4:10 p.m. UTC
Since pci_dev already has a pci_sbdf_t field switch the capability
related functions SBDF parameters to a single pci_sbdf_t parameter.

No functional change expected.

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 <wei.liu2@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>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/msi.c                         | 42 ++++++----------------
 xen/drivers/char/ehci-dbgp.c               |  3 +-
 xen/drivers/passthrough/amd/iommu_detect.c |  2 +-
 xen/drivers/passthrough/ats.h              | 12 +++++--
 xen/drivers/passthrough/pci.c              | 22 +++++-------
 xen/drivers/passthrough/vtd/quirks.c       | 12 +++----
 xen/drivers/passthrough/vtd/x86/ats.c      |  3 +-
 xen/drivers/passthrough/x86/ats.c          |  4 +--
 xen/drivers/pci/pci.c                      | 31 +++++-----------
 xen/drivers/vpci/msi.c                     |  4 +--
 xen/drivers/vpci/msix.c                    |  4 +--
 xen/include/xen/pci.h                      |  9 ++---
 12 files changed, 52 insertions(+), 96 deletions(-)

Comments

Jan Beulich May 24, 2019, 10:52 a.m. UTC | #1
>>> On 10.05.19 at 18:10, <roger.pau@citrix.com> wrote:
> --- a/xen/drivers/pci/pci.c
> +++ b/xen/drivers/pci/pci.c
> @@ -8,18 +8,12 @@
>  #include <xen/pci.h>
>  #include <xen/pci_regs.h>
>  
> -int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
> +int pci_find_cap_offset(pci_sbdf_t sbdf, unsigned int cap)

The secondary type change here and ...

> @@ -45,15 +39,10 @@ int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
>      return 0;
>  }
>  
> -int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap)
> +int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos, unsigned int cap)

... the two ones here aren't obviously safe, so should at least be
mentioned in the description. The latter function has no caller at
all, so is fine simply by that face, for the former this could in principle
result in change in behavior due to the compiler no longer truncating
possible out-of-range arguments. All callers look to be fine though.
(I don't view this as a potential issue for the "ext" counterparts, as
there it's only a change from plain int to unsigned int.)

Some of the comments given on earlier patches apply here as well.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 6832211772..27ea11b769 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -339,7 +339,7 @@  static void msi_set_enable(struct pci_dev *dev, int enable)
     uint8_t slot = dev->sbdf.dev;
     uint8_t func = dev->sbdf.func;
 
-    pos = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSI);
+    pos = pci_find_cap_offset(dev->sbdf, PCI_CAP_ID_MSI);
     if ( pos )
         __msi_set_enable(seg, bus, slot, func, pos, enable);
 }
@@ -347,12 +347,9 @@  static void msi_set_enable(struct pci_dev *dev, int enable)
 static void msix_set_enable(struct pci_dev *dev, int enable)
 {
     int pos;
-    uint16_t control, seg = dev->sbdf.seg;
-    uint8_t bus = dev->sbdf.bus;
-    uint8_t slot = dev->sbdf.dev;
-    uint8_t func = dev->sbdf.func;
+    uint16_t control;
 
-    pos = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSIX);
+    pos = pci_find_cap_offset(dev->sbdf, PCI_CAP_ID_MSIX);
     if ( pos )
     {
         control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
@@ -669,13 +666,10 @@  static int msi_capability_init(struct pci_dev *dev,
     struct msi_desc *entry;
     int pos;
     unsigned int i, maxvec, mpos;
-    uint16_t control, seg = dev->sbdf.seg;
-    uint8_t bus = dev->sbdf.bus;
-    uint8_t slot = dev->sbdf.dev;
-    uint8_t func = dev->sbdf.func;
+    uint16_t control;
 
     ASSERT(pcidevs_locked());
-    pos = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSI);
+    pos = pci_find_cap_offset(dev->sbdf, PCI_CAP_ID_MSI);
     if ( !pos )
         return -ENODEV;
     control = pci_conf_read16(dev->sbdf, msi_control_reg(pos));
@@ -752,9 +746,7 @@  static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf)
     if ( vf >= 0 )
     {
         struct pci_dev *pdev = pci_get_pdev(seg, bus, PCI_DEVFN(slot, func));
-        unsigned int pos = pci_find_ext_capability(seg, bus,
-                                                   PCI_DEVFN(slot, func),
-                                                   PCI_EXT_CAP_ID_SRIOV);
+        unsigned int pos = pci_find_ext_capability(sbdf, PCI_EXT_CAP_ID_SRIOV);
         u16 ctrl = pci_conf_read16(sbdf, pos + PCI_SRIOV_CTRL);
         u16 num_vf = pci_conf_read16(sbdf, pos + PCI_SRIOV_NUM_VF);
         u16 offset = pci_conf_read16(sbdf, pos + PCI_SRIOV_VF_OFFSET);
@@ -1096,13 +1088,12 @@  static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc)
     int pos, nr_entries;
     struct pci_dev *pdev;
     u16 control;
-    u8 slot = PCI_SLOT(msi->devfn);
-    u8 func = PCI_FUNC(msi->devfn);
     struct msi_desc *old_desc;
 
     ASSERT(pcidevs_locked());
     pdev = pci_get_pdev(msi->seg, msi->bus, msi->devfn);
-    pos = pci_find_cap_offset(msi->seg, msi->bus, slot, func, PCI_CAP_ID_MSIX);
+    pos = pci_find_cap_offset(PCI_SBDF3_T(msi->seg, msi->bus, msi->devfn),
+                              PCI_CAP_ID_MSIX);
     if ( !pdev || !pos )
         return -ENODEV;
 
@@ -1145,12 +1136,7 @@  static void _pci_cleanup_msix(struct arch_msix *msix)
 static void __pci_disable_msix(struct msi_desc *entry)
 {
     struct pci_dev *dev = entry->dev;
-    uint16_t seg = dev->sbdf.seg;
-    uint8_t bus = dev->sbdf.bus;
-    uint8_t slot = dev->sbdf.dev;
-    uint8_t func = dev->sbdf.func;
-    unsigned int pos = pci_find_cap_offset(seg, bus, slot, func,
-                                           PCI_CAP_ID_MSIX);
+    unsigned int pos = pci_find_cap_offset(dev->sbdf, PCI_CAP_ID_MSIX);
     uint16_t control = pci_conf_read16(dev->sbdf,
                                        msix_control_reg(entry->msi_attrib.pos));
     bool maskall = dev->msix->host_maskall;
@@ -1186,8 +1172,7 @@  int pci_prepare_msix(u16 seg, u8 bus, u8 devfn, bool off)
 {
     int rc;
     struct pci_dev *pdev;
-    u8 slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
-    unsigned int pos = pci_find_cap_offset(seg, bus, slot, func,
+    unsigned int pos = pci_find_cap_offset(PCI_SBDF3_T(seg, bus, devfn),
                                            PCI_CAP_ID_MSIX);
 
     if ( !use_msi )
@@ -1267,10 +1252,6 @@  void pci_cleanup_msi(struct pci_dev *pdev)
 int pci_msi_conf_write_intercept(struct pci_dev *pdev, unsigned int reg,
                                  unsigned int size, uint32_t *data)
 {
-    uint16_t seg = pdev->sbdf.seg;
-    uint8_t bus = pdev->sbdf.bus;
-    uint8_t slot = pdev->sbdf.dev;
-    uint8_t func = pdev->sbdf.func;
     struct msi_desc *entry;
     unsigned int pos;
 
@@ -1278,8 +1259,7 @@  int pci_msi_conf_write_intercept(struct pci_dev *pdev, unsigned int reg,
     {
         entry = find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX);
         pos = entry ? entry->msi_attrib.pos
-                    : pci_find_cap_offset(seg, bus, slot, func,
-                                          PCI_CAP_ID_MSIX);
+                    : pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSIX);
         ASSERT(pos);
 
         if ( reg >= pos && reg < msix_pba_offset_reg(pos) + 4 )
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 4cd4157353..27bd4b935d 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -688,7 +688,8 @@  static unsigned int __init __find_dbgp(u8 bus, u8 slot, u8 func)
     if ( (class >> 8) != PCI_CLASS_SERIAL_USB_EHCI )
         return 0;
 
-    return pci_find_cap_offset(0, bus, slot, func, PCI_CAP_ID_EHCI_DEBUG);
+    return pci_find_cap_offset(PCI_SBDF_T(0, bus, slot, func),
+                               PCI_CAP_ID_EHCI_DEBUG);
 }
 
 static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp,
diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/amd/iommu_detect.c
index 91f5ea6bff..3b36827c14 100644
--- a/xen/drivers/passthrough/amd/iommu_detect.c
+++ b/xen/drivers/passthrough/amd/iommu_detect.c
@@ -30,7 +30,7 @@  static int __init get_iommu_msi_capabilities(
 {
     int pos;
 
-    pos = pci_find_cap_offset(seg, bus, dev, func, PCI_CAP_ID_MSI);
+    pos = pci_find_cap_offset(PCI_SBDF_T(seg, bus, dev, func), PCI_CAP_ID_MSI);
 
     if ( !pos )
         return -ENODEV;
diff --git a/xen/drivers/passthrough/ats.h b/xen/drivers/passthrough/ats.h
index e83a45d16e..3b996c748a 100644
--- a/xen/drivers/passthrough/ats.h
+++ b/xen/drivers/passthrough/ats.h
@@ -31,11 +31,16 @@  static inline int pci_ats_enabled(int seg, int bus, int devfn)
 {
     u32 value;
     int pos;
+    const pci_sbdf_t sbdf = {
+        .seg = seg,
+        .bus = bus,
+        .extfunc = devfn,
+    };
 
-    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
+    pos = pci_find_ext_capability(sbdf, PCI_EXT_CAP_ID_ATS);
     BUG_ON(!pos);
 
-    value = pci_conf_read16(PCI_SBDF3_T(seg, bus, devfn), pos + ATS_REG_CTL);
+    value = pci_conf_read16(sbdf, pos + ATS_REG_CTL);
 
     return value & ATS_ENABLE;
 }
@@ -45,7 +50,8 @@  static inline int pci_ats_device(int seg, int bus, int devfn)
     if ( !ats_enabled )
         return 0;
 
-    return pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
+    return pci_find_ext_capability(PCI_SBDF3_T(seg, bus, devfn),
+                                   PCI_EXT_CAP_ID_ATS);
 }
 
 #endif /* _ATS_H_ */
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index a9667ca21c..cfe2dda733 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -332,8 +332,7 @@  static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
     pdev->domain = NULL;
     INIT_LIST_HEAD(&pdev->msi_list);
 
-    if ( pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                             PCI_CAP_ID_MSIX) )
+    if ( pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSIX) )
     {
         struct arch_msix *msix = xzalloc(struct arch_msix);
 
@@ -371,8 +370,7 @@  static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
             break;
 
         case DEV_TYPE_PCIe_ENDPOINT:
-            pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn),
-                                      PCI_FUNC(devfn), PCI_CAP_ID_EXP);
+            pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP);
             BUG_ON(!pos);
             cap = pci_conf_read16(pdev->sbdf, pos + PCI_EXP_DEVCAP);
             if ( cap & PCI_EXP_DEVCAP_PHANTOM )
@@ -585,13 +583,12 @@  struct pci_dev *pci_get_pdev_by_domain(const struct domain *d, int seg,
 static void pci_enable_acs(struct pci_dev *pdev)
 {
     int pos;
-    uint16_t cap, ctrl, seg = pdev->sbdf.seg;
-    uint8_t bus = pdev->sbdf.bus;
+    uint16_t cap, ctrl;
 
     if ( !iommu_enabled )
         return;
 
-    pos = pci_find_ext_capability(seg, bus, pdev->sbdf.extfunc, PCI_EXT_CAP_ID_ACS);
+    pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_ACS);
     if (!pos)
         return;
 
@@ -722,7 +719,7 @@  int pci_add_device(u16 seg, u8 bus, u8 devfn,
 
     if ( !pdev->info.is_virtfn && !pdev->vf_rlen[0] )
     {
-        unsigned int pos = pci_find_ext_capability(seg, bus, devfn,
+        unsigned int pos = pci_find_ext_capability(pdev->sbdf,
                                                    PCI_EXT_CAP_ID_SRIOV);
         u16 ctrl = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_CTRL);
 
@@ -907,13 +904,13 @@  enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
 {
     u16 class_device, creg;
     u8 d = PCI_SLOT(devfn), f = PCI_FUNC(devfn);
-    int pos = pci_find_cap_offset(seg, bus, d, f, PCI_CAP_ID_EXP);
     const pci_sbdf_t sbdf = {
         .seg = seg,
         .bus = bus,
         .dev = d,
         .func = f,
     };
+    int pos = pci_find_cap_offset(sbdf, PCI_CAP_ID_EXP);
 
     class_device = pci_conf_read16(sbdf, PCI_CLASS_DEVICE);
     switch ( class_device )
@@ -1177,9 +1174,7 @@  static int hest_match_pci(const struct acpi_hest_aer_common *p,
 static bool_t hest_match_type(const struct acpi_hest_header *hest_hdr,
                               const struct pci_dev *pdev)
 {
-    unsigned int pos = pci_find_cap_offset(pdev->sbdf.seg, pdev->sbdf.bus,
-                                           pdev->sbdf.dev, pdev->sbdf.func,
-                                           PCI_CAP_ID_EXP);
+    unsigned int pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP);
     uint8_t pcie = MASK_EXTR(pci_conf_read16(pdev->sbdf, pos + PCI_EXP_FLAGS),
                              PCI_EXP_FLAGS_TYPE);
 
@@ -1249,8 +1244,7 @@  bool_t pcie_aer_get_firmware_first(const struct pci_dev *pdev)
 {
     struct aer_hest_parse_info info = { .pdev = pdev };
 
-    return pci_find_cap_offset(pdev->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                               pdev->sbdf.func, PCI_CAP_ID_EXP) &&
+    return pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP) &&
            apei_hest_parse(aer_hest_parse, &info) >= 0 &&
            info.firmware_first;
 }
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 0302c503fb..289d43f0fe 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -426,8 +426,6 @@  int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
 
 void pci_vtd_quirk(const struct pci_dev *pdev)
 {
-    int seg = pdev->sbdf.seg;
-    int bus = pdev->sbdf.bus;
     int pos;
     bool_t ff;
     u32 val, val2;
@@ -464,12 +462,10 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
     /* Sandybridge-EP (Romley) */
     case 0x3c00: /* host bridge */
     case 0x3c01 ... 0x3c0b: /* root ports */
-        pos = pci_find_ext_capability(seg, bus, pdev->sbdf.extfunc,
-                                      PCI_EXT_CAP_ID_ERR);
+        pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_ERR);
         if ( !pos )
         {
-            pos = pci_find_ext_capability(seg, bus, pdev->sbdf.extfunc,
-                                          PCI_EXT_CAP_ID_VNDR);
+            pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_VNDR);
             while ( pos )
             {
                 val = pci_conf_read32(pdev->sbdf, pos + PCI_VNDR_HEADER);
@@ -478,8 +474,8 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
                     pos += PCI_VNDR_HEADER;
                     break;
                 }
-                pos = pci_find_next_ext_capability(seg, bus, pdev->sbdf.extfunc,
-                                                   pos, PCI_EXT_CAP_ID_VNDR);
+                pos = pci_find_next_ext_capability(pdev->sbdf, pos,
+                                                   PCI_EXT_CAP_ID_VNDR);
             }
             ff = 0;
         }
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c
index e0904df5b6..260b06a856 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/x86/ats.c
@@ -57,8 +57,7 @@  int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd)
         return 0;
 
     ats_drhd = find_ats_dev_drhd(drhd->iommu);
-    pos = pci_find_ext_capability(pdev->sbdf.seg, pdev->sbdf.bus,
-                                  pdev->sbdf.extfunc, PCI_EXT_CAP_ID_ATS);
+    pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_ATS);
 
     if ( pos && (ats_drhd == NULL) )
     {
diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index cfd610229d..bfbd50100a 100644
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -23,11 +23,9 @@  boolean_param("ats", ats_enabled);
 int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
 {
     u32 value;
-    uint16_t seg = pdev->sbdf.seg;
-    uint16_t bus = pdev->sbdf.bus, devfn = pdev->sbdf.extfunc;
     int pos;
 
-    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
+    pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_ATS);
     BUG_ON(!pos);
 
     if ( iommu_verbose )
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index 3d2acf6f77..e902a0aace 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -8,18 +8,12 @@ 
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
 
-int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
+int pci_find_cap_offset(pci_sbdf_t sbdf, unsigned int cap)
 {
     u8 id;
     int max_cap = 48;
     u8 pos = PCI_CAPABILITY_LIST;
     u16 status;
-    const pci_sbdf_t sbdf = {
-        .seg = seg,
-        .bus = bus,
-        .dev = dev,
-        .func = func,
-    };
 
     status = pci_conf_read16(sbdf, PCI_STATUS);
     if ( (status & PCI_STATUS_CAP_LIST) == 0 )
@@ -45,15 +39,10 @@  int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap)
     return 0;
 }
 
-int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap)
+int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos, unsigned int cap)
 {
     u8 id;
     int ttl = 48;
-    const pci_sbdf_t sbdf = {
-        .seg = seg,
-        .bus = bus,
-        .extfunc = devfn,
-    };
 
     while ( ttl-- )
     {
@@ -83,9 +72,9 @@  int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap)
  * within the device's PCI configuration space or 0 if the device does
  * not support it.
  */
-int pci_find_ext_capability(int seg, int bus, int devfn, int cap)
+int pci_find_ext_capability(pci_sbdf_t sbdf, unsigned int cap)
 {
-    return pci_find_next_ext_capability(seg, bus, devfn, 0, cap);
+    return pci_find_next_ext_capability(sbdf, 0, cap);
 }
 
 /**
@@ -98,15 +87,11 @@  int pci_find_ext_capability(int seg, int bus, int devfn, int cap)
  * within the device's PCI configuration space or 0 if the device does
  * not support it.
  */
-int pci_find_next_ext_capability(int seg, int bus, int devfn, int start, int cap)
+int pci_find_next_ext_capability(pci_sbdf_t sbdf, unsigned int start,
+                                 unsigned int cap)
 {
-    int ttl = 480; /* 3840 bytes, minimum 8 bytes per capability */
-    int pos = max(start, 0x100);
-    const pci_sbdf_t sbdf = {
-        .seg = seg,
-        .bus = bus,
-        .extfunc = devfn,
-    };
+    unsigned int ttl = 480; /* 3840 bytes, minimum 8 bytes per capability */
+    unsigned int pos = max(start, 0x100u);
     uint32_t header = pci_conf_read32(sbdf, pos);
 
     /*
diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
index 2bdae48edf..2a366828a1 100644
--- a/xen/drivers/vpci/msi.c
+++ b/xen/drivers/vpci/msi.c
@@ -185,9 +185,7 @@  static void mask_write(const struct pci_dev *pdev, unsigned int reg,
 
 static int init_msi(struct pci_dev *pdev)
 {
-    unsigned int pos = pci_find_cap_offset(pdev->sbdf.seg, pdev->sbdf.bus,
-                                           pdev->sbdf.dev, pdev->sbdf.func,
-                                           PCI_CAP_ID_MSI);
+    unsigned int pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSI);
     uint16_t control;
     int ret;
 
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 1006b01f4b..44dbb086fd 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -445,9 +445,7 @@  static int init_msix(struct pci_dev *pdev)
     uint16_t control;
     int rc;
 
-    msix_offset = pci_find_cap_offset(pdev->sbdf.seg, pdev->sbdf.bus,
-                                      pdev->sbdf.dev, pdev->sbdf.func,
-                                      PCI_CAP_ID_MSIX);
+    msix_offset = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSIX);
     if ( !msix_offset )
         return 0;
 
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 5e345c417f..937465f5b5 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -178,10 +178,11 @@  int pci_mmcfg_read(unsigned int seg, unsigned int bus,
                    unsigned int devfn, int reg, int len, u32 *value);
 int pci_mmcfg_write(unsigned int seg, unsigned int bus,
                     unsigned int devfn, int reg, int len, u32 value);
-int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap);
-int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap);
-int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
-int pci_find_next_ext_capability(int seg, int bus, int devfn, int pos, int cap);
+int pci_find_cap_offset(pci_sbdf_t sbdf, unsigned int cap);
+int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos, unsigned int cap);
+int pci_find_ext_capability(pci_sbdf_t sbdf, unsigned int cap);
+int pci_find_next_ext_capability(pci_sbdf_t sbdf, unsigned int pos,
+                                 unsigned int cap);
 const char *parse_pci(const char *, unsigned int *seg, unsigned int *bus,
                       unsigned int *dev, unsigned int *func);
 const char *parse_pci_seg(const char *, unsigned int *seg, unsigned int *bus,