diff mbox series

[4/5] print: introduce a format specifier for pci_sbdf_t

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

Commit Message

Roger Pau Monné May 10, 2019, 4:10 p.m. UTC 
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 expected.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 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    |  11 +-
 xen/drivers/passthrough/amd/iommu_intr.c    |   8 +-
 xen/drivers/passthrough/amd/pci_amd_iommu.c |  31 +++--
 xen/drivers/passthrough/pci.c               | 121 ++++++++------------
 xen/drivers/passthrough/vtd/dmar.c          |  26 ++---
 xen/drivers/passthrough/vtd/intremap.c      |  11 +-
 xen/drivers/passthrough/vtd/iommu.c         |  74 +++++-------
 xen/drivers/passthrough/vtd/quirks.c        |  23 ++--
 xen/drivers/passthrough/vtd/utils.c         |   6 +-
 xen/drivers/passthrough/x86/ats.c           |  13 +--
 xen/drivers/vpci/header.c                   |  10 +-
 xen/drivers/vpci/msi.c                      |   6 +-
 xen/drivers/vpci/msix.c                     |  25 ++--
 xen/include/xen/pci.h                       |   2 +
 21 files changed, 197 insertions(+), 265 deletions(-)

Comments

Jan Beulich May 24, 2019, 10:36 a.m. UTC | #1 
>>> On 10.05.19 at 18:10, <roger.pau@citrix.com> 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.

So on the positive side Linux doesn't use 'p' yet, so we're only at risk
of a future conflict. However, having to pass a 64-bit pointer just
to print a 32-bit entity seems rather wasteful to me. Since we can't
use entirely new format specifiers, did you consider (ab)using one
we rarely use, like %o, suffixed similarly like we do for %p? The
extension could be restricted to apply only when neither field width
nor precision nor any flags were specified, i.e. only to plain %o (at
least initially).

We'd then have something along the lines of

#define PRI_sbdf "op"
#define PRI_SBDF(v) ((v).sbdf)

and

    printk("%" PRI_sbdf ": ...\n", PRI_SBDF(pdev->sbdf), ...);

> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -392,6 +392,20 @@ static char *print_vcpu(char *str, 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, 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->func, 10, -1, -1, 0);

It shouldn't really matter, but may I suggest to use 8 instead of 10
here?

> @@ -519,6 +533,10 @@ static char *pointer(char *str, char *end, const char **fmt_ptr,
>      case 'v': /* d<domain-id>v<vcpu-id> from a struct vcpu */
>          ++*fmt_ptr;
>          return print_vcpu(str, end, arg);
> +
> +    case 'p': /* PCI SBDF. */
> +        ++*fmt_ptr;
> +        return print_pci_addr(str, end, arg);
>      }

Please insert at the alphabetically correct place.

> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -717,9 +717,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_T(seg, bdf), special->variety, special->handle);

The inefficiency of the%p-based approach is perhaps best seen with an
example like this: The compiler will have to instantiate an unnamed variable
on the stack to hold the value of the compound literal, just to be able to
take its address.

> @@ -900,14 +891,10 @@ int pci_release_devices(struct domain *d)
>          return ret;
>      }
>      while ( (pdev = pci_get_pdev_by_domain(d, -1, -1, -1)) )
> -    {
> -        bus = pdev->sbdf.bus;
> -        devfn = pdev->sbdf.extfunc;
> -        if ( deassign_device(d, pdev->sbdf.seg, bus, devfn) )
> -            printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!\n",
> -                   d->domain_id, pdev->sbdf.seg, bus,
> -                   PCI_SLOT(devfn), PCI_FUNC(devfn));
> -    }
> +        if ( deassign_device(d, pdev->sbdf.seg, pdev->sbdf.bus,
> +                             pdev->sbdf.extfunc) )
> +            printk("domain %d: deassign device (%pp) failed!\n",
> +                   d->domain_id, &pdev->sbdf);

Could you switch to %pd here (and elsewhere) at the same time?

Jan
Andrew Cooper May 24, 2019, 10:59 a.m. UTC | #2 
On 24/05/2019 11:36, Jan Beulich wrote:
>>>> On 10.05.19 at 18:10, <roger.pau@citrix.com> 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.
> So on the positive side Linux doesn't use 'p' yet, so we're only at risk
> of a future conflict. However, having to pass a 64-bit pointer just
> to print a 32-bit entity seems rather wasteful to me. Since we can't
> use entirely new format specifiers, did you consider (ab)using one
> we rarely use, like %o, suffixed similarly like we do for %p? The
> extension could be restricted to apply only when neither field width
> nor precision nor any flags were specified, i.e. only to plain %o (at
> least initially).
>
> We'd then have something along the lines of
>
> #define PRI_sbdf "op"
> #define PRI_SBDF(v) ((v).sbdf)
>
> and
>
>     printk("%" PRI_sbdf ": ...\n", PRI_SBDF(pdev->sbdf), ...);

Except the answer will be the same as every time you've asked this in
the past.

No, because -Wformat doesn't tolerate it.

The *only* flexibility we have to play with is suffixes to %p

~Andrew
Jan Beulich May 24, 2019, 11:16 a.m. UTC | #3 
>>> On 24.05.19 at 12:59, <andrew.cooper3@citrix.com> wrote:
> On 24/05/2019 11:36, Jan Beulich wrote:
>>>>> On 10.05.19 at 18:10, <roger.pau@citrix.com> 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.
>> So on the positive side Linux doesn't use 'p' yet, so we're only at risk
>> of a future conflict. However, having to pass a 64-bit pointer just
>> to print a 32-bit entity seems rather wasteful to me. Since we can't
>> use entirely new format specifiers, did you consider (ab)using one
>> we rarely use, like %o, suffixed similarly like we do for %p? The
>> extension could be restricted to apply only when neither field width
>> nor precision nor any flags were specified, i.e. only to plain %o (at
>> least initially).
>>
>> We'd then have something along the lines of
>>
>> #define PRI_sbdf "op"
>> #define PRI_SBDF(v) ((v).sbdf)
>>
>> and
>>
>>     printk("%" PRI_sbdf ": ...\n", PRI_SBDF(pdev->sbdf), ...);
> 
> Except the answer will be the same as every time you've asked this in
> the past.

I don't recall suggesting any use of %o so far. The one thing I
do recall suggesting (and which turned out bad) was using an
l modifier with %pb.

> No, because -Wformat doesn't tolerate it.

How would -Wformat choke here? %o accepts (unsigned) integers,
doesn't it?

Jan
Roger Pau Monné May 27, 2019, 3:48 p.m. UTC | #4 
On Fri, May 24, 2019 at 04:36:42AM -0600, Jan Beulich wrote:
> >>> On 10.05.19 at 18:10, <roger.pau@citrix.com> 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.
> 
> So on the positive side Linux doesn't use 'p' yet, so we're only at risk
> of a future conflict. However, having to pass a 64-bit pointer just
> to print a 32-bit entity seems rather wasteful to me.

I think there are two issues here, one that you mention is the waste
of using a 64bit pointer to pass a 32bit value, the other one would
be the unneeded pointer indirection.

I've thought about the same, but then realized that this is used
(always?) to output messages, which is in itself a slow operation, and
the pointer indirection is likely negligible compared to the cost of
the rest of the operation. The usage of 64bit is also wasteful, but
again this shouldn't be a hotpath anyway, and this code replaces the
usage of four parameters to print a SBDF into a single one.

> Since we can't
> use entirely new format specifiers, did you consider (ab)using one
> we rarely use, like %o, suffixed similarly like we do for %p? The
> extension could be restricted to apply only when neither field width
> nor precision nor any flags were specified, i.e. only to plain %o (at
> least initially).
> 
> We'd then have something along the lines of
> 
> #define PRI_sbdf "op"
> #define PRI_SBDF(v) ((v).sbdf)
> 
> and
> 
>     printk("%" PRI_sbdf ": ...\n", PRI_SBDF(pdev->sbdf), ...);

I have to admit this looks more hacky than my current suggestion IMO.
The %p formatter overloading seems more standard and expected rather
than overloading %o.

Plus, one thing I didn't realize, I think Xen could even use %pci to
print and SBDF, which will make it even clearer.

> > --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> > @@ -717,9 +717,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_T(seg, bdf), special->variety, special->handle);
> 
> The inefficiency of the%p-based approach is perhaps best seen with an
> example like this: The compiler will have to instantiate an unnamed variable
> on the stack to hold the value of the compound literal, just to be able to
> take its address.

Right, and such iommu debug is enabled or disabled at runtime, so
regardless of whether iommu debug is enabled or not you will end up
with such stack variable pointer.

In this specific example such usage is not that bad because that's a
boot time only message, but I'm sure there are others which are not
boot time only. The vast majority of messages however don't require
the usage of a literal.

Thanks, Roger.
Jan Beulich May 27, 2019, 3:58 p.m. UTC | #5 
>>> On 27.05.19 at 17:48, <roger.pau@citrix.com> wrote:
> On Fri, May 24, 2019 at 04:36:42AM -0600, Jan Beulich wrote:
>> Since we can't
>> use entirely new format specifiers, did you consider (ab)using one
>> we rarely use, like %o, suffixed similarly like we do for %p? The
>> extension could be restricted to apply only when neither field width
>> nor precision nor any flags were specified, i.e. only to plain %o (at
>> least initially).
>> 
>> We'd then have something along the lines of
>> 
>> #define PRI_sbdf "op"
>> #define PRI_SBDF(v) ((v).sbdf)
>> 
>> and
>> 
>>     printk("%" PRI_sbdf ": ...\n", PRI_SBDF(pdev->sbdf), ...);
> 
> I have to admit this looks more hacky than my current suggestion IMO.

Hmm, a matter of taste perhaps. I certainly consider constructs
like "&PCI_SBDF2_T(seg, bdf)" ugly/hacky enough. Taking
Andrew's position of wanting function-style macros to behave
function-like, this isn't even legal C then (because you can't
take the address of the result of a function call).

> The %p formatter overloading seems more standard and expected rather
> than overloading %o.

Well, it looked odd (to me at least) for %p in the beginning too, so
perhaps it's just a matter of getting used to it.

> Plus, one thing I didn't realize, I think Xen could even use %pci to
> print and SBDF, which will make it even clearer.

Documentation-wise - nice. But making every involved string literal
one character longer again.

Jan
diff mbox series

Patch

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 15cfe8d057..6f4641afbc 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -686,10 +686,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                     pdev->sbdf.func, 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;
@@ -743,9 +741,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                 pdev->sbdf.func, 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 ad4a72d56b..6832211772 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -432,8 +432,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;
@@ -991,11 +991,10 @@  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 Dom%d\n",
                        is_hardware_domain(currd)
                        ? XENLOG_WARNING "Potentially insecure"
-                       : XENLOG_ERR "Insecure",
-                       seg, bus, slot, func, d->domain_id);
+                       : XENLOG_ERR "Insecure", &dev->sbdf, d->domain_id);
             if ( !is_hardware_domain(d) &&
                  /* Assume a domain without memory has no mappings yet. */
                  (!is_hardware_domain(currd) || d->tot_pages) )
@@ -1050,18 +1049,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);
     }
 
@@ -1118,16 +1114,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);
     }
 
@@ -1175,8 +1170,8 @@  static void __pci_disable_msix(struct msi_desc *entry)
     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);
+               "cannot disable IRQ %d: masking MSI-X on %pp\n",
+               entry->irq, &dev->sbdf);
         maskall = true;
     }
     dev->msix->host_maskall = maskall;
@@ -1342,7 +1337,6 @@  int pci_restore_msi_state(struct pci_dev *pdev)
     struct msi_desc *entry, *tmp;
     struct irq_desc *desc;
     struct msi_msg msg;
-    uint8_t slot = pdev->sbdf.dev, func = pdev->sbdf.func;
     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->sbdf.seg, pdev->sbdf.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 352d43b425..b30ed08687 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -392,6 +392,20 @@  static char *print_vcpu(char *str, 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, 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->func, 10, -1, -1, 0);
+}
+
 static char *pointer(char *str, char *end, const char **fmt_ptr,
                      const void *arg, int field_width, int precision,
                      int flags)
@@ -519,6 +533,10 @@  static char *pointer(char *str, char *end, const char **fmt_ptr,
     case 'v': /* d<domain-id>v<vcpu-id> from a struct vcpu */
         ++*fmt_ptr;
         return print_vcpu(str, end, arg);
+
+    case 'p': /* PCI SBDF. */
+        ++*fmt_ptr;
+        return print_pci_addr(str, end, arg);
     }
 
     if ( field_width == -1 )
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 64d10481d7..d900feef09 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -717,9 +717,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_T(seg, bdf), special->variety, special->handle);
     add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);
 
     switch ( special->variety )
@@ -742,9 +741,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_T(seg, bdf));
             break;
         }
 
@@ -814,9 +813,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_T(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 82330c24ba..9bbc5a5545 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->sbdf.seg, pdev->sbdf.bus,
-                        pdev->sbdf.dev, pdev->sbdf.func);
+        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 e8d8ec59bd..91f5ea6bff 100644
--- a/xen/drivers/passthrough/amd/iommu_detect.c
+++ b/xen/drivers/passthrough/amd/iommu_detect.c
@@ -153,9 +153,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_T(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 fe0516f788..f39c67949f 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -793,9 +793,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_T(iommu->seg, iommu->bdf));
         return 0;
     }
     control = pci_conf_read16(iommu->msi.dev->sbdf,
@@ -838,9 +837,6 @@  static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
         .seg = iommu->seg,
         .bdf = iommu->bdf,
     };
-    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) ||
@@ -858,8 +854,7 @@  static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
 
     pci_conf_write32(sbdf, 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", &sbdf);
 
     /* Clear the enable writing bit */
     pci_conf_write32(sbdf, 0xf0, 0x90);
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 71594cc27d..eca8decc05 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -511,8 +511,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_T(seg, bdf));
     return ERR_PTR(-EINVAL);
 }
 
@@ -687,10 +686,7 @@  static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)
     if ( !ivrs_mapping )
         return 0;
 
-    printk("  %04x:%02x:%02x:%u:\n", 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_T(seg,ivrs_mapping->dte_requestor_id));
 
     spin_lock_irqsave(&(ivrs_mapping->intremap_lock), flags);
     dump_intremap_table(ivrs_mapping->intremap_table);
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 0e4c5b4994..5787dff914 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_T(seg, bdf));
 
             /* write iommu field last */
             ivrs_mappings[bdf].iommu = ivrs_mappings[bd0].iommu;
@@ -306,9 +305,9 @@  static int reassign_device(struct domain *source, struct domain *target,
     if ( !iommu )
     {
         AMD_IOMMU_DEBUG("Fail to find iommu."
-                        " %04x:%02x:%x02.%x cannot be assigned to dom%d\n",
-                        pdev->sbdf.seg, pdev->sbdf.bus, PCI_SLOT(devfn),
-                        PCI_FUNC(devfn), target->domain_id);
+                        " %pp cannot be assigned to dom%d\n",
+                        &PCI_SBDF3_T(pdev->sbdf.seg, pdev->sbdf.bus, devfn),
+                        target->domain_id);
         return -ENODEV;
     }
 
@@ -325,9 +324,9 @@  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->sbdf.seg, pdev->sbdf.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",
+                    &PCI_SBDF3_T(pdev->sbdf.seg, pdev->sbdf.bus, devfn),
+                    source->domain_id, target->domain_id);
 
     return 0;
 }
@@ -430,15 +429,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                            pdev->sbdf.func);
+            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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                        pdev->sbdf.func, 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;
     }
 
@@ -457,9 +453,8 @@  static int amd_iommu_remove_device(u8 devfn, struct pci_dev *pdev)
     if ( !iommu )
     {
         AMD_IOMMU_DEBUG("Fail to find iommu."
-                        " %04x:%02x:%02x.%u cannot be removed from dom%d\n",
-                        pdev->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                        pdev->sbdf.func, pdev->domain->domain_id);
+                        " %pp cannot be removed from dom%d\n",
+                        &pdev->sbdf, pdev->domain->domain_id);
         return -ENODEV;
     }
 
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 94fb3a183d..a9667ca21c 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -238,10 +238,6 @@  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)
-    uint16_t seg = pdev->sbdf.seg;
-    uint8_t bus = pdev->sbdf.bus;
-    uint8_t dev = pdev->sbdf.dev;
-    uint8_t func = pdev->sbdf.func;
     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);
         }
@@ -409,8 +404,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;
     }
 
@@ -642,9 +637,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.func, (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;
         }
@@ -674,7 +669,7 @@  int pci_add_device(u16 seg, u8 bus, u8 devfn,
 {
     struct pci_seg *pseg;
     struct pci_dev *pdev;
-    unsigned int slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
+    unsigned int func = PCI_FUNC(devfn);
     const char *pdev_type;
     int ret;
     bool pf_is_extfn = false;
@@ -747,9 +742,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(pdev->sbdf, idx, NULL, &pdev->vf_rlen[i],
@@ -762,9 +756,9 @@  int pci_add_device(u16 seg, u8 bus, u8 devfn,
         }
         else
             printk(XENLOG_WARNING
-                   "SR-IOV device %04x:%02x:%02x.%u has its virtual"
+                   "SR-IOV device %pp has its virtual"
                    " functions already enabled (%04x)\n",
-                   seg, bus, slot, func, ctrl);
+                   &pdev->sbdf, ctrl);
     }
 
     check_pdev(pdev);
@@ -791,15 +785,13 @@  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", &pdev->sbdf);
         }
     }
     return ret;
@@ -829,8 +821,8 @@  int pci_remove_device(u16 seg, u8 bus, u8 devfn)
                 list_del(&pdev->domain_list);
             pci_cleanup_msi(pdev);
             free_pdev(pseg, pdev);
-            printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+            printk(XENLOG_DEBUG "PCI remove device %pp\n",
+                   &PCI_SBDF3_T(seg, bus, devfn));
             break;
         }
 
@@ -889,7 +881,6 @@  static int pci_clean_dpci_irqs(struct domain *d)
 int pci_release_devices(struct domain *d)
 {
     struct pci_dev *pdev;
-    u8 bus, devfn;
     int ret;
 
     pcidevs_lock();
@@ -900,14 +891,10 @@  int pci_release_devices(struct domain *d)
         return ret;
     }
     while ( (pdev = pci_get_pdev_by_domain(d, -1, -1, -1)) )
-    {
-        bus = pdev->sbdf.bus;
-        devfn = pdev->sbdf.extfunc;
-        if ( deassign_device(d, pdev->sbdf.seg, bus, devfn) )
-            printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!\n",
-                   d->domain_id, pdev->sbdf.seg, bus,
-                   PCI_SLOT(devfn), PCI_FUNC(devfn));
-    }
+        if ( deassign_device(d, pdev->sbdf.seg, pdev->sbdf.bus,
+                             pdev->sbdf.extfunc) )
+            printk("domain %d: deassign device (%pp) failed!\n",
+                   d->domain_id, &pdev->sbdf);
     pcidevs_unlock();
 
     return 0;
@@ -1059,8 +1046,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_T(pseg->nr, bus, dev, func));
                     return -ENOMEM;
                 }
 
@@ -1100,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                   pdev->sbdf.func, 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->sbdf.extfunc )
                 return;
         }
@@ -1143,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 )
             {
@@ -1280,8 +1265,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->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.func,
+        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 )
@@ -1347,9 +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->sbdf.seg, pdev->sbdf.bus, PCI_SLOT(devfn),
-                   PCI_FUNC(devfn), rc);
+            printk(XENLOG_WARNING "IOMMU: add %pp failed (%d)\n",
+                   &pdev->sbdf, rc);
     }
 }
 
@@ -1393,9 +1377,8 @@  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->sbdf.seg, pdev->sbdf.bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-               rc);
+        printk(XENLOG_ERR "IOMMU: remove %pp failed (%d)\n",
+               &pdev->sbdf, rc);
         return rc;
     }
 
@@ -1465,9 +1448,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_T(seg, bus, devfn), rc);
     }
 
  done:
@@ -1503,8 +1485,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_T(seg, bus, devfn), ret);
         return ret;
     }
 
@@ -1513,9 +1495,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, &PCI_SBDF3_T(seg, bus, devfn));
         return ret;
     }
 
@@ -1590,10 +1571,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-               pdev->sbdf.func);
+        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 +1661,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_T(seg, bus, devfn));
                 ret = -EINVAL;
             }
             break;
@@ -1696,9 +1674,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_T(seg, bus, devfn), d->domain_id, ret);
 
         break;
 
@@ -1732,10 +1709,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_T(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 9d30188ab9..911494752f 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -367,18 +367,16 @@  static int __init acpi_parse_dev_scope(
             sec_bus = pci_conf_read8(sbdf, PCI_SECONDARY_BUS);
             sub_bus = pci_conf_read8(sbdf, 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,
-                       acpi_scope->bus, sec_bus, sub_bus);
+                printk(VTDPREFIX " bridge: %pp start=%x sec=%x sub=%x\n",
+                       &sbdf, acpi_scope->bus, sec_bus, sub_bus);
 
             dmar_scope_add_buses(scope, sec_bus, sub_bus);
             break;
         }
         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_T(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -399,8 +397,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_T(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -413,8 +411,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_T(seg, bus, path->dev, path->fn));
 
             if ( drhd )
             {
@@ -543,8 +541,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_T(drhd->segment, b, d, f));
         }
 
         acpi_register_drhd_unit(dmaru);
@@ -580,9 +578,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_T(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 0dbf99551e..bdf3758549 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 7b70732863..3942df04b5 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_T(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_T(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_T(seg, source_id), addr, iommu->reg);
         kind = "UNKNOWN";
         break;
     }
@@ -1354,10 +1351,8 @@  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),
+                printk(XENLOG_G_INFO VTDPREFIX "d%d: %pp owned by d%d!",
+                       domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn),
                        pdev->domain ? pdev->domain->domain_id : -1);
                 res = -EINVAL;
             }
@@ -1370,17 +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));
+                       "d%d: %pp mapped, but can't find owner!\n",
+                       domain->domain_id, &PCI_SBDF3_T(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),
+                       "d%d: %pp already mapped to d%d!",
+                       domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn),
                        cdomain);
                 res = -EINVAL;
             }
@@ -1496,9 +1489,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 "d%d:Hostbridge: skip %pp map\n",
+                   domain->domain_id, &pdev->sbdf);
         if ( !is_hardware_domain(domain) )
             return -EPERM;
         break;
@@ -1510,9 +1502,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 "d%d:PCIe: map %pp\n",
+                   domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn));
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
                                          pdev);
         if ( !ret && devfn == pdev->sbdf.extfunc && ats_device(pdev, drhd) > 0 )
@@ -1522,9 +1513,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 "d%d:PCI: map %pp\n",
+                   domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn));
 
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
                                          pdev);
@@ -1550,9 +1540,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, "d%d:unknown(%u): %pp\n",
+                domain->domain_id, pdev->type, &PCI_SBDF3_T(seg, bus, devfn));
         ret = -EINVAL;
         break;
     }
@@ -1648,9 +1637,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 "d%d:Hostbridge: skip %pp unmap\n",
+                   domain->domain_id, &pdev->sbdf);
         if ( !is_hardware_domain(domain) )
             return -EPERM;
         goto out;
@@ -1662,9 +1650,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 "d%d:PCIe: unmap %pp\n",
+                   domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn));
         ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( !ret && devfn == pdev->sbdf.extfunc && ats_device(pdev, drhd) > 0 )
             disable_ats_device(pdev);
@@ -1673,8 +1660,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 "d%d:PCI: unmap %pp\n",
+                   domain->domain_id, &PCI_SBDF3_T(seg, bus, devfn));
         ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( ret )
             break;
@@ -1699,9 +1686,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, "d%d:unknown(%u): %pp\n",
+                domain->domain_id, pdev->type, &PCI_SBDF3_T(seg, bus, devfn));
         ret = -EINVAL;
         goto out;
     }
@@ -2499,11 +2485,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"
+                   " It's %s to assign %pp"
                    " with shared RMRR at %"PRIx64" for Dom%d.\n",
                    relaxed ? XENLOG_WARNING : XENLOG_ERR,
                    relaxed ? "risky" : "disallowed",
-                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                   &PCI_SBDF3_T(seg, bus, devfn),
                    rmrr->base_address, d->domain_id);
             if ( !relaxed )
                 return -EPERM;
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 48c7384b82..0302c503fb 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -428,8 +428,6 @@  void pci_vtd_quirk(const struct pci_dev *pdev)
 {
     int seg = pdev->sbdf.seg;
     int bus = pdev->sbdf.bus;
-    int dev = pdev->sbdf.dev;
-    int func = pdev->sbdf.func;
     int pos;
     bool_t ff;
     u32 val, val2;
@@ -454,8 +452,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) */
@@ -490,8 +487,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;
         }
 
@@ -514,8 +510,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 */
@@ -540,16 +535,16 @@  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..5665bf7697 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_T(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 c3203793a6..cfd610229d 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;
-    uint16_t seg = pdev->sbdf.seg;
-    uint8_t bus = pdev->sbdf.bus, devfn = pdev->sbdf.extfunc;
 
     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 d693b1376c..ddda881366 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -371,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                    pdev->sbdf.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;
     }
 
@@ -407,9 +406,8 @@  static void rom_write(const struct pci_dev *pdev, unsigned int reg,
     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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                pdev->sbdf.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 4f2e55f3fd..2bdae48edf 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->sbdf.seg,
-                       pdev->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.func);
+                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->sbdf.seg,
-                       pdev->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.func);
+                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 47d569121f..1006b01f4b 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -48,9 +48,8 @@  static int update_entry(struct vpci_msix_entry *entry,
     if ( rc && rc != -ENOENT )
     {
         gprintk(XENLOG_WARNING,
-                "%04x:%02x:%02x.%u: unable to disable entry %u for update: %d\n",
-                pdev->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.func,
-                nr, rc);
+                "%pp: unable to disable entry %u for update: %d\n",
+                &pdev->sbdf, nr, rc);
         return rc;
     }
 
@@ -59,10 +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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.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;
     }
@@ -133,10 +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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                        pdev->sbdf.func, i, rc);
+                gprintk(XENLOG_WARNING, "%pp: unable to disable entry %u: %d\n",
+                        &pdev->sbdf, i, rc);
                 return;
             }
         }
@@ -181,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev, pdev->sbdf.func);
+            "%pp: unaligned or invalid size MSI-X table access\n", &pdev->sbdf);
 
     return false;
 }
@@ -432,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->sbdf.seg, pdev->sbdf.bus, pdev->sbdf.dev,
-                        pdev->sbdf.func, mfn_x(mfn), t, start);
+                        &pdev->sbdf, mfn_x(mfn), t, start);
                 return -EEXIST;
             }
             put_gfn(d, start);
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 9908d0fe5d..5e345c417f 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -60,6 +60,8 @@  typedef union {
 
 #define PCI_SBDF_T(s, b, d, f) \
     ((pci_sbdf_t) { .seg = (s), .bus = (b), .dev = (d), .func = (f) })
+#define PCI_SBDF2_T(s, b) \
+    ((pci_sbdf_t) { .seg = (s), .bdf = (b) })
 #define PCI_SBDF3_T(s, b, e) \
     ((pci_sbdf_t) { .seg = (s), .bus = (b), .extfunc = (e) })