Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t
@ 2019-08-22  6:51 Roger Pau Monne
  2019-08-26  8:42 ` Roger Pau Monné
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Roger Pau Monne @ 2019-08-22  6:51 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

The new format specifier is '%pp', and prints a pci_sbdf_t using the
seg:bus:dev.func format. Replace all SBDFs printed using
'%04x:%02x:%02x.%u' to use the new format specifier.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Use base 8 to print the function number.
 - Sort the addition in the pointer function alphabetically.
---
 docs/misc/printk-formats.txt                |   5 +
 xen/arch/x86/hvm/vmsi.c                     |  10 +-
 xen/arch/x86/msi.c                          |  35 +++---
 xen/common/vsprintf.c                       |  18 ++++
 xen/drivers/passthrough/amd/iommu_acpi.c    |  17 ++-
 xen/drivers/passthrough/amd/iommu_cmd.c     |   5 +-
 xen/drivers/passthrough/amd/iommu_detect.c  |   5 +-
 xen/drivers/passthrough/amd/iommu_init.c    |  12 +--
 xen/drivers/passthrough/amd/iommu_intr.c    |   3 +-
 xen/drivers/passthrough/amd/pci_amd_iommu.c |  25 ++---
 xen/drivers/passthrough/pci.c               | 114 ++++++++------------
 xen/drivers/passthrough/vtd/dmar.c          |  25 +++--
 xen/drivers/passthrough/vtd/intremap.c      |  11 +-
 xen/drivers/passthrough/vtd/iommu.c         |  80 ++++++--------
 xen/drivers/passthrough/vtd/quirks.c        |  22 ++--
 xen/drivers/passthrough/vtd/utils.c         |   6 +-
 xen/drivers/passthrough/x86/ats.c           |  13 +--
 xen/drivers/vpci/header.c                   |  11 +-
 xen/drivers/vpci/msi.c                      |   6 +-
 xen/drivers/vpci/msix.c                     |  24 ++---
 20 files changed, 189 insertions(+), 258 deletions(-)

diff --git a/docs/misc/printk-formats.txt b/docs/misc/printk-formats.txt
index 080f498f65..8f666f696a 100644
--- a/docs/misc/printk-formats.txt
+++ b/docs/misc/printk-formats.txt
@@ -48,3 +48,8 @@ Domain and vCPU information:
                The domain part as above, with the vcpu_id printed in decimal.
                  e.g.  d0v1
                        d[IDLE]v0
+
+PCI:
+
+       %pp     PCI device address in S:B:D.F format from a pci_sbdf_t.
+                 e.g.  0004:02:00.0
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 6597d9f719..c9cd1c26c6 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -693,10 +693,8 @@ static int vpci_msi_update(const struct pci_dev *pdev, uint32_t data,
 
         if ( rc )
         {
-            gdprintk(XENLOG_ERR,
-                     "%04x:%02x:%02x.%u: failed to bind PIRQ %u: %d\n",
-                     pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                     PCI_FUNC(pdev->devfn), pirq + i, rc);
+            gdprintk(XENLOG_ERR, "%pp: failed to bind PIRQ %u: %d\n",
+                     &pdev->sbdf, pirq + i, rc);
             while ( bind.machine_irq-- > pirq )
                 pt_irq_destroy_bind(pdev->domain, &bind);
             return rc;
@@ -750,9 +748,7 @@ static int vpci_msi_enable(const struct pci_dev *pdev, uint32_t data,
                                    &msi_info);
     if ( rc )
     {
-        gdprintk(XENLOG_ERR, "%04x:%02x:%02x.%u: failed to map PIRQ: %d\n",
-                 pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                 PCI_FUNC(pdev->devfn), rc);
+        gdprintk(XENLOG_ERR, "%pp: failed to map PIRQ: %d\n", &pdev->sbdf, rc);
         return rc;
     }
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index d6306005a9..14d86f9195 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -428,8 +428,8 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
             {
                 pdev->msix->warned = domid;
                 printk(XENLOG_G_WARNING
-                       "cannot mask IRQ %d: masking MSI-X on Dom%d's %04x:%02x:%02x.%u\n",
-                       desc->irq, domid, seg, bus, slot, func);
+                       "cannot mask IRQ %d: masking MSI-X on Dom%d's %pp\n",
+                       desc->irq, domid, &pdev->sbdf);
             }
         }
         pdev->msix->host_maskall = maskall;
@@ -987,11 +987,11 @@ static int msix_capability_init(struct pci_dev *dev,
             struct domain *d = dev->domain ?: currd;
 
             if ( !is_hardware_domain(currd) || d != currd )
-                printk("%s use of MSI-X on %04x:%02x:%02x.%u by Dom%d\n",
+                printk("%s use of MSI-X on %pp by %pd\n",
                        is_hardware_domain(currd)
                        ? XENLOG_WARNING "Potentially insecure"
                        : XENLOG_ERR "Insecure",
-                       seg, bus, slot, func, d->domain_id);
+                       &dev->sbdf, d);
             if ( !is_hardware_domain(d) &&
                  /* Assume a domain without memory has no mappings yet. */
                  (!is_hardware_domain(currd) || d->tot_pages) )
@@ -1046,18 +1046,15 @@ static int __pci_enable_msi(struct msi_info *msi, struct msi_desc **desc)
     old_desc = find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSI);
     if ( old_desc )
     {
-        printk(XENLOG_ERR "irq %d already mapped to MSI on %04x:%02x:%02x.%u\n",
-               msi->irq, msi->seg, msi->bus,
-               PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
+        printk(XENLOG_ERR "irq %d already mapped to MSI on %pp\n",
+               msi->irq, &pdev->sbdf);
         return -EEXIST;
     }
 
     old_desc = find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX);
     if ( old_desc )
     {
-        printk(XENLOG_WARNING "MSI-X already in use on %04x:%02x:%02x.%u\n",
-               msi->seg, msi->bus,
-               PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
+        printk(XENLOG_WARNING "MSI-X already in use on %pp\n", &pdev->sbdf);
         __pci_disable_msix(old_desc);
     }
 
@@ -1114,16 +1111,15 @@ static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc)
     old_desc = find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSIX);
     if ( old_desc )
     {
-        printk(XENLOG_ERR "irq %d already mapped to MSI-X on %04x:%02x:%02x.%u\n",
-               msi->irq, msi->seg, msi->bus, slot, func);
+        printk(XENLOG_ERR "irq %d already mapped to MSI-X on %pp\n",
+               msi->irq, &pdev->sbdf);
         return -EEXIST;
     }
 
     old_desc = find_msi_entry(pdev, -1, PCI_CAP_ID_MSI);
     if ( old_desc )
     {
-        printk(XENLOG_WARNING "MSI already in use on %04x:%02x:%02x.%u\n",
-               msi->seg, msi->bus, slot, func);
+        printk(XENLOG_WARNING "MSI already in use on %pp\n", &pdev->sbdf);
         __pci_disable_msi(old_desc);
     }
 
@@ -1170,9 +1166,8 @@ static void __pci_disable_msix(struct msi_desc *entry)
         writel(1, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
     else if ( !(control & PCI_MSIX_FLAGS_MASKALL) )
     {
-        printk(XENLOG_WARNING
-               "cannot disable IRQ %d: masking MSI-X on %04x:%02x:%02x.%u\n",
-               entry->irq, seg, bus, slot, func);
+        printk(XENLOG_WARNING "cannot disable IRQ %d: masking MSI-X on %pp\n",
+               entry->irq, &dev->sbdf);
         maskall = true;
     }
     dev->msix->host_maskall = maskall;
@@ -1340,7 +1335,6 @@ int pci_restore_msi_state(struct pci_dev *pdev)
     struct msi_desc *entry, *tmp;
     struct irq_desc *desc;
     struct msi_msg msg;
-    u8 slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     unsigned int type = 0, pos = 0;
     u16 control = 0;
 
@@ -1369,9 +1363,8 @@ int pci_restore_msi_state(struct pci_dev *pdev)
         if (desc->msi_desc != entry)
         {
     bogus:
-            dprintk(XENLOG_ERR,
-                    "Restore MSI for %04x:%02x:%02x:%u entry %u not set?\n",
-                    pdev->seg, pdev->bus, slot, func, i);
+            dprintk(XENLOG_ERR, "Restore MSI for %pp entry %u not set?\n",
+                    &pdev->sbdf, i);
             spin_unlock_irqrestore(&desc->lock, flags);
             if ( type == PCI_CAP_ID_MSIX )
                 pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 183d3ed4bb..185a4bd561 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -394,6 +394,20 @@ static char *print_vcpu(char *str, const char *end, const struct vcpu *v)
     return number(str + 1, end, v->vcpu_id, 10, -1, -1, 0);
 }
 
+static char *print_pci_addr(char *str, const char *end, const pci_sbdf_t *sbdf)
+{
+    str = number(str, end, sbdf->seg, 16, 4, -1, ZEROPAD);
+    if ( str < end )
+        *str = ':';
+    str = number(str + 1, end, sbdf->bus, 16, 2, -1, ZEROPAD);
+    if ( str < end )
+        *str = ':';
+    str = number(str + 1, end, sbdf->dev, 16, 2, -1, ZEROPAD);
+    if ( str < end )
+        *str = '.';
+    return number(str + 1, end, sbdf->fn, 8, -1, -1, 0);
+}
+
 static char *pointer(char *str, const char *end, const char **fmt_ptr,
                      const void *arg, int field_width, int precision,
                      int flags)
@@ -476,6 +490,10 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
         }
     }
 
+    case 'p': /* PCI SBDF. */
+        ++*fmt_ptr;
+        return print_pci_addr(str, end, arg);
+
     case 's': /* Symbol name with offset and size (iff offset != 0) */
     case 'S': /* Symbol name unconditionally with offset and size */
     {
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 668a9805ef..947a24d719 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -724,9 +724,8 @@ static u16 __init parse_ivhd_device_special(
         return 0;
     }
 
-    AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle %#x\n",
-                    seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
-                    special->variety, special->handle);
+    AMD_IOMMU_DEBUG("IVHD Special: %pp variety %#x handle %#x\n",
+                    &PCI_SBDF2(seg, bdf), special->variety, special->handle);
     add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);
 
     switch ( special->variety )
@@ -749,9 +748,9 @@ static u16 __init parse_ivhd_device_special(
         if ( idx < nr_ioapic_sbdf )
         {
             AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x"
-                            "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
-                            ioapic_sbdf[idx].id, special->handle, seg,
-                            PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+                            "(IVRS: %#x devID %pp)\n",
+                            ioapic_sbdf[idx].id, special->handle,
+                            &PCI_SBDF2(seg, bdf));
             break;
         }
 
@@ -821,9 +820,9 @@ static u16 __init parse_ivhd_device_special(
             break;
         case HPET_CMDL:
             AMD_IOMMU_DEBUG("IVHD: Command line override present for HPET %#x "
-                            "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
-                            hpet_sbdf.id, special->handle, seg, PCI_BUS(bdf),
-                            PCI_SLOT(bdf), PCI_FUNC(bdf));
+                            "(IVRS: %#x devID %pp)\n",
+                            hpet_sbdf.id, special->handle,
+                            &PCI_SBDF2(seg, bdf));
             break;
         case HPET_NONE:
             /* set device id of hpet */
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
index af3a1fb865..3eeb214ba3 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -296,9 +296,8 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
 
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("%s: Can't find iommu for %04x:%02x:%02x.%u\n",
-                        __func__, pdev->seg, pdev->bus,
-                        PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+        AMD_IOMMU_DEBUG("%s: Can't find iommu for %pp\n",
+                        __func__, &pdev->sbdf);
         return;
     }
 
diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/amd/iommu_detect.c
index d782e66eee..21a5275d6d 100644
--- a/xen/drivers/passthrough/amd/iommu_detect.c
+++ b/xen/drivers/passthrough/amd/iommu_detect.c
@@ -185,9 +185,8 @@ int __init amd_iommu_detect_one_acpi(
 
     rt = pci_ro_device(iommu->seg, bus, PCI_DEVFN(dev, func));
     if ( rt )
-        printk(XENLOG_ERR
-               "Could not mark config space of %04x:%02x:%02x.%u read-only (%d)\n",
-               iommu->seg, bus, dev, func, rt);
+        printk(XENLOG_ERR "Could not mark config space of %pp read-only (%d)\n",
+               &PCI_SBDF2(iommu->seg, iommu->bdf), rt);
 
     list_add_tail(&iommu->list, &amd_iommu_head);
     rt = 0;
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index bb5a3e57c9..634c167982 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -778,9 +778,8 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
     pcidevs_unlock();
     if ( !iommu->msi.dev )
     {
-        AMD_IOMMU_DEBUG("IOMMU: no pdev for %04x:%02x:%02x.%u\n",
-                        iommu->seg, PCI_BUS(iommu->bdf),
-                        PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf));
+        AMD_IOMMU_DEBUG("IOMMU: no pdev for %pp\n",
+                        &PCI_SBDF2(iommu->seg, iommu->bdf));
         return 0;
     }
 
@@ -866,9 +865,6 @@ __initcall(iov_adjust_irq_affinities);
 static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
 {
     u32 value;
-    u8 bus = PCI_BUS(iommu->bdf);
-    u8 dev = PCI_SLOT(iommu->bdf);
-    u8 func = PCI_FUNC(iommu->bdf);
 
     if ( (boot_cpu_data.x86 != 0x15) ||
          (boot_cpu_data.x86_model < 0x10) ||
@@ -886,8 +882,8 @@ static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
 
     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);
+           "AMD-Vi: Applying erratum 746 workaround for IOMMU at %pp\n",
+           &PCI_SBDF2(iommu->seg, iommu->bdf));
 
     /* Clear the enable writing bit */
     pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf0, 0x90);
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 0164ceac3b..687a7fa922 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -685,8 +685,7 @@ static struct amd_iommu *_find_iommu_for_device(int seg, int bdf)
     if ( iommu )
         return iommu;
 
-    AMD_IOMMU_DEBUG("No IOMMU for MSI dev = %04x:%02x:%02x.%u\n",
-                    seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+    AMD_IOMMU_DEBUG("No IOMMU for MSI dev = %pp\n", &PCI_SBDF2(seg, bdf));
     return ERR_PTR(-EINVAL);
 }
 
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 8d4a5fbc37..ae20e9b6e5 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -52,9 +52,8 @@ struct amd_iommu *find_iommu_for_device(int seg, int bdf)
                 tmp.dte_requestor_id = bdf;
             ivrs_mappings[bdf] = tmp;
 
-            printk(XENLOG_WARNING "%04x:%02x:%02x.%u not found in ACPI tables;"
-                   " using same IOMMU as function 0\n",
-                   seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+            printk(XENLOG_WARNING "%pp not found in ACPI tables;"
+                   " using same IOMMU as function 0\n", &PCI_SBDF2(seg, bdf));
 
             /* write iommu field last */
             ivrs_mappings[bdf].iommu = ivrs_mappings[bd0].iommu;
@@ -337,9 +336,8 @@ static int reassign_device(struct domain *source, struct domain *target,
         return rc;
 
     amd_iommu_setup_domain_device(target, iommu, devfn, pdev);
-    AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
-                    pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                    source->domain_id, target->domain_id);
+    AMD_IOMMU_DEBUG("Re-assign %pp from dom%d to dom%d\n",
+                    &pdev->sbdf, source->domain_id, target->domain_id);
 
     return 0;
 }
@@ -446,15 +444,12 @@ static int amd_iommu_add_device(u8 devfn, struct pci_dev *pdev)
         if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE &&
              is_hardware_domain(pdev->domain) )
         {
-            AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
-                            pdev->seg, pdev->bus, PCI_SLOT(devfn),
-                            PCI_FUNC(devfn));
+            AMD_IOMMU_DEBUG("Skipping host bridge %pp\n", &pdev->sbdf);
             return 0;
         }
 
-        AMD_IOMMU_DEBUG("No iommu for %04x:%02x:%02x.%u; cannot be handed to d%d\n",
-                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                        pdev->domain->domain_id);
+        AMD_IOMMU_DEBUG("No iommu for %pp; cannot be handed to d%d\n",
+                        &pdev->sbdf, pdev->domain->domain_id);
         return -ENODEV;
     }
 
@@ -473,10 +468,8 @@ static int amd_iommu_remove_device(u8 devfn, struct pci_dev *pdev)
     iommu = find_iommu_for_device(pdev->seg, bdf);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("Fail to find iommu."
-                        " %04x:%02x:%02x.%u cannot be removed from dom%d\n",
-                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                        pdev->domain->domain_id);
+        AMD_IOMMU_DEBUG("Fail to find iommu. %pp cannot be removed from %pd\n",
+                        &pdev->sbdf, pdev->domain);
         return -ENODEV;
     }
 
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 7c196ba58b..bb916d4294 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -238,11 +238,7 @@ static void check_pdev(const struct pci_dev *pdev)
     (PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT | \
      PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT | \
      PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY)
-    u16 seg = pdev->seg;
-    u8 bus = pdev->bus;
-    u8 dev = PCI_SLOT(pdev->devfn);
-    u8 func = PCI_FUNC(pdev->devfn);
-    u16 val;
+     u16 val;
 
     if ( command_mask )
     {
@@ -252,8 +248,8 @@ static void check_pdev(const struct pci_dev *pdev)
         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);
+            printk(XENLOG_INFO "%pp status %04x -> %04x\n",
+                   &pdev->sbdf, val, val & ~PCI_STATUS_CHECK);
             pci_conf_write16(pdev->sbdf, PCI_STATUS, val & PCI_STATUS_CHECK);
         }
     }
@@ -270,9 +266,8 @@ static void check_pdev(const struct pci_dev *pdev)
         val = pci_conf_read16(pdev->sbdf, PCI_SEC_STATUS);
         if ( val & PCI_STATUS_CHECK )
         {
-            printk(XENLOG_INFO
-                   "%04x:%02x:%02x.%u secondary status %04x -> %04x\n",
-                   seg, bus, dev, func, val, val & ~PCI_STATUS_CHECK);
+            printk(XENLOG_INFO "%pp secondary status %04x -> %04x\n",
+                   &pdev->sbdf, val, val & ~PCI_STATUS_CHECK);
             pci_conf_write16(pdev->sbdf, PCI_SEC_STATUS,
                              val & PCI_STATUS_CHECK);
         }
@@ -411,8 +406,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
             break;
 
         default:
-            printk(XENLOG_WARNING "%04x:%02x:%02x.%u: unknown type %d\n",
-                   pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pdev->type);
+            printk(XENLOG_WARNING "%pp: unknown type %d\n",
+                   &pdev->sbdf, pdev->type);
             break;
     }
 
@@ -644,9 +639,9 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
         if ( flags & PCI_BAR_LAST )
         {
             printk(XENLOG_WARNING
-                   "%sdevice %04x:%02x:%02x.%u with 64-bit %sBAR in last slot\n",
-                   (flags & PCI_BAR_VF) ? "SR-IOV " : "", sbdf.seg, sbdf.bus,
-                   sbdf.dev, sbdf.fn, (flags & PCI_BAR_VF) ? "vf " : "");
+                   "%sdevice %pp with 64-bit %sBAR in last slot\n",
+                   (flags & PCI_BAR_VF) ? "SR-IOV " : "", &sbdf,
+                   (flags & PCI_BAR_VF) ? "vf " : "");
             *psize = 0;
             return 1;
         }
@@ -750,9 +745,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
                      PCI_BASE_ADDRESS_SPACE_IO )
                 {
                     printk(XENLOG_WARNING
-                           "SR-IOV device %04x:%02x:%02x.%u with vf BAR%u"
-                           " in IO space\n",
-                           seg, bus, slot, func, i);
+                           "SR-IOV device %pp with vf BAR%u in IO space\n",
+                           &pdev->sbdf, i);
                     continue;
                 }
                 ret = pci_size_mem_bar(sbdf, idx, NULL, &pdev->vf_rlen[i],
@@ -764,10 +758,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
             }
         }
         else
-            printk(XENLOG_WARNING
-                   "SR-IOV device %04x:%02x:%02x.%u has its virtual"
-                   " functions already enabled (%04x)\n",
-                   seg, bus, slot, func, ctrl);
+            printk(XENLOG_WARNING "SR-IOV device %pp has its virtual"
+                   " functions already enabled (%04x)\n", &pdev->sbdf, ctrl);
     }
 
     check_pdev(pdev);
@@ -794,15 +786,14 @@ out:
     pcidevs_unlock();
     if ( !ret )
     {
-        printk(XENLOG_DEBUG "PCI add %s %04x:%02x:%02x.%u\n", pdev_type,
-               seg, bus, slot, func);
+        printk(XENLOG_DEBUG "PCI add %s %pp\n", pdev_type,  &pdev->sbdf);
         while ( pdev->phantom_stride )
         {
             func += pdev->phantom_stride;
             if ( PCI_SLOT(func) )
                 break;
-            printk(XENLOG_DEBUG "PCI phantom %04x:%02x:%02x.%u\n",
-                   seg, bus, slot, func);
+            printk(XENLOG_DEBUG "PCI phantom %pp\n",
+                   &PCI_SBDF(seg, bus, slot, func));
         }
     }
     return ret;
@@ -831,9 +822,8 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
             if ( pdev->domain )
                 list_del(&pdev->domain_list);
             pci_cleanup_msi(pdev);
+            printk(XENLOG_DEBUG "PCI remove device %pp\n", &pdev->sbdf);
             free_pdev(pseg, pdev);
-            printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
             break;
         }
 
@@ -907,9 +897,8 @@ int pci_release_devices(struct domain *d)
         bus = pdev->bus;
         devfn = pdev->devfn;
         if ( deassign_device(d, pdev->seg, bus, devfn) )
-            printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!\n",
-                   d->domain_id, pdev->seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk("domain %d: deassign device (%pp) failed!\n",
+                   d->domain_id, &pdev->sbdf);
     }
     pcidevs_unlock();
 
@@ -1056,8 +1045,8 @@ static int __init _scan_pci_devices(struct pci_seg *pseg, void *arg)
                 pdev = alloc_pdev(pseg, bus, PCI_DEVFN(dev, func));
                 if ( !pdev )
                 {
-                    printk(XENLOG_WARNING "%04x:%02x:%02x.%u: alloc_pdev failed\n",
-                           pseg->nr, bus, dev, func);
+                    printk(XENLOG_WARNING "%pp: alloc_pdev failed\n",
+                           &PCI_SBDF(pseg->nr, bus, dev, func));
                     return -ENOMEM;
                 }
 
@@ -1098,9 +1087,8 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
         err = ctxt->handler(devfn, pdev);
         if ( err )
         {
-            printk(XENLOG_ERR "setup %04x:%02x:%02x.%u for d%d failed (%d)\n",
-                   pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                   ctxt->d->domain_id, err);
+            printk(XENLOG_ERR "setup %pp for d%d failed (%d)\n",
+                   &pdev->sbdf, ctxt->d->domain_id, err);
             if ( devfn == pdev->devfn )
                 return;
         }
@@ -1141,9 +1129,8 @@ static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void *arg
                 pdev->domain = dom_xen;
             }
             else if ( pdev->domain != ctxt->d )
-                printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
-                       pdev->domain->domain_id, pseg->nr, bus,
-                       PCI_SLOT(devfn), PCI_FUNC(devfn));
+                printk(XENLOG_WARNING "Dom%d owning %pp?\n",
+                       pdev->domain->domain_id, &pdev->sbdf);
 
             if ( iommu_verbose )
             {
@@ -1279,10 +1266,8 @@ static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
 
     list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
     {
-        printk("%04x:%02x:%02x.%u - dom %-3d - node %-3d - MSIs < ",
-               pseg->nr, pdev->bus,
-               PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
-               pdev->domain ? pdev->domain->domain_id : -1,
+        printk("%pp - dom %-3d - node %-3d - MSIs < ",
+               &pdev->sbdf, pdev->domain ? pdev->domain->domain_id : -1,
                (pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
         list_for_each_entry ( msi, &pdev->msi_list, list )
                printk("%d ", msi->irq);
@@ -1347,8 +1332,8 @@ static int iommu_add_device(struct pci_dev *pdev)
             return 0;
         rc = hd->platform_ops->add_device(devfn, pci_to_dev(pdev));
         if ( rc )
-            printk(XENLOG_WARNING "IOMMU: add %04x:%02x:%02x.%u failed (%d)\n",
-                   pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc);
+            printk(XENLOG_WARNING "IOMMU: add %pp failed (%d)\n",
+                   &pdev->sbdf, rc);
     }
 }
 
@@ -1392,8 +1377,7 @@ static int iommu_remove_device(struct pci_dev *pdev)
         if ( !rc )
             continue;
 
-        printk(XENLOG_ERR "IOMMU: remove %04x:%02x:%02x.%u failed (%d)\n",
-               pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc);
+        printk(XENLOG_ERR "IOMMU: remove %pp failed (%d)\n", &pdev->sbdf, rc);
         return rc;
     }
 
@@ -1463,9 +1447,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
             break;
         rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), flag);
         if ( rc )
-            printk(XENLOG_G_WARNING "d%d: assign %04x:%02x:%02x.%u failed (%d)\n",
-                   d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                   rc);
+            printk(XENLOG_G_WARNING "d%d: assign %pp failed (%d)\n",
+                   d->domain_id, &PCI_SBDF3(seg, bus, devfn), rc);
     }
 
  done:
@@ -1501,8 +1484,8 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
         if ( !ret )
             continue;
 
-        printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n",
-               d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
+        printk(XENLOG_G_ERR "d%d: deassign %pp failed (%d)\n",
+               d->domain_id, &PCI_SBDF3(seg, bus, devfn), ret);
         return ret;
     }
 
@@ -1511,9 +1494,8 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
                                             pci_to_dev(pdev));
     if ( ret )
     {
-        dprintk(XENLOG_G_ERR,
-                "d%d: deassign device (%04x:%02x:%02x.%u) failed\n",
-                d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_G_ERR, "d%d: deassign device (%pp) failed\n",
+                d->domain_id, &pdev->sbdf);
         return ret;
     }
 
@@ -1590,10 +1572,8 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev)
     _pci_hide_device(pdev);
 
     if ( !d->is_shutting_down && printk_ratelimit() )
-        printk(XENLOG_ERR
-               "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
-               d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-               PCI_FUNC(pdev->devfn));
+        printk(XENLOG_ERR "dom%d: ATS device %pp flush failed\n",
+               d->domain_id, &pdev->sbdf);
     if ( !is_hardware_domain(d) )
         domain_crash(d);
 
@@ -1682,9 +1662,8 @@ int iommu_do_pci_domctl(
         {
             if ( ret )
             {
-                printk(XENLOG_G_INFO
-                       "%04x:%02x:%02x.%u already assigned, or non-existent\n",
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+                printk(XENLOG_G_INFO "%pp already assigned, or non-existent\n",
+                       &PCI_SBDF3(seg, bus, devfn));
                 ret = -EINVAL;
             }
             break;
@@ -1696,9 +1675,8 @@ int iommu_do_pci_domctl(
                                                 "h", u_domctl);
         else if ( ret )
             printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: "
-                   "assign %04x:%02x:%02x.%u to dom%d failed (%d)\n",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                   d->domain_id, ret);
+                   "assign %pp to dom%d failed (%d)\n",
+                   &PCI_SBDF3(seg, bus, devfn), d->domain_id, ret);
 
         break;
 
@@ -1732,10 +1710,8 @@ int iommu_do_pci_domctl(
         ret = deassign_device(d, seg, bus, devfn);
         pcidevs_unlock();
         if ( ret )
-            printk(XENLOG_G_ERR
-                   "deassign %04x:%02x:%02x.%u from dom%d failed (%d)\n",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                   d->domain_id, ret);
+            printk(XENLOG_G_ERR "deassign %pp from dom%d failed (%d)\n",
+                   &PCI_SBDF3(seg, bus, devfn), d->domain_id, ret);
 
         break;
 
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 9c94deac0b..4c0d2f6672 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -361,9 +361,8 @@ static int __init acpi_parse_dev_scope(
             sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
                                      PCI_SUBORDINATE_BUS);
             if ( iommu_verbose )
-                printk(VTDPREFIX
-                       " bridge: %04x:%02x:%02x.%u start=%x sec=%x sub=%x\n",
-                       seg, bus, path->dev, path->fn,
+                printk(VTDPREFIX " bridge: %pp start=%x sec=%x sub=%x\n",
+                       &PCI_SBDF(seg, bus, path->dev, path->fn),
                        acpi_scope->bus, sec_bus, sub_bus);
 
             dmar_scope_add_buses(scope, sec_bus, sub_bus);
@@ -371,8 +370,8 @@ static int __init acpi_parse_dev_scope(
 
         case ACPI_DMAR_SCOPE_TYPE_HPET:
             if ( iommu_verbose )
-                printk(VTDPREFIX " MSI HPET: %04x:%02x:%02x.%u\n",
-                       seg, bus, path->dev, path->fn);
+                printk(VTDPREFIX " MSI HPET: %pp\n",
+                       &PCI_SBDF(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -393,8 +392,8 @@ static int __init acpi_parse_dev_scope(
 
         case ACPI_DMAR_SCOPE_TYPE_ENDPOINT:
             if ( iommu_verbose )
-                printk(VTDPREFIX " endpoint: %04x:%02x:%02x.%u\n",
-                       seg, bus, path->dev, path->fn);
+                printk(VTDPREFIX " endpoint: %pp\n",
+                       &PCI_SBDF(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -407,8 +406,8 @@ static int __init acpi_parse_dev_scope(
 
         case ACPI_DMAR_SCOPE_TYPE_IOAPIC:
             if ( iommu_verbose )
-                printk(VTDPREFIX " IOAPIC: %04x:%02x:%02x.%u\n",
-                       seg, bus, path->dev, path->fn);
+                printk(VTDPREFIX " IOAPIC: %pp\n",
+                       &PCI_SBDF(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -537,8 +536,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
 
             if ( !pci_device_detect(drhd->segment, b, d, f) )
                 printk(XENLOG_WARNING VTDPREFIX
-                       " Non-existent device (%04x:%02x:%02x.%u) in this DRHD's scope!\n",
-                       drhd->segment, b, d, f);
+                       " Non-existent device (%pp) in this DRHD's scope!\n",
+                       &PCI_SBDF(drhd->segment, b, d, f));
         }
 
         acpi_register_drhd_unit(dmaru);
@@ -574,9 +573,9 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
         if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
         {
             dprintk(XENLOG_WARNING VTDPREFIX,
-                    " Non-existent device (%04x:%02x:%02x.%u) is reported"
+                    " Non-existent device (%pp) is reported"
                     " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
-                    rmrru->segment, b, d, f,
+                    &PCI_SBDF(rmrru->segment, b, d, f),
                     rmrru->base_address, rmrru->end_address);
             ignore = true;
         }
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index df0e8ac5cb..9b86fa8bb7 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -524,16 +524,13 @@ static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire)
         }
         else
             dprintk(XENLOG_WARNING VTDPREFIX,
-                    "d%d: no upstream bridge for %04x:%02x:%02x.%u\n",
-                    pdev->domain->domain_id,
-                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+                    "d%d: no upstream bridge for %pp\n",
+                    pdev->domain->domain_id, &pdev->sbdf);
         break;
 
     default:
-        dprintk(XENLOG_WARNING VTDPREFIX,
-                "d%d: unknown(%u): %04x:%02x:%02x.%u\n",
-                pdev->domain->domain_id, pdev->type,
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_WARNING VTDPREFIX, "d%d: unknown(%u): %pp\n",
+                pdev->domain->domain_id, pdev->type, &pdev->sbdf);
         break;
    }
 }
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 5d72270c5b..cfaee1503f 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -882,27 +882,24 @@ static int iommu_page_fault_do_one(struct iommu *iommu, int type,
     {
     case DMA_REMAP:
         printk(XENLOG_G_WARNING VTDPREFIX
-               "DMAR:[%s] Request device [%04x:%02x:%02x.%u] "
+               "DMAR:[%s] Request device [%pp] "
                "fault addr %"PRIx64", iommu reg = %p\n",
                (type ? "DMA Read" : "DMA Write"),
-               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
-               PCI_FUNC(source_id), addr, iommu->reg);
+               &PCI_SBDF2(seg, source_id), addr, iommu->reg);
         kind = "DMAR";
         break;
     case INTR_REMAP:
         printk(XENLOG_G_WARNING VTDPREFIX
-               "INTR-REMAP: Request device [%04x:%02x:%02x.%u] "
+               "INTR-REMAP: Request device [%pp] "
                "fault index %"PRIx64", iommu reg = %p\n",
-               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
-               PCI_FUNC(source_id), addr >> 48, iommu->reg);
+               &PCI_SBDF2(seg, source_id), addr >> 48, iommu->reg);
         kind = "INTR-REMAP";
         break;
     default:
         printk(XENLOG_G_WARNING VTDPREFIX
-               "UNKNOWN: Request device [%04x:%02x:%02x.%u] "
+               "UNKNOWN: Request device [%pp] "
                "fault addr %"PRIx64", iommu reg = %p\n",
-               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
-               PCI_FUNC(source_id), addr, iommu->reg);
+               &PCI_SBDF2(seg, source_id), addr, iommu->reg);
         kind = "UNKNOWN";
         break;
     }
@@ -1354,11 +1351,9 @@ int domain_context_mapping_one(
         {
             if ( pdev->domain != domain )
             {
-                printk(XENLOG_G_INFO VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u owned by d%d!",
-                       domain->domain_id,
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                       pdev->domain ? pdev->domain->domain_id : -1);
+                printk(XENLOG_G_INFO VTDPREFIX "%pd: %pp owned by %pd!",
+                       domain, &PCI_SBDF3(seg, bus, devfn),
+                       pdev->domain ?: NULL);
                 res = -EINVAL;
             }
         }
@@ -1370,18 +1365,15 @@ int domain_context_mapping_one(
             if ( cdomain < 0 )
             {
                 printk(XENLOG_G_WARNING VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u mapped, but can't find owner!\n",
-                       domain->domain_id,
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+                       "%pd: %pp mapped, but can't find owner!\n",
+                       domain, &PCI_SBDF3(seg, bus, devfn));
                 res = -EINVAL;
             }
             else if ( cdomain != domain->domain_id )
             {
                 printk(XENLOG_G_INFO VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u already mapped to d%d!",
-                       domain->domain_id,
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                       cdomain);
+                       "%pd: %pp already mapped to d%d!",
+                       domain, &PCI_SBDF3(seg, bus, devfn), cdomain);
                 res = -EINVAL;
             }
         }
@@ -1496,9 +1488,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
     {
     case DEV_TYPE_PCI_HOST_BRIDGE:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
-                   domain->domain_id, seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:Hostbridge: skip %pp map\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
         if ( !is_hardware_domain(domain) )
             return -EPERM;
         break;
@@ -1510,9 +1501,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
 
     case DEV_TYPE_PCIe_ENDPOINT:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:PCIe: map %04x:%02x:%02x.%u\n",
-                   domain->domain_id, seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:PCIe: map %pp\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
                                          pdev);
         if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
@@ -1522,9 +1512,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
 
     case DEV_TYPE_PCI:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:PCI: map %04x:%02x:%02x.%u\n",
-                   domain->domain_id, seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:PCI: map %pp\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
 
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
                                          pdev);
@@ -1550,9 +1539,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
         break;
 
     default:
-        dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
-                domain->domain_id, pdev->type,
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_ERR VTDPREFIX, "%pd:unknown(%u): %pp\n",
+                domain, pdev->type, &PCI_SBDF3(seg, bus, devfn));
         ret = -EINVAL;
         break;
     }
@@ -1647,9 +1635,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
     {
     case DEV_TYPE_PCI_HOST_BRIDGE:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
-                   domain->domain_id, seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:Hostbridge: skip %pp unmap\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
         if ( !is_hardware_domain(domain) )
             return -EPERM;
         goto out;
@@ -1661,9 +1648,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
 
     case DEV_TYPE_PCIe_ENDPOINT:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:PCIe: unmap %04x:%02x:%02x.%u\n",
-                   domain->domain_id, seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:PCIe: unmap %pp\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
         ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
             disable_ats_device(pdev);
@@ -1672,8 +1658,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
 
     case DEV_TYPE_PCI:
         if ( iommu_debug )
-            printk(VTDPREFIX "d%d:PCI: unmap %04x:%02x:%02x.%u\n",
-                   domain->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(VTDPREFIX "%pd:PCI: unmap %pp\n",
+                   domain, &PCI_SBDF3(seg, bus, devfn));
         ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( ret )
             break;
@@ -1698,9 +1684,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
         break;
 
     default:
-        dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
-                domain->domain_id, pdev->type,
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_ERR VTDPREFIX, "%pd:unknown(%u): %pp\n",
+                domain, pdev->type, &PCI_SBDF3(seg, bus, devfn));
         ret = -EINVAL;
         goto out;
     }
@@ -2498,12 +2483,11 @@ static int intel_iommu_assign_device(
             bool_t relaxed = !!(flag & XEN_DOMCTL_DEV_RDM_RELAXED);
 
             printk(XENLOG_GUEST "%s" VTDPREFIX
-                   " It's %s to assign %04x:%02x:%02x.%u"
-                   " with shared RMRR at %"PRIx64" for Dom%d.\n",
+                   " It's %s to assign %pp"
+                   " with shared RMRR at %"PRIx64" for %pd.\n",
                    relaxed ? XENLOG_WARNING : XENLOG_ERR,
                    relaxed ? "risky" : "disallowed",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                   rmrr->base_address, d->domain_id);
+                   &PCI_SBDF3(seg, bus, devfn), rmrr->base_address, d);
             if ( !relaxed )
                 return -EPERM;
         }
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 19ffae69c9..cd8842ed43 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -415,8 +415,6 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
 {
     int seg = pdev->seg;
     int bus = pdev->bus;
-    int dev = PCI_SLOT(pdev->devfn);
-    int func = PCI_FUNC(pdev->devfn);
     int pos;
     bool_t ff;
     u32 val, val2;
@@ -440,8 +438,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
     case 0x3c28: /* Sandybridge */
         val = pci_conf_read32(pdev->sbdf, 0x1AC);
         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);
+        printk(XENLOG_INFO "Masked VT-d error signaling on %pp\n", &pdev->sbdf);
         break;
 
     /* Tylersburg (EP)/Boxboro (MP) chipsets (NHM-EP/EX, WSM-EP/EX) */
@@ -476,8 +473,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
             ff = pcie_aer_get_firmware_first(pdev);
         if ( !pos )
         {
-            printk(XENLOG_WARNING "%04x:%02x:%02x.%u without AER capability?\n",
-                   seg, bus, dev, func);
+            printk(XENLOG_WARNING "%pp without AER capability?\n", &pdev->sbdf);
             break;
         }
 
@@ -500,8 +496,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
         val = pci_conf_read32(pdev->sbdf, 0x20c);
         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);
+        printk(XENLOG_INFO "%s UR signaling on %pp\n", action, &pdev->sbdf);
         break;
 
     case 0x0040: case 0x0044: case 0x0048: /* Nehalem/Westmere */
@@ -526,16 +521,15 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
             {
                 __set_bit(0x1c8 * 8 + 20, va);
                 iounmap(va);
-                printk(XENLOG_INFO "Masked UR signaling on %04x:%02x:%02x.%u\n",
-                       seg, bus, dev, func);
+                printk(XENLOG_INFO "Masked UR signaling on %pp\n", &pdev->sbdf);
             }
             else
-                printk(XENLOG_ERR "Could not map %"PRIpaddr" for %04x:%02x:%02x.%u\n",
-                       pa, seg, bus, dev, func);
+                printk(XENLOG_ERR "Could not map %"PRIpaddr" for %pp\n",
+                       pa, &pdev->sbdf);
         }
         else
-            printk(XENLOG_WARNING "Bogus DMIBAR %#"PRIx64" on %04x:%02x:%02x.%u\n",
-                   bar, seg, bus, dev, func);
+            printk(XENLOG_WARNING "Bogus DMIBAR %#"PRIx64" on %pp\n",
+                   bar, &pdev->sbdf);
         break;
     }
 }
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
index 94a6e4eec9..68304a61e8 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -95,9 +95,9 @@ void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn)
     u64 *l, val;
     u32 l_index, level;
 
-    printk("print_vtd_entries: iommu #%u dev %04x:%02x:%02x.%u gmfn %"PRI_gfn"\n",
-           iommu->index, iommu->intel->drhd->segment, bus,
-           PCI_SLOT(devfn), PCI_FUNC(devfn), gmfn);
+    printk("print_vtd_entries: iommu #%u dev %pp gmfn %"PRI_gfn"\n",
+           iommu->index, &PCI_SBDF3(iommu->intel->drhd->segment, bus, devfn),
+           gmfn);
 
     if ( iommu->root_maddr == 0 )
     {
diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index 3eea7f89fc..dc0584b423 100644
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -31,8 +31,7 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
     BUG_ON(!pos);
 
     if ( iommu_verbose )
-        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS capability found\n",
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_INFO, "%pp: ATS capability found\n", &pdev->sbdf);
 
     value = pci_conf_read16(pdev->sbdf, pos + ATS_REG_CTL);
     if ( value & ATS_ENABLE )
@@ -63,9 +62,8 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
     }
 
     if ( iommu_verbose )
-        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS %s enabled\n",
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                pos ? "is" : "was");
+        dprintk(XENLOG_INFO, "%pp: ATS %s enabled\n",
+                &pdev->sbdf, pos ? "is" : "was");
 
     return pos;
 }
@@ -73,8 +71,6 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
 void disable_ats_device(struct pci_dev *pdev)
 {
     u32 value;
-    u16 seg = pdev->seg;
-    u8 bus = pdev->bus, devfn = pdev->devfn;
 
     BUG_ON(!pdev->ats.cap_pos);
 
@@ -85,6 +81,5 @@ void disable_ats_device(struct pci_dev *pdev)
     list_del(&pdev->ats.list);
 
     if ( iommu_verbose )
-        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS is disabled\n",
-                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+        dprintk(XENLOG_INFO, "%pp: ATS is disabled\n", &pdev->sbdf);
 }
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 3c794f486d..ba9a036202 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -355,7 +355,6 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
                       uint32_t val, void *data)
 {
     struct vpci_bar *bar = data;
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     bool hi = false;
 
     if ( bar->type == VPCI_BAR_MEM64_HI )
@@ -372,9 +371,8 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
         /* If the value written is the current one avoid printing a warning. */
         if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
             gprintk(XENLOG_WARNING,
-                    "%04x:%02x:%02x.%u: ignored BAR %lu write with memory decoding enabled\n",
-                    pdev->seg, pdev->bus, slot, func,
-                    bar - pdev->vpci->header.bars + hi);
+                    "%pp: ignored BAR %lu write with memory decoding enabled\n",
+                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
         return;
     }
 
@@ -402,15 +400,14 @@ 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->sbdf, PCI_COMMAND);
     bool new_enabled = val & PCI_ROM_ADDRESS_ENABLE;
 
     if ( (cmd & PCI_COMMAND_MEMORY) && header->rom_enabled && new_enabled )
     {
         gprintk(XENLOG_WARNING,
-                "%04x:%02x:%02x.%u: ignored ROM BAR write with memory decoding enabled\n",
-                pdev->seg, pdev->bus, slot, func);
+                "%pp: ignored ROM BAR write with memory decoding enabled\n",
+                &pdev->sbdf);
         return;
     }
 
diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
index 5b6602f3c2..40e4fca132 100644
--- a/xen/drivers/vpci/msi.c
+++ b/xen/drivers/vpci/msi.c
@@ -290,8 +290,7 @@ void vpci_dump_msi(void)
             msi = pdev->vpci->msi;
             if ( msi && msi->enabled )
             {
-                printk("%04x:%02x:%02x.%u MSI\n", pdev->seg, pdev->bus,
-                       PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+                printk("%pp MSI\n", &pdev->sbdf);
 
                 printk("  enabled: %d 64-bit: %d",
                        msi->enabled, msi->address64);
@@ -308,8 +307,7 @@ void vpci_dump_msi(void)
             {
                 int rc;
 
-                printk("%04x:%02x:%02x.%u MSI-X\n", pdev->seg, pdev->bus,
-                       PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+                printk("%pp MSI-X\n", &pdev->sbdf);
 
                 printk("  entries: %u maskall: %d enabled: %d\n",
                        msix->max_entries, msix->masked, msix->enabled);
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 38c1e7e5dd..64dd0a929c 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -42,15 +42,14 @@ static uint32_t control_read(const struct pci_dev *pdev, unsigned int reg,
 static int update_entry(struct vpci_msix_entry *entry,
                         const struct pci_dev *pdev, unsigned int nr)
 {
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     int rc = vpci_msix_arch_disable_entry(entry, pdev);
 
     /* Ignore ENOENT, it means the entry wasn't setup. */
     if ( rc && rc != -ENOENT )
     {
         gprintk(XENLOG_WARNING,
-                "%04x:%02x:%02x.%u: unable to disable entry %u for update: %d\n",
-                pdev->seg, pdev->bus, slot, func, nr, rc);
+                "%pp: unable to disable entry %u for update: %d\n",
+                &pdev->sbdf, nr, rc);
         return rc;
     }
 
@@ -59,9 +58,8 @@ static int update_entry(struct vpci_msix_entry *entry,
                                                       VPCI_MSIX_TABLE));
     if ( rc )
     {
-        gprintk(XENLOG_WARNING,
-                "%04x:%02x:%02x.%u: unable to enable entry %u: %d\n",
-                pdev->seg, pdev->bus, slot, func, nr, rc);
+        gprintk(XENLOG_WARNING, "%pp: unable to enable entry %u: %d\n",
+                &pdev->sbdf, nr, rc);
         /* Entry is likely not properly configured. */
         return rc;
     }
@@ -72,7 +70,6 @@ static int update_entry(struct vpci_msix_entry *entry,
 static void control_write(const struct pci_dev *pdev, unsigned int reg,
                           uint32_t val, void *data)
 {
-    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     struct vpci_msix *msix = data;
     bool new_masked = val & PCI_MSIX_FLAGS_MASKALL;
     bool new_enabled = val & PCI_MSIX_FLAGS_ENABLE;
@@ -133,9 +130,8 @@ static void control_write(const struct pci_dev *pdev, unsigned int reg,
                 /* Ignore non-present entry. */
                 break;
             default:
-                gprintk(XENLOG_WARNING,
-                        "%04x:%02x:%02x.%u: unable to disable entry %u: %d\n",
-                        pdev->seg, pdev->bus, slot, func, i, rc);
+                gprintk(XENLOG_WARNING, "%pp: unable to disable entry %u: %d\n",
+                        &pdev->sbdf, i, rc);
                 return;
             }
         }
@@ -180,8 +176,7 @@ static bool access_allowed(const struct pci_dev *pdev, unsigned long addr,
         return true;
 
     gprintk(XENLOG_WARNING,
-            "%04x:%02x:%02x.%u: unaligned or invalid size MSI-X table access\n",
-            pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+            "%pp: unaligned or invalid size MSI-X table access\n", &pdev->sbdf);
 
     return false;
 }
@@ -431,10 +426,9 @@ int vpci_make_msix_hole(const struct pci_dev *pdev)
             default:
                 put_gfn(d, start);
                 gprintk(XENLOG_WARNING,
-                        "%04x:%02x:%02x.%u: existing mapping (mfn: %" PRI_mfn
+                        "%pp: existing mapping (mfn: %" PRI_mfn
                         "type: %d) at %#lx clobbers MSIX MMIO area\n",
-                        pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
-                        PCI_FUNC(pdev->devfn), mfn_x(mfn), t, start);
+                        &pdev->sbdf, mfn_x(mfn), t, start);
                 return -EEXIST;
             }
             put_gfn(d, start);
-- 
2.22.0


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

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

* Re: [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t
  2019-08-22  6:51 [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t Roger Pau Monne
@ 2019-08-26  8:42 ` Roger Pau Monné
  2019-08-27 14:14 ` Jan Beulich
  2019-09-19  9:15 ` Julien Grall
  2 siblings, 0 replies; 4+ messages in thread
