Making these dependent upon "iommu=debug" isn't really helpful in the
field. Where touching respective code anyway also make use of %pp and
%pd.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While I'm adding AMD_IOMMU_VERBOSE(), there aren't any uses for now.
It's not really clear to me where to draw the boundary to
AMD_IOMMU_DEBUG().

I didn't bother touching iommu_guest.c here.
---
v8: New.

--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -203,6 +203,18 @@ struct acpi_ivrs_hardware;
 
 #define DMA_32BIT_MASK  0x00000000ffffffffULL
 
+#define AMD_IOMMU_ERROR(fmt, args...) \
+    printk(XENLOG_ERR "AMD-Vi: Error: " fmt, ## args)
+
+#define AMD_IOMMU_WARN(fmt, args...) \
+    printk(XENLOG_WARNING "AMD-Vi: Warning: " fmt, ## args)
+
+#define AMD_IOMMU_VERBOSE(fmt, args...) \
+    do { \
+        if ( iommu_verbose ) \
+            printk(XENLOG_INFO "AMD-Vi: " fmt, ## args); \
+    } while ( false )
+
 #define AMD_IOMMU_DEBUG(fmt, args...) \
     do  \
     {   \
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -174,7 +174,7 @@ static int __init reserve_unity_map_for_
         if ( unity_map->addr + unity_map->length > base &&
              base + length > unity_map->addr )
         {
-            AMD_IOMMU_DEBUG("IVMD Error: overlap [%lx,%lx) vs [%lx,%lx)\n",
+            AMD_IOMMU_ERROR("IVMD: overlap [%lx,%lx) vs [%lx,%lx)\n",
                             base, base + length, unity_map->addr,
                             unity_map->addr + unity_map->length);
             return -EPERM;
@@ -248,7 +248,7 @@ static int __init register_range_for_dev
     iommu = find_iommu_for_device(seg, bdf);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: No IOMMU for Dev_Id %#x!\n", bdf);
+        AMD_IOMMU_ERROR("IVMD: no IOMMU for Dev_Id %#x\n", bdf);
         return -ENODEV;
     }
     req = ivrs_mappings[bdf].dte_requestor_id;
@@ -318,7 +318,7 @@ static int __init parse_ivmd_device_sele
     bdf = ivmd_block->header.device_id;
     if ( bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: Invalid Dev_Id %#x\n", bdf);
+        AMD_IOMMU_ERROR("IVMD: invalid Dev_Id %#x\n", bdf);
         return -ENODEV;
     }
 
@@ -335,16 +335,14 @@ static int __init parse_ivmd_device_rang
     first_bdf = ivmd_block->header.device_id;
     if ( first_bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: "
-                        "Invalid Range_First Dev_Id %#x\n", first_bdf);
+        AMD_IOMMU_ERROR("IVMD: invalid Range_First Dev_Id %#x\n", first_bdf);
         return -ENODEV;
     }
 
     last_bdf = ivmd_block->aux_data;
     if ( (last_bdf >= ivrs_bdf_entries) || (last_bdf <= first_bdf) )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: "
-                        "Invalid Range_Last Dev_Id %#x\n", last_bdf);
+        AMD_IOMMU_ERROR("IVMD: invalid Range_Last Dev_Id %#x\n", last_bdf);
         return -ENODEV;
     }
 
@@ -367,7 +365,7 @@ static int __init parse_ivmd_device_iomm
                                     ivmd_block->aux_data);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: No IOMMU for Dev_Id %#x Cap %#x\n",
+        AMD_IOMMU_ERROR("IVMD: no IOMMU for Dev_Id %#x Cap %#x\n",
                         ivmd_block->header.device_id, ivmd_block->aux_data);
         return -ENODEV;
     }
@@ -384,7 +382,7 @@ static int __init parse_ivmd_block(const
 
     if ( ivmd_block->header.length < sizeof(*ivmd_block) )
     {
-        AMD_IOMMU_DEBUG("IVMD Error: Invalid Block Length!\n");
+        AMD_IOMMU_ERROR("IVMD: invalid block length\n");
         return -ENODEV;
     }
 
@@ -402,8 +400,8 @@ static int __init parse_ivmd_block(const
          (addr_bits < BITS_PER_LONG &&
           ((start_addr + mem_length - 1) >> addr_bits)) )
     {
-        AMD_IOMMU_DEBUG("IVMD: [%lx,%lx) is not IOMMU addressable\n",
-                        start_addr, start_addr + mem_length);
+        AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not IOMMU addressable\n",
+                       start_addr, start_addr + mem_length);
         return 0;
     }
 
@@ -411,8 +409,8 @@ static int __init parse_ivmd_block(const
     {
         paddr_t addr;
 
-        AMD_IOMMU_DEBUG("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n",
-                        base, limit + PAGE_SIZE);
+        AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n",
+                       base, limit + PAGE_SIZE);
 
         for ( addr = base; addr <= limit; addr += PAGE_SIZE )
         {
@@ -423,7 +421,7 @@ static int __init parse_ivmd_block(const
                 if ( e820_add_range(&e820, addr, addr + PAGE_SIZE,
                                     E820_RESERVED) )
                     continue;
-                AMD_IOMMU_DEBUG("IVMD Error: Page at %lx couldn't be reserved\n",
+                AMD_IOMMU_ERROR("IVMD: page at %lx couldn't be reserved\n",
                                 addr);
                 return -EIO;
             }
@@ -433,8 +431,7 @@ static int __init parse_ivmd_block(const
                            RAM_TYPE_UNUSABLE)) )
                 continue;
 
-            AMD_IOMMU_DEBUG("IVMD Error: Page at %lx can't be converted\n",
-                            addr);
+            AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr);
             return -EIO;
         }
     }
@@ -448,7 +445,7 @@ static int __init parse_ivmd_block(const
     }
     else
     {
-        AMD_IOMMU_DEBUG("IVMD Error: Invalid Flag Field!\n");
+        AMD_IOMMU_ERROR("IVMD: invalid flag field\n");
         return -ENODEV;
     }
 
@@ -471,7 +468,8 @@ static int __init parse_ivmd_block(const
                                        iw, ir, exclusion);
 
     default:
-        AMD_IOMMU_DEBUG("IVMD Error: Invalid Block Type!\n");
+        AMD_IOMMU_ERROR("IVMD: unknown block type %#x\n",
+                        ivmd_block->header.type);
         return -ENODEV;
     }
 }
@@ -481,7 +479,7 @@ static u16 __init parse_ivhd_device_padd
 {
     if ( header_length < (block_length + pad_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
@@ -496,7 +494,7 @@ static u16 __init parse_ivhd_device_sele
     bdf = select->header.id;
     if ( bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Dev_Id %#x\n", bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf);
         return 0;
     }
 
@@ -515,14 +513,13 @@ static u16 __init parse_ivhd_device_rang
     dev_length = sizeof(*range);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     if ( range->end.header.type != ACPI_IVRS_TYPE_END )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: End_Type %#x\n",
+        AMD_IOMMU_ERROR("IVHD Error: invalid range: End_Type %#x\n",
                         range->end.header.type);
         return 0;
     }
@@ -530,16 +527,14 @@ static u16 __init parse_ivhd_device_rang
     first_bdf = range->start.header.id;
     if ( first_bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: First Dev_Id %#x\n", first_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: First Dev_Id %#x\n", first_bdf);
         return 0;
     }
 
     last_bdf = range->end.header.id;
     if ( (last_bdf >= ivrs_bdf_entries) || (last_bdf <= first_bdf) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: Last Dev_Id %#x\n", last_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: Last Dev_Id %#x\n", last_bdf);
         return 0;
     }
 
@@ -561,21 +556,21 @@ static u16 __init parse_ivhd_device_alia
     dev_length = sizeof(*alias);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     bdf = alias->header.id;
     if ( bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Dev_Id %#x\n", bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf);
         return 0;
     }
 
     alias_id = alias->used_id;
     if ( alias_id >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Alias Dev_Id %#x\n", alias_id);
+        AMD_IOMMU_ERROR("IVHD: invalid Alias Dev_Id %#x\n", alias_id);
         return 0;
     }
 
@@ -597,14 +592,13 @@ static u16 __init parse_ivhd_device_alia
     dev_length = sizeof(*range);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     if ( range->end.header.type != ACPI_IVRS_TYPE_END )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: End_Type %#x\n",
+        AMD_IOMMU_ERROR("IVHD: invalid range: End_Type %#x\n",
                         range->end.header.type);
         return 0;
     }
@@ -612,16 +606,14 @@ static u16 __init parse_ivhd_device_alia
     first_bdf = range->alias.header.id;
     if ( first_bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: First Dev_Id %#x\n", first_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: First Dev_Id %#x\n", first_bdf);
         return 0;
     }
 
     last_bdf = range->end.header.id;
     if ( last_bdf >= ivrs_bdf_entries || last_bdf <= first_bdf )
     {
-        AMD_IOMMU_DEBUG(
-            "IVHD Error: Invalid Range: Last Dev_Id %#x\n", last_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: Last Dev_Id %#x\n", last_bdf);
         return 0;
     }
 
@@ -651,14 +643,14 @@ static u16 __init parse_ivhd_device_exte
     dev_length = sizeof(*ext);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     bdf = ext->header.id;
     if ( bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Dev_Id %#x\n", bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf);
         return 0;
     }
 
@@ -677,14 +669,13 @@ static u16 __init parse_ivhd_device_exte
     dev_length = sizeof(*range);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     if ( range->end.header.type != ACPI_IVRS_TYPE_END )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: End_Type %#x\n",
+        AMD_IOMMU_ERROR("IVHD: invalid range: End_Type %#x\n",
                         range->end.header.type);
         return 0;
     }
@@ -692,16 +683,14 @@ static u16 __init parse_ivhd_device_exte
     first_bdf = range->extended.header.id;
     if ( first_bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: First Dev_Id %#x\n", first_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: First Dev_Id %#x\n", first_bdf);
         return 0;
     }
 
     last_bdf = range->end.header.id;
     if ( (last_bdf >= ivrs_bdf_entries) || (last_bdf <= first_bdf) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: "
-                        "Invalid Range: Last Dev_Id %#x\n", last_bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid range: Last Dev_Id %#x\n", last_bdf);
         return 0;
     }
 
@@ -789,14 +778,14 @@ static u16 __init parse_ivhd_device_spec
     dev_length = sizeof(*special);
     if ( header_length < (block_length + dev_length) )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
         return 0;
     }
 
     bdf = special->used_id;
     if ( bdf >= ivrs_bdf_entries )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Dev_Id %#x\n", bdf);
+        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf);
         return 0;
     }
 
@@ -844,12 +833,12 @@ static u16 __init parse_ivhd_device_spec
             {
                 if ( ioapic_sbdf[idx].bdf == bdf &&
                      ioapic_sbdf[idx].seg == seg )
-                    AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC %#x entries\n",
+                    AMD_IOMMU_WARN("IVHD: duplicate IO-APIC %#x entries\n",
                                     special->handle);
                 else
                 {
-                    printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC %#x entries\n",
-                           special->handle);
+                    AMD_IOMMU_ERROR("IVHD: conflicting IO-APIC %#x entries\n",
+                                    special->handle);
                     if ( amd_iommu_perdev_intremap )
                         return 0;
                 }
@@ -944,7 +933,7 @@ static int __init parse_ivhd_block(const
 
     if ( ivhd_block->header.length < hdr_size )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Block Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid block length\n");
         return -ENODEV;
     }
 
@@ -953,7 +942,7 @@ static int __init parse_ivhd_block(const
                                     ivhd_block->capability_offset);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: No IOMMU for Dev_Id %#x Cap %#x\n",
+        AMD_IOMMU_ERROR("IVHD: no IOMMU for Dev_Id %#x Cap %#x\n",
                         ivhd_block->header.device_id,
                         ivhd_block->capability_offset);
         return -ENODEV;
@@ -1016,7 +1005,8 @@ static int __init parse_ivhd_block(const
                 ivhd_block->header.length, block_length, iommu);
             break;
         default:
-            AMD_IOMMU_DEBUG("IVHD Error: Invalid Device Type!\n");
+            AMD_IOMMU_WARN("IVHD: unknown device type %#x\n",
+                           ivhd_device->header.type);
             dev_length = 0;
             break;
         }
@@ -1113,8 +1103,7 @@ static int __init parse_ivrs_table(struc
 
         if ( table->length < (length + ivrs_block->length) )
         {
-            AMD_IOMMU_DEBUG("IVRS Error: "
-                            "Table Length Exceeded: %#x -> %#lx\n",
+            AMD_IOMMU_ERROR("IVRS: table length exceeded: %#x -> %#lx\n",
                             table->length,
                             (length + ivrs_block->length));
             return -ENODEV;
@@ -1214,7 +1203,7 @@ static int __init get_last_bdf_ivhd(
 
     if ( ivhd_block->header.length < hdr_size )
     {
-        AMD_IOMMU_DEBUG("IVHD Error: Invalid Block Length!\n");
+        AMD_IOMMU_ERROR("IVHD: invalid block length\n");
         return -ENODEV;
     }
 
@@ -1261,7 +1250,8 @@ static int __init get_last_bdf_ivhd(
             dev_length = sizeof(ivhd_device->special);
             break;
         default:
-            AMD_IOMMU_DEBUG("IVHD Error: Invalid Device Type!\n");
+            AMD_IOMMU_WARN("IVHD: unknown device type %#x\n",
+                           ivhd_device->header.type);
             dev_length = 0;
             break;
         }
@@ -1327,7 +1317,7 @@ get_supported_ivhd_type(struct acpi_tabl
     checksum = acpi_tb_checksum(ACPI_CAST_PTR(uint8_t, table), table->length);
     if ( checksum )
     {
-        AMD_IOMMU_DEBUG("IVRS Error: Invalid Checksum %#x\n", checksum);
+        AMD_IOMMU_ERROR("IVRS: invalid checksum %#x\n", checksum);
         return -ENODEV;
     }
 
@@ -1340,8 +1330,7 @@ get_supported_ivhd_type(struct acpi_tabl
 
         if ( table->length < (length + ivrs_block->length) )
         {
-            AMD_IOMMU_DEBUG("IVRS Error: "
-                            "Table Length Exceeded: %#x -> %#lx\n",
+            AMD_IOMMU_ERROR("IVRS: table length exceeded: %#x -> %#lx\n",
                             table->length,
                             (length + ivrs_block->length));
             return -ENODEV;
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -291,8 +291,7 @@ void amd_iommu_flush_iotlb(u8 devfn, con
 
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("%s: Can't find iommu for %pp\n",
-                        __func__, &pdev->sbdf);
+        AMD_IOMMU_WARN("can't find IOMMU for %pp\n", &pdev->sbdf);
         return;
     }
 
--- a/xen/drivers/passthrough/amd/iommu_detect.c
+++ b/xen/drivers/passthrough/amd/iommu_detect.c
@@ -141,21 +141,21 @@ int __init amd_iommu_detect_one_acpi(
 
     if ( ivhd_block->header.length < sizeof(*ivhd_block) )
     {
-        AMD_IOMMU_DEBUG("Invalid IVHD Block Length!\n");
+        AMD_IOMMU_ERROR("invalid IVHD block length\n");
         return -ENODEV;
     }
 
     if ( !ivhd_block->header.device_id ||
         !ivhd_block->capability_offset || !ivhd_block->base_address)
     {
-        AMD_IOMMU_DEBUG("Invalid IVHD Block!\n");
+        AMD_IOMMU_ERROR("invalid IVHD block\n");
         return -ENODEV;
     }
 
     iommu = xzalloc(struct amd_iommu);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("Error allocating amd_iommu\n");
+        AMD_IOMMU_ERROR("cannot allocate amd_iommu\n");
         return -ENOMEM;
     }
 
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -386,8 +386,8 @@ static void iommu_reset_log(struct amd_i
 
     if ( log_run )
     {
-        AMD_IOMMU_DEBUG("Warning: Log Run bit %d is not cleared"
-                        "before reset!\n", run_bit);
+        AMD_IOMMU_WARN("Log Run bit %d is not cleared before reset\n",
+                       run_bit);
         return;
     }
 
@@ -754,8 +754,8 @@ static bool_t __init set_iommu_interrupt
     pcidevs_unlock();
     if ( !iommu->msi.dev )
     {
-        AMD_IOMMU_DEBUG("IOMMU: no pdev for %pp\n",
-                        &PCI_SBDF2(iommu->seg, iommu->bdf));
+        AMD_IOMMU_WARN("no pdev for %pp\n",
+                       &PCI_SBDF2(iommu->seg, iommu->bdf));
         return 0;
     }
 
@@ -799,7 +799,7 @@ static bool_t __init set_iommu_interrupt
     if ( ret )
     {
         destroy_irq(irq);
-        AMD_IOMMU_DEBUG("can't request irq\n");
+        AMD_IOMMU_ERROR("can't request irq\n");
         return 0;
     }
 
@@ -992,7 +992,7 @@ static void *__init allocate_buffer(unsi
 
     if ( buffer == NULL )
     {
-        AMD_IOMMU_DEBUG("Error allocating %s\n", name);
+        AMD_IOMMU_ERROR("cannot allocate %s\n", name);
         return NULL;
     }
 
@@ -1224,7 +1224,7 @@ static int __init alloc_ivrs_mappings(u1
     ivrs_mappings = xzalloc_array(struct ivrs_mappings, ivrs_bdf_entries + 1);
     if ( ivrs_mappings == NULL )
     {
-        AMD_IOMMU_DEBUG("Error allocating IVRS Mappings table\n");
+        AMD_IOMMU_ERROR("cannot allocate IVRS Mappings table\n");
         return -ENOMEM;
     }
     IVRS_MAPPINGS_SEG(ivrs_mappings) = seg;
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -377,8 +377,8 @@ void amd_iommu_ioapic_update_ire(
     iommu = find_iommu_for_device(seg, bdf);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("Fail to find iommu for ioapic device id ="
-                        " %04x:%04x\n", seg, bdf);
+        AMD_IOMMU_WARN("failed to find IOMMU for IO-APIC @ %04x:%04x\n",
+                       seg, bdf);
         __io_apic_write(apic, reg, value);
         return;
     }
@@ -747,8 +747,8 @@ bool __init iov_supports_xt(void)
         if ( !find_iommu_for_device(ioapic_sbdf[idx].seg,
                                     ioapic_sbdf[idx].bdf) )
         {
-            AMD_IOMMU_DEBUG("No IOMMU for IO-APIC %#x (ID %x)\n",
-                            apic, IO_APIC_ID(apic));
+            AMD_IOMMU_WARN("no IOMMU for IO-APIC %#x (ID %x)\n",
+                           apic, IO_APIC_ID(apic));
             return false;
         }
     }
@@ -765,14 +765,12 @@ int __init amd_setup_hpet_msi(struct msi
 
     if ( hpet_sbdf.init == HPET_NONE )
     {
-        AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping."
-                        " Missing IVRS HPET info.\n");
+        AMD_IOMMU_ERROR("failed to setup HPET MSI remapping: missing IVRS HPET info\n");
         return -ENODEV;
     }
     if ( msi_desc->hpet_id != hpet_sbdf.id )
     {
-        AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping."
-                        " Wrong HPET.\n");
+        AMD_IOMMU_ERROR("failed to setup HPET MSI remapping: wrong HPET\n");
         return -ENODEV;
     }
 
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -222,7 +222,7 @@ static int iommu_pde_from_dfn(struct dom
             table = iommu_alloc_pgtable(d);
             if ( table == NULL )
             {
-                AMD_IOMMU_DEBUG("Cannot allocate I/O page table\n");
+                AMD_IOMMU_ERROR("cannot allocate I/O page table\n");
                 unmap_domain_page(next_table_vaddr);
                 return 1;
             }
@@ -252,7 +252,7 @@ static int iommu_pde_from_dfn(struct dom
                 table = iommu_alloc_pgtable(d);
                 if ( table == NULL )
                 {
-                    AMD_IOMMU_DEBUG("Cannot allocate I/O page table\n");
+                    AMD_IOMMU_ERROR("cannot allocate I/O page table\n");
                     unmap_domain_page(next_table_vaddr);
                     return 1;
                 }
@@ -301,7 +301,7 @@ int amd_iommu_map_page(struct domain *d,
     if ( rc )
     {
         spin_unlock(&hd->arch.mapping_lock);
-        AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %"PRI_dfn"\n",
+        AMD_IOMMU_ERROR("root table alloc failed, dfn = %"PRI_dfn"\n",
                         dfn_x(dfn));
         domain_crash(d);
         return rc;
@@ -310,7 +310,7 @@ int amd_iommu_map_page(struct domain *d,
     if ( iommu_pde_from_dfn(d, dfn_x(dfn), &pt_mfn, true) || !pt_mfn )
     {
         spin_unlock(&hd->arch.mapping_lock);
-        AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
+        AMD_IOMMU_ERROR("invalid IO pagetable entry dfn = %"PRI_dfn"\n",
                         dfn_x(dfn));
         domain_crash(d);
         return -EFAULT;
@@ -343,7 +343,7 @@ int amd_iommu_unmap_page(struct domain *
     if ( iommu_pde_from_dfn(d, dfn_x(dfn), &pt_mfn, false) )
     {
         spin_unlock(&hd->arch.mapping_lock);
-        AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
+        AMD_IOMMU_ERROR("invalid IO pagetable entry dfn = %"PRI_dfn"\n",
                         dfn_x(dfn));
         domain_crash(d);
         return -EFAULT;
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -367,10 +367,8 @@ static int reassign_device(struct domain
     iommu = find_iommu_for_device(pdev->seg, bdf);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("Fail to find iommu."
-                        " %04x:%02x:%x02.%x cannot be assigned to dom%d\n",
-                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                        target->domain_id);
+        AMD_IOMMU_WARN("failed to find IOMMU: %pp cannot be assigned to %pd\n",
+                       &pdev->sbdf, target);
         return -ENODEV;
     }
 
@@ -484,8 +482,8 @@ static int amd_iommu_add_device(u8 devfn
             return 0;
         }
 
-        AMD_IOMMU_DEBUG("No iommu for %pp; cannot be handed to d%d\n",
-                        &pdev->sbdf, pdev->domain->domain_id);
+        AMD_IOMMU_WARN("no IOMMU for %pp; cannot be handed to %pd\n",
+                        &pdev->sbdf, pdev->domain);
         return -ENODEV;
     }
 
@@ -527,9 +525,8 @@ static int amd_iommu_add_device(u8 devfn
              pdev->domain,
              ivrs_mappings[ivrs_mappings[bdf].dte_requestor_id].unity_map,
              0) )
-        AMD_IOMMU_DEBUG("%pd: unity mapping failed for %04x:%02x:%02x.%u\n",
-                        pdev->domain, pdev->seg, pdev->bus, PCI_SLOT(devfn),
-                        PCI_FUNC(devfn));
+        AMD_IOMMU_WARN("%pd: unity mapping failed for %pp\n",
+                       pdev->domain, &pdev->sbdf);
 
     return amd_iommu_setup_domain_device(pdev->domain, iommu, devfn, pdev);
 }
@@ -547,7 +544,7 @@ static int amd_iommu_remove_device(u8 de
     iommu = find_iommu_for_device(pdev->seg, bdf);
     if ( !iommu )
     {
-        AMD_IOMMU_DEBUG("Fail to find iommu. %pp cannot be removed from %pd\n",
+        AMD_IOMMU_WARN("failed to find IOMMU: %pp cannot be removed from %pd\n",
                         &pdev->sbdf, pdev->domain);
         return -ENODEV;
     }
@@ -560,9 +557,8 @@ static int amd_iommu_remove_device(u8 de
     if ( amd_iommu_reserve_domain_unity_unmap(
              pdev->domain,
              ivrs_mappings[ivrs_mappings[bdf].dte_requestor_id].unity_map) )
-        AMD_IOMMU_DEBUG("%pd: unity unmapping failed for %04x:%02x:%02x.%u\n",
-                        pdev->domain, pdev->seg, pdev->bus, PCI_SLOT(devfn),
-                        PCI_FUNC(devfn));
+        AMD_IOMMU_WARN("%pd: unity unmapping failed for %pp\n",
+                       pdev->domain, &pdev->sbdf);
 
     if ( amd_iommu_perdev_intremap &&
          ivrs_mappings[bdf].dte_requestor_id == bdf &&

On 22/09/2021 15:38, Jan Beulich wrote:
> Making these dependent upon "iommu=debug" isn't really helpful in the
> field. Where touching respective code anyway also make use of %pp and
> %pd.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul@xen.org>

... with one nit below...

> ---
[snip]
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -174,7 +174,7 @@ static int __init reserve_unity_map_for_
>           if ( unity_map->addr + unity_map->length > base &&
>                base + length > unity_map->addr )
>           {
> -            AMD_IOMMU_DEBUG("IVMD Error: overlap [%lx,%lx) vs [%lx,%lx)\n",
> +            AMD_IOMMU_ERROR("IVMD: overlap [%lx,%lx) vs [%lx,%lx)\n",
>                               base, base + length, unity_map->addr,
>                               unity_map->addr + unity_map->length);
>               return -EPERM;
> @@ -248,7 +248,7 @@ static int __init register_range_for_dev
>       iommu = find_iommu_for_device(seg, bdf);
>       if ( !iommu )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: No IOMMU for Dev_Id %#x!\n", bdf);
> +        AMD_IOMMU_ERROR("IVMD: no IOMMU for Dev_Id %#x\n", bdf);
>           return -ENODEV;
>       }
>       req = ivrs_mappings[bdf].dte_requestor_id;
> @@ -318,7 +318,7 @@ static int __init parse_ivmd_device_sele
>       bdf = ivmd_block->header.device_id;
>       if ( bdf >= ivrs_bdf_entries )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: Invalid Dev_Id %#x\n", bdf);
> +        AMD_IOMMU_ERROR("IVMD: invalid Dev_Id %#x\n", bdf);
>           return -ENODEV;
>       }
>   
> @@ -335,16 +335,14 @@ static int __init parse_ivmd_device_rang
>       first_bdf = ivmd_block->header.device_id;
>       if ( first_bdf >= ivrs_bdf_entries )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: "
> -                        "Invalid Range_First Dev_Id %#x\n", first_bdf);
> +        AMD_IOMMU_ERROR("IVMD: invalid Range_First Dev_Id %#x\n", first_bdf);
>           return -ENODEV;
>       }
>   
>       last_bdf = ivmd_block->aux_data;
>       if ( (last_bdf >= ivrs_bdf_entries) || (last_bdf <= first_bdf) )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: "
> -                        "Invalid Range_Last Dev_Id %#x\n", last_bdf);
> +        AMD_IOMMU_ERROR("IVMD: invalid Range_Last Dev_Id %#x\n", last_bdf);
>           return -ENODEV;
>       }
>   
> @@ -367,7 +365,7 @@ static int __init parse_ivmd_device_iomm
>                                       ivmd_block->aux_data);
>       if ( !iommu )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: No IOMMU for Dev_Id %#x Cap %#x\n",
> +        AMD_IOMMU_ERROR("IVMD: no IOMMU for Dev_Id %#x Cap %#x\n",
>                           ivmd_block->header.device_id, ivmd_block->aux_data);
>           return -ENODEV;
>       }
> @@ -384,7 +382,7 @@ static int __init parse_ivmd_block(const
>   
>       if ( ivmd_block->header.length < sizeof(*ivmd_block) )
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: Invalid Block Length!\n");
> +        AMD_IOMMU_ERROR("IVMD: invalid block length\n");
>           return -ENODEV;
>       }
>   
> @@ -402,8 +400,8 @@ static int __init parse_ivmd_block(const
>            (addr_bits < BITS_PER_LONG &&
>             ((start_addr + mem_length - 1) >> addr_bits)) )
>       {
> -        AMD_IOMMU_DEBUG("IVMD: [%lx,%lx) is not IOMMU addressable\n",
> -                        start_addr, start_addr + mem_length);
> +        AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not IOMMU addressable\n",
> +                       start_addr, start_addr + mem_length);
>           return 0;
>       }
>   
> @@ -411,8 +409,8 @@ static int __init parse_ivmd_block(const
>       {
>           paddr_t addr;
>   
> -        AMD_IOMMU_DEBUG("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n",
> -                        base, limit + PAGE_SIZE);
> +        AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n",
> +                       base, limit + PAGE_SIZE);
>   
>           for ( addr = base; addr <= limit; addr += PAGE_SIZE )
>           {
> @@ -423,7 +421,7 @@ static int __init parse_ivmd_block(const
>                   if ( e820_add_range(&e820, addr, addr + PAGE_SIZE,
>                                       E820_RESERVED) )
>                       continue;
> -                AMD_IOMMU_DEBUG("IVMD Error: Page at %lx couldn't be reserved\n",
> +                AMD_IOMMU_ERROR("IVMD: page at %lx couldn't be reserved\n",
>                                   addr);
>                   return -EIO;
>               }
> @@ -433,8 +431,7 @@ static int __init parse_ivmd_block(const
>                              RAM_TYPE_UNUSABLE)) )
>                   continue;
>   
> -            AMD_IOMMU_DEBUG("IVMD Error: Page at %lx can't be converted\n",
> -                            addr);
> +            AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr);
>               return -EIO;
>           }
>       }
> @@ -448,7 +445,7 @@ static int __init parse_ivmd_block(const
>       }
>       else
>       {
> -        AMD_IOMMU_DEBUG("IVMD Error: Invalid Flag Field!\n");
> +        AMD_IOMMU_ERROR("IVMD: invalid flag field\n");
>           return -ENODEV;
>       }
>   
> @@ -471,7 +468,8 @@ static int __init parse_ivmd_block(const
>                                          iw, ir, exclusion);
>   
>       default:
> -        AMD_IOMMU_DEBUG("IVMD Error: Invalid Block Type!\n");
> +        AMD_IOMMU_ERROR("IVMD: unknown block type %#x\n",
> +                        ivmd_block->header.type);
>           return -ENODEV;
>       }
>   }
> @@ -481,7 +479,7 @@ static u16 __init parse_ivhd_device_padd
>   {
>       if ( header_length < (block_length + pad_length) )
>       {
> -        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
> +        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
>           return 0;
>       }
>   
> @@ -496,7 +494,7 @@ static u16 __init parse_ivhd_device_sele
>       bdf = select->header.id;
>       if ( bdf >= ivrs_bdf_entries )
>       {
> -        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Dev_Id %#x\n", bdf);
> +        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf);
>           return 0;
>       }
>   
> @@ -515,14 +513,13 @@ static u16 __init parse_ivhd_device_rang
>       dev_length = sizeof(*range);
>       if ( header_length < (block_length + dev_length) )
>       {
> -        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
> +        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
>           return 0;
>       }
>   
>       if ( range->end.header.type != ACPI_IVRS_TYPE_END )
>       {
> -        AMD_IOMMU_DEBUG("IVHD Error: "
> -                        "Invalid Range: End_Type %#x\n",
> +        AMD_IOMMU_ERROR("IVHD Error: invalid range: End_Type %#x\n",

NIT: I guess you want to drop the 'Error' here like you did elsewhere.

On 28.09.2021 09:42, Durrant, Paul wrote:
> On 22/09/2021 15:38, Jan Beulich wrote:
>> Making these dependent upon "iommu=debug" isn't really helpful in the
>> field. Where touching respective code anyway also make use of %pp and
>> %pd.
>>
>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Paul Durrant <paul@xen.org>

Thanks.

>> @@ -515,14 +513,13 @@ static u16 __init parse_ivhd_device_rang
>>       dev_length = sizeof(*range);
>>       if ( header_length < (block_length + dev_length) )
>>       {
>> -        AMD_IOMMU_DEBUG("IVHD Error: Invalid Device_Entry Length!\n");
>> +        AMD_IOMMU_ERROR("IVHD: invalid Device_Entry length\n");
>>           return 0;
>>       }
>>   
>>       if ( range->end.header.type != ACPI_IVRS_TYPE_END )
>>       {
>> -        AMD_IOMMU_DEBUG("IVHD Error: "
>> -                        "Invalid Range: End_Type %#x\n",
>> +        AMD_IOMMU_ERROR("IVHD Error: invalid range: End_Type %#x\n",
> 
> NIT: I guess you want to drop the 'Error' here like you did elsewhere.

Yes indeed. Fixed. Thanks for spotting.

Jan