From: Roger Pau Monné @ 2019-08-26  8:42 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

On Thu, Aug 22, 2019 at 08:51:32AM +0200, Roger Pau Monne wrote:
> The new format specifier is '%pp', and prints a pci_sbdf_t using the
> seg:bus:dev.func format. Replace all SBDFs printed using
> '%04x:%02x:%02x.%u' to use the new format specifier.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

This patch is missing the chunk below, I would appreciate if it can be
merged while commiting if the patch gets the relevant Acks/RBs, or
else I can resend with the chunk added.

Thanks, Roger.
---8<---
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 687a7fa922..1c907cff52 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -898,10 +898,8 @@ static void dump_intremap_table(const struct amd_iommu *iommu,
 
         if ( ivrs_mapping )
         {
-            printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
-                   PCI_BUS(ivrs_mapping->dte_requestor_id),
-                   PCI_SLOT(ivrs_mapping->dte_requestor_id),
-                   PCI_FUNC(ivrs_mapping->dte_requestor_id));
+            printk("  %pp:\n",
+                   &PCI_SBDF2(iommu->seg, ivrs_mapping->dte_requestor_id));
             ivrs_mapping = NULL;
         }
 

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

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

* Re: [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t
  2019-08-22  6:51 [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t Roger Pau Monne
  2019-08-26  8:42 ` Roger Pau Monné
@ 2019-08-27 14:14 ` Jan Beulich
  2019-09-19  9:15 ` Julien Grall
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2019-08-27 14:14 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: KevinTian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, TimDeegan,
	Julien Grall, Suravee Suthikulpanit, xen-devel, Brian Woods

On 22.08.2019 08:51, Roger Pau Monne wrote:
> The new format specifier is '%pp', and prints a pci_sbdf_t using the
> seg:bus:dev.func format. Replace all SBDFs printed using
> '%04x:%02x:%02x.%u' to use the new format specifier.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v1:
>   - Use base 8 to print the function number.
>   - Sort the addition in the pointer function alphabetically.
> ---
>   docs/misc/printk-formats.txt                |   5 +
>   xen/arch/x86/hvm/vmsi.c                     |  10 +-
>   xen/arch/x86/msi.c                          |  35 +++---
>   xen/common/vsprintf.c                       |  18 ++++
>   xen/drivers/passthrough/amd/iommu_acpi.c    |  17 ++-
>   xen/drivers/passthrough/amd/iommu_cmd.c     |   5 +-
>   xen/drivers/passthrough/amd/iommu_detect.c  |   5 +-
>   xen/drivers/passthrough/amd/iommu_init.c    |  12 +--
>   xen/drivers/passthrough/amd/iommu_intr.c    |   3 +-
>   xen/drivers/passthrough/amd/pci_amd_iommu.c |  25 ++---
>   xen/drivers/passthrough/pci.c               | 114 ++++++++------------
>   xen/drivers/passthrough/vtd/dmar.c          |  25 +++--
>   xen/drivers/passthrough/vtd/intremap.c      |  11 +-
>   xen/drivers/passthrough/vtd/iommu.c         |  80 ++++++--------
>   xen/drivers/passthrough/vtd/quirks.c        |  22 ++--
>   xen/drivers/passthrough/vtd/utils.c         |   6 +-
>   xen/drivers/passthrough/x86/ats.c           |  13 +--
>   xen/drivers/vpci/header.c                   |  11 +-
>   xen/drivers/vpci/msi.c                      |   6 +-
>   xen/drivers/vpci/msix.c                     |  24 ++---
>   20 files changed, 189 insertions(+), 258 deletions(-)

In order to not block forward progress, for just the pieces where I am
the only maintainer
Acked-by: Jan Beulich <jbeulich@suse.com>
By implication I still don't really agree with using a %p extension
here.

Jan

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

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

* Re: [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t
  2019-08-22  6:51 [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t Roger Pau Monne
  2019-08-26  8:42 ` Roger Pau Monné
  2019-08-27 14:14 ` Jan Beulich
@ 2019-09-19  9:15 ` Julien Grall
  2 siblings, 0 replies; 4+ messages in thread
From: Julien Grall @ 2019-09-19  9:15 UTC (permalink / raw)
  To: Roger Pau Monne, xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Jan Beulich, Brian Woods

Hi Roger,

On 22/08/2019 07:51, Roger Pau Monne wrote:
> The new format specifier is '%pp', and prints a pci_sbdf_t using the
> seg:bus:dev.func format. Replace all SBDFs printed using
> '%04x:%02x:%02x.%u' to use the new format specifier.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

For the common code:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> Changes since v1:
>   - Use base 8 to print the function number.
>   - Sort the addition in the pointer function alphabetically.
> ---
>   docs/misc/printk-formats.txt                |   5 +
>   xen/arch/x86/hvm/vmsi.c                     |  10 +-
>   xen/arch/x86/msi.c                          |  35 +++---
>   xen/common/vsprintf.c                       |  18 ++++
>   xen/drivers/passthrough/amd/iommu_acpi.c    |  17 ++-
>   xen/drivers/passthrough/amd/iommu_cmd.c     |   5 +-
>   xen/drivers/passthrough/amd/iommu_detect.c  |   5 +-
>   xen/drivers/passthrough/amd/iommu_init.c    |  12 +--
>   xen/drivers/passthrough/amd/iommu_intr.c    |   3 +-
>   xen/drivers/passthrough/amd/pci_amd_iommu.c |  25 ++---
>   xen/drivers/passthrough/pci.c               | 114 ++++++++------------
>   xen/drivers/passthrough/vtd/dmar.c          |  25 +++--
>   xen/drivers/passthrough/vtd/intremap.c      |  11 +-
>   xen/drivers/passthrough/vtd/iommu.c         |  80 ++++++--------
>   xen/drivers/passthrough/vtd/quirks.c        |  22 ++--
>   xen/drivers/passthrough/vtd/utils.c         |   6 +-
>   xen/drivers/passthrough/x86/ats.c           |  13 +--
>   xen/drivers/vpci/header.c                   |  11 +-
>   xen/drivers/vpci/msi.c                      |   6 +-
>   xen/drivers/vpci/msix.c                     |  24 ++---
>   20 files changed, 189 insertions(+), 258 deletions(-)
> 
> diff --git a/docs/misc/printk-formats.txt b/docs/misc/printk-formats.txt
> index 080f498f65..8f666f696a 100644
> --- a/docs/misc/printk-formats.txt
> +++ b/docs/misc/printk-formats.txt
> @@ -48,3 +48,8 @@ Domain and vCPU information:
>                  The domain part as above, with the vcpu_id printed in decimal.
>                    e.g.  d0v1
>                          d[IDLE]v0
> +
> +PCI:
> +
> +       %pp     PCI device address in S:B:D.F format from a pci_sbdf_t.
> +                 e.g.  0004:02:00.0
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index 6597d9f719..c9cd1c26c6 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -693,10 +693,8 @@ static int vpci_msi_update(const struct pci_dev *pdev, uint32_t data,
>   
>           if ( rc )
>           {
> -            gdprintk(XENLOG_ERR,
> -                     "%04x:%02x:%02x.%u: failed to bind PIRQ %u: %d\n",
> -                     pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> -                     PCI_FUNC(pdev->devfn), pirq + i, rc);
> +            gdprintk(XENLOG_ERR, "%pp: failed to bind PIRQ %u: %d\n",
> +                     &pdev->sbdf, pirq + i, rc);
>               while ( bind.machine_irq-- > pirq )
>                   pt_irq_destroy_bind(pdev->domain, &bind);
>               return rc;
> @@ -750,9 +748,7 @@ static int vpci_msi_enable(const struct pci_dev *pdev, uint32_t data,
>                                      &msi_info);
>       if ( rc )
>       {
> -        gdprintk(XENLOG_ERR, "%04x:%02x:%02x.%u: failed to map PIRQ: %d\n",
> -                 pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> -                 PCI_FUNC(pdev->devfn), rc);
> +        gdprintk(XENLOG_ERR, "%pp: failed to map PIRQ: %d\n", &pdev->sbdf, rc);
>           return rc;
>       }
>   
> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
> index d6306005a9..14d86f9195 100644
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -428,8 +428,8 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
>               {
>                   pdev->msix->warned = domid;
>                   printk(XENLOG_G_WARNING
> -                       "cannot mask IRQ %d: masking MSI-X on Dom%d's %04x:%02x:%02x.%u\n",
> -                       desc->irq, domid, seg, bus, slot, func);
> +                       "cannot mask IRQ %d: masking MSI-X on Dom%d's %pp\n",
> +                       desc->irq, domid, &pdev->sbdf);
>               }
>           }
>           pdev->msix->host_maskall = maskall;
> @@ -987,11 +987,11 @@ static int msix_capability_init(struct pci_dev *dev,
>               struct domain *d = dev->domain ?: currd;
>   
>               if ( !is_hardware_domain(currd) || d != currd )
> -                printk("%s use of MSI-X on %04x:%02x:%02x.%u by Dom%d\n",
> +                printk("%s use of MSI-X on %pp by %pd\n",
>                          is_hardware_domain(currd)
>                          ? XENLOG_WARNING "Potentially insecure"
>                          : XENLOG_ERR "Insecure",
> -                       seg, bus, slot, func, d->domain_id);
> +                       &dev->sbdf, d);
>               if ( !is_hardware_domain(d) &&
>                    /* Assume a domain without memory has no mappings yet. */
>                    (!is_hardware_domain(currd) || d->tot_pages) )
> @@ -1046,18 +1046,15 @@ static int __pci_enable_msi(struct msi_info *msi, struct msi_desc **desc)
>       old_desc = find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSI);
>       if ( old_desc )
>       {
> -        printk(XENLOG_ERR "irq %d already mapped to MSI on %04x:%02x:%02x.%u\n",
> -               msi->irq, msi->seg, msi->bus,
> -               PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
> +        printk(XENLOG_ERR "irq %d already mapped to MSI on %pp\n",
> +               msi->irq, &pdev->sbdf);
>           return -EEXIST;
>       }
>   
>       old_desc = find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX);
>       if ( old_desc )
>       {
> -        printk(XENLOG_WARNING "MSI-X already in use on %04x:%02x:%02x.%u\n",
> -               msi->seg, msi->bus,
> -               PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
> +        printk(XENLOG_WARNING "MSI-X already in use on %pp\n", &pdev->sbdf);
>           __pci_disable_msix(old_desc);
>       }
>   
> @@ -1114,16 +1111,15 @@ static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc)
>       old_desc = find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSIX);
>       if ( old_desc )
>       {
> -        printk(XENLOG_ERR "irq %d already mapped to MSI-X on %04x:%02x:%02x.%u\n",
> -               msi->irq, msi->seg, msi->bus, slot, func);
> +        printk(XENLOG_ERR "irq %d already mapped to MSI-X on %pp\n",
> +               msi->irq, &pdev->sbdf);
>           return -EEXIST;
>       }
>   
>       old_desc = find_msi_entry(pdev, -1, PCI_CAP_ID_MSI);
>       if ( old_desc )
>       {
> -        printk(XENLOG_WARNING "MSI already in use on %04x:%02x:%02x.%u\n",
> -               msi->seg, msi->bus, slot, func);
> +        printk(XENLOG_WARNING "MSI already in use on %pp\n", &pdev->sbdf);
>           __pci_disable_msi(old_desc);
>       }
>   
> @@ -1170,9 +1166,8 @@ static void __pci_disable_msix(struct msi_desc *entry)
>           writel(1, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>       else if ( !(control & PCI_MSIX_FLAGS_MASKALL) )
>       {
> -        printk(XENLOG_WARNING
> -               "cannot disable IRQ %d: masking MSI-X on %04x:%02x:%02x.%u\n",
> -               entry->irq, seg, bus, slot, func);
> +        printk(XENLOG_WARNING "cannot disable IRQ %d: masking MSI-X on %pp\n",
> +               entry->irq, &dev->sbdf);
>           maskall = true;
>       }
>       dev->msix->host_maskall = maskall;
> @@ -1340,7 +1335,6 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>       struct msi_desc *entry, *tmp;
>       struct irq_desc *desc;
>       struct msi_msg msg;
> -    u8 slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
>       unsigned int type = 0, pos = 0;
>       u16 control = 0;
>   
> @@ -1369,9 +1363,8 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>           if (desc->msi_desc != entry)
>           {
>       bogus:
> -            dprintk(XENLOG_ERR,
> -                    "Restore MSI for %04x:%02x:%02x:%u entry %u not set?\n",
> -                    pdev->seg, pdev->bus, slot, func, i);
> +            dprintk(XENLOG_ERR, "Restore MSI for %pp entry %u not set?\n",
> +                    &pdev->sbdf, i);
>               spin_unlock_irqrestore(&desc->lock, flags);
>               if ( type == PCI_CAP_ID_MSIX )
>                   pci_conf_write16(pdev->sbdf, msix_control_reg(pos),
> diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
> index 183d3ed4bb..185a4bd561 100644
> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -394,6 +394,20 @@ static char *print_vcpu(char *str, const char *end, const struct vcpu *v)
>       return number(str + 1, end, v->vcpu_id, 10, -1, -1, 0);
>   }
>   
> +static char *print_pci_addr(char *str, const char *end, const pci_sbdf_t *sbdf)
> +{
> +    str = number(str, end, sbdf->seg, 16, 4, -1, ZEROPAD);
> +    if ( str < end )
> +        *str = ':';
> +    str = number(str + 1, end, sbdf->bus, 16, 2, -1, ZEROPAD);
> +    if ( str < end )
> +        *str = ':';
> +    str = number(str + 1, end, sbdf->dev, 16, 2, -1, ZEROPAD);
> +    if ( str < end )
> +        *str = '.';
> +    return number(str + 1, end, sbdf->fn, 8, -1, -1, 0);
> +}
> +
>   static char *pointer(char *str, const char *end, const char **fmt_ptr,
>                        const void *arg, int field_width, int precision,
>                        int flags)
> @@ -476,6 +490,10 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>           }
>       }
>   
> +    case 'p': /* PCI SBDF. */
> +        ++*fmt_ptr;
> +        return print_pci_addr(str, end, arg);
> +
>       case 's': /* Symbol name with offset and size (iff offset != 0) */
>       case 'S': /* Symbol name unconditionally with offset and size */
>       {
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 668a9805ef..947a24d719 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -724,9 +724,8 @@ static u16 __init parse_ivhd_device_special(
>           return 0;
>       }
>   
> -    AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle %#x\n",
> -                    seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
> -                    special->variety, special->handle);
> +    AMD_IOMMU_DEBUG("IVHD Special: %pp variety %#x handle %#x\n",
> +                    &PCI_SBDF2(seg, bdf), special->variety, special->handle);
>       add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);
>   
>       switch ( special->variety )
> @@ -749,9 +748,9 @@ static u16 __init parse_ivhd_device_special(
>           if ( idx < nr_ioapic_sbdf )
>           {
>               AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x"
> -                            "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
> -                            ioapic_sbdf[idx].id, special->handle, seg,
> -                            PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
> +                            "(IVRS: %#x devID %pp)\n",
> +                            ioapic_sbdf[idx].id, special->handle,
> +                            &PCI_SBDF2(seg, bdf));
>               break;
>           }
>   
> @@ -821,9 +820,9 @@ static u16 __init parse_ivhd_device_special(
>               break;
>           case HPET_CMDL:
>               AMD_IOMMU_DEBUG("IVHD: Command line override present for HPET %#x "
> -                            "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
> -                            hpet_sbdf.id, special->handle, seg, PCI_BUS(bdf),
> -                            PCI_SLOT(bdf), PCI_FUNC(bdf));
> +                            "(IVRS: %#x devID %pp)\n",
> +                            hpet_sbdf.id, special->handle,
> +                            &PCI_SBDF2(seg, bdf));
>               break;
>           case HPET_NONE:
>               /* set device id of hpet */
> diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
> index af3a1fb865..3eeb214ba3 100644
> --- a/xen/drivers/passthrough/amd/iommu_cmd.c
> +++ b/xen/drivers/passthrough/amd/iommu_cmd.c
> @@ -296,9 +296,8 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
>   
>       if ( !iommu )
>       {
> -        AMD_IOMMU_DEBUG("%s: Can't find iommu for %04x:%02x:%02x.%u\n",
> -                        __func__, pdev->seg, pdev->bus,
> -                        PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +        AMD_IOMMU_DEBUG("%s: Can't find iommu for %pp\n",
> +                        __func__, &pdev->sbdf);
>           return;
>       }
>   
> diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/amd/iommu_detect.c
> index d782e66eee..21a5275d6d 100644
> --- a/xen/drivers/passthrough/amd/iommu_detect.c
> +++ b/xen/drivers/passthrough/amd/iommu_detect.c
> @@ -185,9 +185,8 @@ int __init amd_iommu_detect_one_acpi(
>   
>       rt = pci_ro_device(iommu->seg, bus, PCI_DEVFN(dev, func));
>       if ( rt )
> -        printk(XENLOG_ERR
> -               "Could not mark config space of %04x:%02x:%02x.%u read-only (%d)\n",
> -               iommu->seg, bus, dev, func, rt);
> +        printk(XENLOG_ERR "Could not mark config space of %pp read-only (%d)\n",
> +               &PCI_SBDF2(iommu->seg, iommu->bdf), rt);
>   
>       list_add_tail(&iommu->list, &amd_iommu_head);
>       rt = 0;
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
> index bb5a3e57c9..634c167982 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -778,9 +778,8 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
>       pcidevs_unlock();
>       if ( !iommu->msi.dev )
>       {
> -        AMD_IOMMU_DEBUG("IOMMU: no pdev for %04x:%02x:%02x.%u\n",
> -                        iommu->seg, PCI_BUS(iommu->bdf),
> -                        PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf));
> +        AMD_IOMMU_DEBUG("IOMMU: no pdev for %pp\n",
> +                        &PCI_SBDF2(iommu->seg, iommu->bdf));
>           return 0;
>       }
>   
> @@ -866,9 +865,6 @@ __initcall(iov_adjust_irq_affinities);
>   static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
>   {
>       u32 value;
> -    u8 bus = PCI_BUS(iommu->bdf);
> -    u8 dev = PCI_SLOT(iommu->bdf);
> -    u8 func = PCI_FUNC(iommu->bdf);
>   
>       if ( (boot_cpu_data.x86 != 0x15) ||
>            (boot_cpu_data.x86_model < 0x10) ||
> @@ -886,8 +882,8 @@ static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
>   
>       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);
> +           "AMD-Vi: Applying erratum 746 workaround for IOMMU at %pp\n",
> +           &PCI_SBDF2(iommu->seg, iommu->bdf));
>   
>       /* Clear the enable writing bit */
>       pci_conf_write32(PCI_SBDF2(iommu->seg, iommu->bdf), 0xf0, 0x90);
> diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
> index 0164ceac3b..687a7fa922 100644
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -685,8 +685,7 @@ static struct amd_iommu *_find_iommu_for_device(int seg, int bdf)
>       if ( iommu )
>           return iommu;
>   
> -    AMD_IOMMU_DEBUG("No IOMMU for MSI dev = %04x:%02x:%02x.%u\n",
> -                    seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
> +    AMD_IOMMU_DEBUG("No IOMMU for MSI dev = %pp\n", &PCI_SBDF2(seg, bdf));
>       return ERR_PTR(-EINVAL);
>   }
>   
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index 8d4a5fbc37..ae20e9b6e5 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -52,9 +52,8 @@ struct amd_iommu *find_iommu_for_device(int seg, int bdf)
>                   tmp.dte_requestor_id = bdf;
>               ivrs_mappings[bdf] = tmp;
>   
> -            printk(XENLOG_WARNING "%04x:%02x:%02x.%u not found in ACPI tables;"
> -                   " using same IOMMU as function 0\n",
> -                   seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
> +            printk(XENLOG_WARNING "%pp not found in ACPI tables;"
> +                   " using same IOMMU as function 0\n", &PCI_SBDF2(seg, bdf));
>   
>               /* write iommu field last */
>               ivrs_mappings[bdf].iommu = ivrs_mappings[bd0].iommu;
> @@ -337,9 +336,8 @@ static int reassign_device(struct domain *source, struct domain *target,
>           return rc;
>   
>       amd_iommu_setup_domain_device(target, iommu, devfn, pdev);
> -    AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
> -                    pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                    source->domain_id, target->domain_id);
> +    AMD_IOMMU_DEBUG("Re-assign %pp from dom%d to dom%d\n",
> +                    &pdev->sbdf, source->domain_id, target->domain_id);
>   
>       return 0;
>   }
> @@ -446,15 +444,12 @@ static int amd_iommu_add_device(u8 devfn, struct pci_dev *pdev)
>           if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE &&
>                is_hardware_domain(pdev->domain) )
>           {
> -            AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
> -                            pdev->seg, pdev->bus, PCI_SLOT(devfn),
> -                            PCI_FUNC(devfn));
> +            AMD_IOMMU_DEBUG("Skipping host bridge %pp\n", &pdev->sbdf);
>               return 0;
>           }
>   
> -        AMD_IOMMU_DEBUG("No iommu for %04x:%02x:%02x.%u; cannot be handed to d%d\n",
> -                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                        pdev->domain->domain_id);
> +        AMD_IOMMU_DEBUG("No iommu for %pp; cannot be handed to d%d\n",
> +                        &pdev->sbdf, pdev->domain->domain_id);
>           return -ENODEV;
>       }
>   
> @@ -473,10 +468,8 @@ static int amd_iommu_remove_device(u8 devfn, struct pci_dev *pdev)
>       iommu = find_iommu_for_device(pdev->seg, bdf);
>       if ( !iommu )
>       {
> -        AMD_IOMMU_DEBUG("Fail to find iommu."
> -                        " %04x:%02x:%02x.%u cannot be removed from dom%d\n",
> -                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                        pdev->domain->domain_id);
> +        AMD_IOMMU_DEBUG("Fail to find iommu. %pp cannot be removed from %pd\n",
> +                        &pdev->sbdf, pdev->domain);
>           return -ENODEV;
>       }
>   
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 7c196ba58b..bb916d4294 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -238,11 +238,7 @@ static void check_pdev(const struct pci_dev *pdev)
>       (PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT | \
>        PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT | \
>        PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY)
> -    u16 seg = pdev->seg;
> -    u8 bus = pdev->bus;
> -    u8 dev = PCI_SLOT(pdev->devfn);
> -    u8 func = PCI_FUNC(pdev->devfn);
> -    u16 val;
> +     u16 val;
>   
>       if ( command_mask )
>       {
> @@ -252,8 +248,8 @@ static void check_pdev(const struct pci_dev *pdev)
>           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);
> +            printk(XENLOG_INFO "%pp status %04x -> %04x\n",
> +                   &pdev->sbdf, val, val & ~PCI_STATUS_CHECK);
>               pci_conf_write16(pdev->sbdf, PCI_STATUS, val & PCI_STATUS_CHECK);
>           }
>       }
> @@ -270,9 +266,8 @@ static void check_pdev(const struct pci_dev *pdev)
>           val = pci_conf_read16(pdev->sbdf, PCI_SEC_STATUS);
>           if ( val & PCI_STATUS_CHECK )
>           {
> -            printk(XENLOG_INFO
> -                   "%04x:%02x:%02x.%u secondary status %04x -> %04x\n",
> -                   seg, bus, dev, func, val, val & ~PCI_STATUS_CHECK);
> +            printk(XENLOG_INFO "%pp secondary status %04x -> %04x\n",
> +                   &pdev->sbdf, val, val & ~PCI_STATUS_CHECK);
>               pci_conf_write16(pdev->sbdf, PCI_SEC_STATUS,
>                                val & PCI_STATUS_CHECK);
>           }
> @@ -411,8 +406,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
>               break;
>   
>           default:
> -            printk(XENLOG_WARNING "%04x:%02x:%02x.%u: unknown type %d\n",
> -                   pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pdev->type);
> +            printk(XENLOG_WARNING "%pp: unknown type %d\n",
> +                   &pdev->sbdf, pdev->type);
>               break;
>       }
>   
> @@ -644,9 +639,9 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos,
>           if ( flags & PCI_BAR_LAST )
>           {
>               printk(XENLOG_WARNING
> -                   "%sdevice %04x:%02x:%02x.%u with 64-bit %sBAR in last slot\n",
> -                   (flags & PCI_BAR_VF) ? "SR-IOV " : "", sbdf.seg, sbdf.bus,
> -                   sbdf.dev, sbdf.fn, (flags & PCI_BAR_VF) ? "vf " : "");
> +                   "%sdevice %pp with 64-bit %sBAR in last slot\n",
> +                   (flags & PCI_BAR_VF) ? "SR-IOV " : "", &sbdf,
> +                   (flags & PCI_BAR_VF) ? "vf " : "");
>               *psize = 0;
>               return 1;
>           }
> @@ -750,9 +745,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>                        PCI_BASE_ADDRESS_SPACE_IO )
>                   {
>                       printk(XENLOG_WARNING
> -                           "SR-IOV device %04x:%02x:%02x.%u with vf BAR%u"
> -                           " in IO space\n",
> -                           seg, bus, slot, func, i);
> +                           "SR-IOV device %pp with vf BAR%u in IO space\n",
> +                           &pdev->sbdf, i);
>                       continue;
>                   }
>                   ret = pci_size_mem_bar(sbdf, idx, NULL, &pdev->vf_rlen[i],
> @@ -764,10 +758,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>               }
>           }
>           else
> -            printk(XENLOG_WARNING
> -                   "SR-IOV device %04x:%02x:%02x.%u has its virtual"
> -                   " functions already enabled (%04x)\n",
> -                   seg, bus, slot, func, ctrl);
> +            printk(XENLOG_WARNING "SR-IOV device %pp has its virtual"
> +                   " functions already enabled (%04x)\n", &pdev->sbdf, ctrl);
>       }
>   
>       check_pdev(pdev);
> @@ -794,15 +786,14 @@ out:
>       pcidevs_unlock();
>       if ( !ret )
>       {
> -        printk(XENLOG_DEBUG "PCI add %s %04x:%02x:%02x.%u\n", pdev_type,
> -               seg, bus, slot, func);
> +        printk(XENLOG_DEBUG "PCI add %s %pp\n", pdev_type,  &pdev->sbdf);
>           while ( pdev->phantom_stride )
>           {
>               func += pdev->phantom_stride;
>               if ( PCI_SLOT(func) )
>                   break;
> -            printk(XENLOG_DEBUG "PCI phantom %04x:%02x:%02x.%u\n",
> -                   seg, bus, slot, func);
> +            printk(XENLOG_DEBUG "PCI phantom %pp\n",
> +                   &PCI_SBDF(seg, bus, slot, func));
>           }
>       }
>       return ret;
> @@ -831,9 +822,8 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>               if ( pdev->domain )
>                   list_del(&pdev->domain_list);
>               pci_cleanup_msi(pdev);
> +            printk(XENLOG_DEBUG "PCI remove device %pp\n", &pdev->sbdf);
>               free_pdev(pseg, pdev);
> -            printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
> -                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
>               break;
>           }
>   
> @@ -907,9 +897,8 @@ int pci_release_devices(struct domain *d)
>           bus = pdev->bus;
>           devfn = pdev->devfn;
>           if ( deassign_device(d, pdev->seg, bus, devfn) )
> -            printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!\n",
> -                   d->domain_id, pdev->seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk("domain %d: deassign device (%pp) failed!\n",
> +                   d->domain_id, &pdev->sbdf);
>       }
>       pcidevs_unlock();
>   
> @@ -1056,8 +1045,8 @@ static int __init _scan_pci_devices(struct pci_seg *pseg, void *arg)
>                   pdev = alloc_pdev(pseg, bus, PCI_DEVFN(dev, func));
>                   if ( !pdev )
>                   {
> -                    printk(XENLOG_WARNING "%04x:%02x:%02x.%u: alloc_pdev failed\n",
> -                           pseg->nr, bus, dev, func);
> +                    printk(XENLOG_WARNING "%pp: alloc_pdev failed\n",
> +                           &PCI_SBDF(pseg->nr, bus, dev, func));
>                       return -ENOMEM;
>                   }
>   
> @@ -1098,9 +1087,8 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
>           err = ctxt->handler(devfn, pdev);
>           if ( err )
>           {
> -            printk(XENLOG_ERR "setup %04x:%02x:%02x.%u for d%d failed (%d)\n",
> -                   pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                   ctxt->d->domain_id, err);
> +            printk(XENLOG_ERR "setup %pp for d%d failed (%d)\n",
> +                   &pdev->sbdf, ctxt->d->domain_id, err);
>               if ( devfn == pdev->devfn )
>                   return;
>           }
> @@ -1141,9 +1129,8 @@ static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void *arg
>                   pdev->domain = dom_xen;
>               }
>               else if ( pdev->domain != ctxt->d )
> -                printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
> -                       pdev->domain->domain_id, pseg->nr, bus,
> -                       PCI_SLOT(devfn), PCI_FUNC(devfn));
> +                printk(XENLOG_WARNING "Dom%d owning %pp?\n",
> +                       pdev->domain->domain_id, &pdev->sbdf);
>   
>               if ( iommu_verbose )
>               {
> @@ -1279,10 +1266,8 @@ static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
>   
>       list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
>       {
> -        printk("%04x:%02x:%02x.%u - dom %-3d - node %-3d - MSIs < ",
> -               pseg->nr, pdev->bus,
> -               PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> -               pdev->domain ? pdev->domain->domain_id : -1,
> +        printk("%pp - dom %-3d - node %-3d - MSIs < ",
> +               &pdev->sbdf, pdev->domain ? pdev->domain->domain_id : -1,
>                  (pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
>           list_for_each_entry ( msi, &pdev->msi_list, list )
>                  printk("%d ", msi->irq);
> @@ -1347,8 +1332,8 @@ static int iommu_add_device(struct pci_dev *pdev)
>               return 0;
>           rc = hd->platform_ops->add_device(devfn, pci_to_dev(pdev));
>           if ( rc )
> -            printk(XENLOG_WARNING "IOMMU: add %04x:%02x:%02x.%u failed (%d)\n",
> -                   pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc);
> +            printk(XENLOG_WARNING "IOMMU: add %pp failed (%d)\n",
> +                   &pdev->sbdf, rc);
>       }
>   }
>   
> @@ -1392,8 +1377,7 @@ static int iommu_remove_device(struct pci_dev *pdev)
>           if ( !rc )
>               continue;
>   
> -        printk(XENLOG_ERR "IOMMU: remove %04x:%02x:%02x.%u failed (%d)\n",
> -               pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc);
> +        printk(XENLOG_ERR "IOMMU: remove %pp failed (%d)\n", &pdev->sbdf, rc);
>           return rc;
>       }
>   
> @@ -1463,9 +1447,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
>               break;
>           rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), flag);
>           if ( rc )
> -            printk(XENLOG_G_WARNING "d%d: assign %04x:%02x:%02x.%u failed (%d)\n",
> -                   d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                   rc);
> +            printk(XENLOG_G_WARNING "d%d: assign %pp failed (%d)\n",
> +                   d->domain_id, &PCI_SBDF3(seg, bus, devfn), rc);
>       }
>   
>    done:
> @@ -1501,8 +1484,8 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
>           if ( !ret )
>               continue;
>   
> -        printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n",
> -               d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
> +        printk(XENLOG_G_ERR "d%d: deassign %pp failed (%d)\n",
> +               d->domain_id, &PCI_SBDF3(seg, bus, devfn), ret);
>           return ret;
>       }
>   
> @@ -1511,9 +1494,8 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
>                                               pci_to_dev(pdev));
>       if ( ret )
>       {
> -        dprintk(XENLOG_G_ERR,
> -                "d%d: deassign device (%04x:%02x:%02x.%u) failed\n",
> -                d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_G_ERR, "d%d: deassign device (%pp) failed\n",
> +                d->domain_id, &pdev->sbdf);
>           return ret;
>       }
>   
> @@ -1590,10 +1572,8 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev)
>       _pci_hide_device(pdev);
>   
>       if ( !d->is_shutting_down && printk_ratelimit() )
> -        printk(XENLOG_ERR
> -               "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> -               d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> -               PCI_FUNC(pdev->devfn));
> +        printk(XENLOG_ERR "dom%d: ATS device %pp flush failed\n",
> +               d->domain_id, &pdev->sbdf);
>       if ( !is_hardware_domain(d) )
>           domain_crash(d);
>   
> @@ -1682,9 +1662,8 @@ int iommu_do_pci_domctl(
>           {
>               if ( ret )
>               {
> -                printk(XENLOG_G_INFO
> -                       "%04x:%02x:%02x.%u already assigned, or non-existent\n",
> -                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +                printk(XENLOG_G_INFO "%pp already assigned, or non-existent\n",
> +                       &PCI_SBDF3(seg, bus, devfn));
>                   ret = -EINVAL;
>               }
>               break;
> @@ -1696,9 +1675,8 @@ int iommu_do_pci_domctl(
>                                                   "h", u_domctl);
>           else if ( ret )
>               printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: "
> -                   "assign %04x:%02x:%02x.%u to dom%d failed (%d)\n",
> -                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                   d->domain_id, ret);
> +                   "assign %pp to dom%d failed (%d)\n",
> +                   &PCI_SBDF3(seg, bus, devfn), d->domain_id, ret);
>   
>           break;
>   
> @@ -1732,10 +1710,8 @@ int iommu_do_pci_domctl(
>           ret = deassign_device(d, seg, bus, devfn);
>           pcidevs_unlock();
>           if ( ret )
> -            printk(XENLOG_G_ERR
> -                   "deassign %04x:%02x:%02x.%u from dom%d failed (%d)\n",
> -                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                   d->domain_id, ret);
> +            printk(XENLOG_G_ERR "deassign %pp from dom%d failed (%d)\n",
> +                   &PCI_SBDF3(seg, bus, devfn), d->domain_id, ret);
>   
>           break;
>   
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index 9c94deac0b..4c0d2f6672 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -361,9 +361,8 @@ static int __init acpi_parse_dev_scope(
>               sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
>                                        PCI_SUBORDINATE_BUS);
>               if ( iommu_verbose )
> -                printk(VTDPREFIX
> -                       " bridge: %04x:%02x:%02x.%u start=%x sec=%x sub=%x\n",
> -                       seg, bus, path->dev, path->fn,
> +                printk(VTDPREFIX " bridge: %pp start=%x sec=%x sub=%x\n",
> +                       &PCI_SBDF(seg, bus, path->dev, path->fn),
>                          acpi_scope->bus, sec_bus, sub_bus);
>   
>               dmar_scope_add_buses(scope, sec_bus, sub_bus);
> @@ -371,8 +370,8 @@ static int __init acpi_parse_dev_scope(
>   
>           case ACPI_DMAR_SCOPE_TYPE_HPET:
>               if ( iommu_verbose )
> -                printk(VTDPREFIX " MSI HPET: %04x:%02x:%02x.%u\n",
> -                       seg, bus, path->dev, path->fn);
> +                printk(VTDPREFIX " MSI HPET: %pp\n",
> +                       &PCI_SBDF(seg, bus, path->dev, path->fn));
>   
>               if ( drhd )
>               {
> @@ -393,8 +392,8 @@ static int __init acpi_parse_dev_scope(
>   
>           case ACPI_DMAR_SCOPE_TYPE_ENDPOINT:
>               if ( iommu_verbose )
> -                printk(VTDPREFIX " endpoint: %04x:%02x:%02x.%u\n",
> -                       seg, bus, path->dev, path->fn);
> +                printk(VTDPREFIX " endpoint: %pp\n",
> +                       &PCI_SBDF(seg, bus, path->dev, path->fn));
>   
>               if ( drhd )
>               {
> @@ -407,8 +406,8 @@ static int __init acpi_parse_dev_scope(
>   
>           case ACPI_DMAR_SCOPE_TYPE_IOAPIC:
>               if ( iommu_verbose )
> -                printk(VTDPREFIX " IOAPIC: %04x:%02x:%02x.%u\n",
> -                       seg, bus, path->dev, path->fn);
> +                printk(VTDPREFIX " IOAPIC: %pp\n",
> +                       &PCI_SBDF(seg, bus, path->dev, path->fn));
>   
>               if ( drhd )
>               {
> @@ -537,8 +536,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>   
>               if ( !pci_device_detect(drhd->segment, b, d, f) )
>                   printk(XENLOG_WARNING VTDPREFIX
> -                       " Non-existent device (%04x:%02x:%02x.%u) in this DRHD's scope!\n",
> -                       drhd->segment, b, d, f);
> +                       " Non-existent device (%pp) in this DRHD's scope!\n",
> +                       &PCI_SBDF(drhd->segment, b, d, f));
>           }
>   
>           acpi_register_drhd_unit(dmaru);
> @@ -574,9 +573,9 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
>           if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
>           {
>               dprintk(XENLOG_WARNING VTDPREFIX,
> -                    " Non-existent device (%04x:%02x:%02x.%u) is reported"
> +                    " Non-existent device (%pp) is reported"
>                       " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> -                    rmrru->segment, b, d, f,
> +                    &PCI_SBDF(rmrru->segment, b, d, f),
>                       rmrru->base_address, rmrru->end_address);
>               ignore = true;
>           }
> diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
> index df0e8ac5cb..9b86fa8bb7 100644
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -524,16 +524,13 @@ static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire)
>           }
>           else
>               dprintk(XENLOG_WARNING VTDPREFIX,
> -                    "d%d: no upstream bridge for %04x:%02x:%02x.%u\n",
> -                    pdev->domain->domain_id,
> -                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +                    "d%d: no upstream bridge for %pp\n",
> +                    pdev->domain->domain_id, &pdev->sbdf);
>           break;
>   
>       default:
> -        dprintk(XENLOG_WARNING VTDPREFIX,
> -                "d%d: unknown(%u): %04x:%02x:%02x.%u\n",
> -                pdev->domain->domain_id, pdev->type,
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_WARNING VTDPREFIX, "d%d: unknown(%u): %pp\n",
> +                pdev->domain->domain_id, pdev->type, &pdev->sbdf);
>           break;
>      }
>   }
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 5d72270c5b..cfaee1503f 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -882,27 +882,24 @@ static int iommu_page_fault_do_one(struct iommu *iommu, int type,
>       {
>       case DMA_REMAP:
>           printk(XENLOG_G_WARNING VTDPREFIX
> -               "DMAR:[%s] Request device [%04x:%02x:%02x.%u] "
> +               "DMAR:[%s] Request device [%pp] "
>                  "fault addr %"PRIx64", iommu reg = %p\n",
>                  (type ? "DMA Read" : "DMA Write"),
> -               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
> -               PCI_FUNC(source_id), addr, iommu->reg);
> +               &PCI_SBDF2(seg, source_id), addr, iommu->reg);
>           kind = "DMAR";
>           break;
>       case INTR_REMAP:
>           printk(XENLOG_G_WARNING VTDPREFIX
> -               "INTR-REMAP: Request device [%04x:%02x:%02x.%u] "
> +               "INTR-REMAP: Request device [%pp] "
>                  "fault index %"PRIx64", iommu reg = %p\n",
> -               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
> -               PCI_FUNC(source_id), addr >> 48, iommu->reg);
> +               &PCI_SBDF2(seg, source_id), addr >> 48, iommu->reg);
>           kind = "INTR-REMAP";
>           break;
>       default:
>           printk(XENLOG_G_WARNING VTDPREFIX
> -               "UNKNOWN: Request device [%04x:%02x:%02x.%u] "
> +               "UNKNOWN: Request device [%pp] "
>                  "fault addr %"PRIx64", iommu reg = %p\n",
> -               seg, PCI_BUS(source_id), PCI_SLOT(source_id),
> -               PCI_FUNC(source_id), addr, iommu->reg);
> +               &PCI_SBDF2(seg, source_id), addr, iommu->reg);
>           kind = "UNKNOWN";
>           break;
>       }
> @@ -1354,11 +1351,9 @@ int domain_context_mapping_one(
>           {
>               if ( pdev->domain != domain )
>               {
> -                printk(XENLOG_G_INFO VTDPREFIX
> -                       "d%d: %04x:%02x:%02x.%u owned by d%d!",
> -                       domain->domain_id,
> -                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                       pdev->domain ? pdev->domain->domain_id : -1);
> +                printk(XENLOG_G_INFO VTDPREFIX "%pd: %pp owned by %pd!",
> +                       domain, &PCI_SBDF3(seg, bus, devfn),
> +                       pdev->domain ?: NULL);
>                   res = -EINVAL;
>               }
>           }
> @@ -1370,18 +1365,15 @@ int domain_context_mapping_one(
>               if ( cdomain < 0 )
>               {
>                   printk(XENLOG_G_WARNING VTDPREFIX
> -                       "d%d: %04x:%02x:%02x.%u mapped, but can't find owner!\n",
> -                       domain->domain_id,
> -                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +                       "%pd: %pp mapped, but can't find owner!\n",
> +                       domain, &PCI_SBDF3(seg, bus, devfn));
>                   res = -EINVAL;
>               }
>               else if ( cdomain != domain->domain_id )
>               {
>                   printk(XENLOG_G_INFO VTDPREFIX
> -                       "d%d: %04x:%02x:%02x.%u already mapped to d%d!",
> -                       domain->domain_id,
> -                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                       cdomain);
> +                       "%pd: %pp already mapped to d%d!",
> +                       domain, &PCI_SBDF3(seg, bus, devfn), cdomain);
>                   res = -EINVAL;
>               }
>           }
> @@ -1496,9 +1488,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
>       {
>       case DEV_TYPE_PCI_HOST_BRIDGE:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
> -                   domain->domain_id, seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:Hostbridge: skip %pp map\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>           if ( !is_hardware_domain(domain) )
>               return -EPERM;
>           break;
> @@ -1510,9 +1501,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
>   
>       case DEV_TYPE_PCIe_ENDPOINT:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:PCIe: map %04x:%02x:%02x.%u\n",
> -                   domain->domain_id, seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:PCIe: map %pp\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>           ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
>                                            pdev);
>           if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
> @@ -1522,9 +1512,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
>   
>       case DEV_TYPE_PCI:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:PCI: map %04x:%02x:%02x.%u\n",
> -                   domain->domain_id, seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:PCI: map %pp\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>   
>           ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
>                                            pdev);
> @@ -1550,9 +1539,8 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
>           break;
>   
>       default:
> -        dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
> -                domain->domain_id, pdev->type,
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_ERR VTDPREFIX, "%pd:unknown(%u): %pp\n",
> +                domain, pdev->type, &PCI_SBDF3(seg, bus, devfn));
>           ret = -EINVAL;
>           break;
>       }
> @@ -1647,9 +1635,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
>       {
>       case DEV_TYPE_PCI_HOST_BRIDGE:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
> -                   domain->domain_id, seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:Hostbridge: skip %pp unmap\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>           if ( !is_hardware_domain(domain) )
>               return -EPERM;
>           goto out;
> @@ -1661,9 +1648,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
>   
>       case DEV_TYPE_PCIe_ENDPOINT:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:PCIe: unmap %04x:%02x:%02x.%u\n",
> -                   domain->domain_id, seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:PCIe: unmap %pp\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>           ret = domain_context_unmap_one(domain, iommu, bus, devfn);
>           if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
>               disable_ats_device(pdev);
> @@ -1672,8 +1658,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
>   
>       case DEV_TYPE_PCI:
>           if ( iommu_debug )
> -            printk(VTDPREFIX "d%d:PCI: unmap %04x:%02x:%02x.%u\n",
> -                   domain->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +            printk(VTDPREFIX "%pd:PCI: unmap %pp\n",
> +                   domain, &PCI_SBDF3(seg, bus, devfn));
>           ret = domain_context_unmap_one(domain, iommu, bus, devfn);
>           if ( ret )
>               break;
> @@ -1698,9 +1684,8 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
>           break;
>   
>       default:
> -        dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
> -                domain->domain_id, pdev->type,
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_ERR VTDPREFIX, "%pd:unknown(%u): %pp\n",
> +                domain, pdev->type, &PCI_SBDF3(seg, bus, devfn));
>           ret = -EINVAL;
>           goto out;
>       }
> @@ -2498,12 +2483,11 @@ static int intel_iommu_assign_device(
>               bool_t relaxed = !!(flag & XEN_DOMCTL_DEV_RDM_RELAXED);
>   
>               printk(XENLOG_GUEST "%s" VTDPREFIX
> -                   " It's %s to assign %04x:%02x:%02x.%u"
> -                   " with shared RMRR at %"PRIx64" for Dom%d.\n",
> +                   " It's %s to assign %pp"
> +                   " with shared RMRR at %"PRIx64" for %pd.\n",
>                      relaxed ? XENLOG_WARNING : XENLOG_ERR,
>                      relaxed ? "risky" : "disallowed",
> -                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                   rmrr->base_address, d->domain_id);
> +                   &PCI_SBDF3(seg, bus, devfn), rmrr->base_address, d);
>               if ( !relaxed )
>                   return -EPERM;
>           }
> diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
> index 19ffae69c9..cd8842ed43 100644
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -415,8 +415,6 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>   {
>       int seg = pdev->seg;
>       int bus = pdev->bus;
> -    int dev = PCI_SLOT(pdev->devfn);
> -    int func = PCI_FUNC(pdev->devfn);
>       int pos;
>       bool_t ff;
>       u32 val, val2;
> @@ -440,8 +438,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>       case 0x3c28: /* Sandybridge */
>           val = pci_conf_read32(pdev->sbdf, 0x1AC);
>           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);
> +        printk(XENLOG_INFO "Masked VT-d error signaling on %pp\n", &pdev->sbdf);
>           break;
>   
>       /* Tylersburg (EP)/Boxboro (MP) chipsets (NHM-EP/EX, WSM-EP/EX) */
> @@ -476,8 +473,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>               ff = pcie_aer_get_firmware_first(pdev);
>           if ( !pos )
>           {
> -            printk(XENLOG_WARNING "%04x:%02x:%02x.%u without AER capability?\n",
> -                   seg, bus, dev, func);
> +            printk(XENLOG_WARNING "%pp without AER capability?\n", &pdev->sbdf);
>               break;
>           }
>   
> @@ -500,8 +496,7 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>           val = pci_conf_read32(pdev->sbdf, 0x20c);
>           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);
> +        printk(XENLOG_INFO "%s UR signaling on %pp\n", action, &pdev->sbdf);
>           break;
>   
>       case 0x0040: case 0x0044: case 0x0048: /* Nehalem/Westmere */
> @@ -526,16 +521,15 @@ void pci_vtd_quirk(const struct pci_dev *pdev)
>               {
>                   __set_bit(0x1c8 * 8 + 20, va);
>                   iounmap(va);
> -                printk(XENLOG_INFO "Masked UR signaling on %04x:%02x:%02x.%u\n",
> -                       seg, bus, dev, func);
> +                printk(XENLOG_INFO "Masked UR signaling on %pp\n", &pdev->sbdf);
>               }
>               else
> -                printk(XENLOG_ERR "Could not map %"PRIpaddr" for %04x:%02x:%02x.%u\n",
> -                       pa, seg, bus, dev, func);
> +                printk(XENLOG_ERR "Could not map %"PRIpaddr" for %pp\n",
> +                       pa, &pdev->sbdf);
>           }
>           else
> -            printk(XENLOG_WARNING "Bogus DMIBAR %#"PRIx64" on %04x:%02x:%02x.%u\n",
> -                   bar, seg, bus, dev, func);
> +            printk(XENLOG_WARNING "Bogus DMIBAR %#"PRIx64" on %pp\n",
> +                   bar, &pdev->sbdf);
>           break;
>       }
>   }
> diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
> index 94a6e4eec9..68304a61e8 100644
> --- a/xen/drivers/passthrough/vtd/utils.c
> +++ b/xen/drivers/passthrough/vtd/utils.c
> @@ -95,9 +95,9 @@ void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn)
>       u64 *l, val;
>       u32 l_index, level;
>   
> -    printk("print_vtd_entries: iommu #%u dev %04x:%02x:%02x.%u gmfn %"PRI_gfn"\n",
> -           iommu->index, iommu->intel->drhd->segment, bus,
> -           PCI_SLOT(devfn), PCI_FUNC(devfn), gmfn);
> +    printk("print_vtd_entries: iommu #%u dev %pp gmfn %"PRI_gfn"\n",
> +           iommu->index, &PCI_SBDF3(iommu->intel->drhd->segment, bus, devfn),
> +           gmfn);
>   
>       if ( iommu->root_maddr == 0 )
>       {
> diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
> index 3eea7f89fc..dc0584b423 100644
> --- a/xen/drivers/passthrough/x86/ats.c
> +++ b/xen/drivers/passthrough/x86/ats.c
> @@ -31,8 +31,7 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
>       BUG_ON(!pos);
>   
>       if ( iommu_verbose )
> -        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS capability found\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_INFO, "%pp: ATS capability found\n", &pdev->sbdf);
>   
>       value = pci_conf_read16(pdev->sbdf, pos + ATS_REG_CTL);
>       if ( value & ATS_ENABLE )
> @@ -63,9 +62,8 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
>       }
>   
>       if ( iommu_verbose )
> -        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS %s enabled\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                pos ? "is" : "was");
> +        dprintk(XENLOG_INFO, "%pp: ATS %s enabled\n",
> +                &pdev->sbdf, pos ? "is" : "was");
>   
>       return pos;
>   }
> @@ -73,8 +71,6 @@ int enable_ats_device(struct pci_dev *pdev, struct list_head *ats_list)
>   void disable_ats_device(struct pci_dev *pdev)
>   {
>       u32 value;
> -    u16 seg = pdev->seg;
> -    u8 bus = pdev->bus, devfn = pdev->devfn;
>   
>       BUG_ON(!pdev->ats.cap_pos);
>   
> @@ -85,6 +81,5 @@ void disable_ats_device(struct pci_dev *pdev)
>       list_del(&pdev->ats.list);
>   
>       if ( iommu_verbose )
> -        dprintk(XENLOG_INFO, "%04x:%02x:%02x.%u: ATS is disabled\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +        dprintk(XENLOG_INFO, "%pp: ATS is disabled\n", &pdev->sbdf);
>   }
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 3c794f486d..ba9a036202 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -355,7 +355,6 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
>                         uint32_t val, void *data)
>   {
>       struct vpci_bar *bar = data;
> -    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
>       bool hi = false;
>   
>       if ( bar->type == VPCI_BAR_MEM64_HI )
> @@ -372,9 +371,8 @@ static void bar_write(const struct pci_dev *pdev, unsigned int reg,
>           /* If the value written is the current one avoid printing a warning. */
>           if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
>               gprintk(XENLOG_WARNING,
> -                    "%04x:%02x:%02x.%u: ignored BAR %lu write with memory decoding enabled\n",
> -                    pdev->seg, pdev->bus, slot, func,
> -                    bar - pdev->vpci->header.bars + hi);
> +                    "%pp: ignored BAR %lu write with memory decoding enabled\n",
> +                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
>           return;
>       }
>   
> @@ -402,15 +400,14 @@ 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->sbdf, PCI_COMMAND);
>       bool new_enabled = val & PCI_ROM_ADDRESS_ENABLE;
>   
>       if ( (cmd & PCI_COMMAND_MEMORY) && header->rom_enabled && new_enabled )
>       {
>           gprintk(XENLOG_WARNING,
> -                "%04x:%02x:%02x.%u: ignored ROM BAR write with memory decoding enabled\n",
> -                pdev->seg, pdev->bus, slot, func);
> +                "%pp: ignored ROM BAR write with memory decoding enabled\n",
> +                &pdev->sbdf);
>           return;
>       }
>   
> diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
> index 5b6602f3c2..40e4fca132 100644
> --- a/xen/drivers/vpci/msi.c
> +++ b/xen/drivers/vpci/msi.c
> @@ -290,8 +290,7 @@ void vpci_dump_msi(void)
>               msi = pdev->vpci->msi;
>               if ( msi && msi->enabled )
>               {
> -                printk("%04x:%02x:%02x.%u MSI\n", pdev->seg, pdev->bus,
> -                       PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +                printk("%pp MSI\n", &pdev->sbdf);
>   
>                   printk("  enabled: %d 64-bit: %d",
>                          msi->enabled, msi->address64);
> @@ -308,8 +307,7 @@ void vpci_dump_msi(void)
>               {
>                   int rc;
>   
> -                printk("%04x:%02x:%02x.%u MSI-X\n", pdev->seg, pdev->bus,
> -                       PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +                printk("%pp MSI-X\n", &pdev->sbdf);
>   
>                   printk("  entries: %u maskall: %d enabled: %d\n",
>                          msix->max_entries, msix->masked, msix->enabled);
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 38c1e7e5dd..64dd0a929c 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -42,15 +42,14 @@ static uint32_t control_read(const struct pci_dev *pdev, unsigned int reg,
>   static int update_entry(struct vpci_msix_entry *entry,
>                           const struct pci_dev *pdev, unsigned int nr)
>   {
> -    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
>       int rc = vpci_msix_arch_disable_entry(entry, pdev);
>   
>       /* Ignore ENOENT, it means the entry wasn't setup. */
>       if ( rc && rc != -ENOENT )
>       {
>           gprintk(XENLOG_WARNING,
> -                "%04x:%02x:%02x.%u: unable to disable entry %u for update: %d\n",
> -                pdev->seg, pdev->bus, slot, func, nr, rc);
> +                "%pp: unable to disable entry %u for update: %d\n",
> +                &pdev->sbdf, nr, rc);
>           return rc;
>       }
>   
> @@ -59,9 +58,8 @@ static int update_entry(struct vpci_msix_entry *entry,
>                                                         VPCI_MSIX_TABLE));
>       if ( rc )
>       {
> -        gprintk(XENLOG_WARNING,
> -                "%04x:%02x:%02x.%u: unable to enable entry %u: %d\n",
> -                pdev->seg, pdev->bus, slot, func, nr, rc);
> +        gprintk(XENLOG_WARNING, "%pp: unable to enable entry %u: %d\n",
> +                &pdev->sbdf, nr, rc);
>           /* Entry is likely not properly configured. */
>           return rc;
>       }
> @@ -72,7 +70,6 @@ static int update_entry(struct vpci_msix_entry *entry,
>   static void control_write(const struct pci_dev *pdev, unsigned int reg,
>                             uint32_t val, void *data)
>   {
> -    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
>       struct vpci_msix *msix = data;
>       bool new_masked = val & PCI_MSIX_FLAGS_MASKALL;
>       bool new_enabled = val & PCI_MSIX_FLAGS_ENABLE;
> @@ -133,9 +130,8 @@ static void control_write(const struct pci_dev *pdev, unsigned int reg,
>                   /* Ignore non-present entry. */
>                   break;
>               default:
> -                gprintk(XENLOG_WARNING,
> -                        "%04x:%02x:%02x.%u: unable to disable entry %u: %d\n",
> -                        pdev->seg, pdev->bus, slot, func, i, rc);
> +                gprintk(XENLOG_WARNING, "%pp: unable to disable entry %u: %d\n",
> +                        &pdev->sbdf, i, rc);
>                   return;
>               }
>           }
> @@ -180,8 +176,7 @@ static bool access_allowed(const struct pci_dev *pdev, unsigned long addr,
>           return true;
>   
>       gprintk(XENLOG_WARNING,
> -            "%04x:%02x:%02x.%u: unaligned or invalid size MSI-X table access\n",
> -            pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +            "%pp: unaligned or invalid size MSI-X table access\n", &pdev->sbdf);
>   
>       return false;
>   }
> @@ -431,10 +426,9 @@ int vpci_make_msix_hole(const struct pci_dev *pdev)
>               default:
>                   put_gfn(d, start);
>                   gprintk(XENLOG_WARNING,
> -                        "%04x:%02x:%02x.%u: existing mapping (mfn: %" PRI_mfn
> +                        "%pp: existing mapping (mfn: %" PRI_mfn
>                           "type: %d) at %#lx clobbers MSIX MMIO area\n",
> -                        pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> -                        PCI_FUNC(pdev->devfn), mfn_x(mfn), t, start);
> +                        &pdev->sbdf, mfn_x(mfn), t, start);
>                   return -EEXIST;
>               }
>               put_gfn(d, start);
> 

-- 
Julien Grall

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

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22  6:51 [Xen-devel] [PATCH v2] print: introduce a format specifier for pci_sbdf_t Roger Pau Monne
2019-08-26  8:42 ` Roger Pau Monné
2019-08-27 14:14 ` Jan Beulich
2019-09-19  9:15 ` Julien Grall

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox