diff mbox series

[v4,11/12] AMD/IOMMU: don't needlessly log headers when dumping IRTs

Message ID 27308615-9199-2183-d987-180520d8afc3@suse.com (mailing list archive)
State New, archived
Headers show
Series [v4,01/12] AMD/IOMMU: use bit field for extended feature register | expand

Commit Message

Jan Beulich July 25, 2019, 1:33 p.m. UTC
Log SBDF headers only when there are actual IRTEs to log. This is
particularly important for the total volume of output when the ACPI
tables describe far more than just the existing devices. On my Rome
system so far there was one line for every function of every device on
all 256 buses of segment 0, with extremely few exceptions (like the
IOMMUs themselves).

Also only log one of the "per-device" or "shared" overall headers.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: New.

Comments

Andrew Cooper July 30, 2019, 10:25 a.m. UTC | #1
On 25/07/2019 14:33, Jan Beulich wrote:
> Log SBDF headers only when there are actual IRTEs to log. This is
> particularly important for the total volume of output when the ACPI
> tables describe far more than just the existing devices. On my Rome
> system so far there was one line for every function of every device on
> all 256 buses of segment 0, with extremely few exceptions (like the
> IOMMUs themselves).
>
> Also only log one of the "per-device" or "shared" overall headers.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Woods, Brian July 30, 2019, 4:36 p.m. UTC | #2
On Thu, Jul 25, 2019 at 01:33:24PM +0000, Jan Beulich wrote:
> Log SBDF headers only when there are actual IRTEs to log. This is
> particularly important for the total volume of output when the ACPI
> tables describe far more than just the existing devices. On my Rome
> system so far there was one line for every function of every device on
> all 256 buses of segment 0, with extremely few exceptions (like the
> IOMMUs themselves).
> 
> Also only log one of the "per-device" or "shared" overall headers.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Brian Woods <brian.woods@amd.com>

> ---
> v4: New.
> 
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -883,7 +883,8 @@ int __init amd_setup_hpet_msi(struct msi
>   }
>   
>   static void dump_intremap_table(const struct amd_iommu *iommu,
> -                                union irte_cptr tbl)
> +                                union irte_cptr tbl,
> +                                const struct ivrs_mappings *ivrs_mapping)
>   {
>       unsigned int count;
>   
> @@ -892,19 +893,25 @@ static void dump_intremap_table(const st
>   
>       for ( count = 0; count < INTREMAP_ENTRIES; count++ )
>       {
> -        if ( iommu->ctrl.ga_en )
> -        {
> -            if ( !tbl.ptr128[count].raw[0] && !tbl.ptr128[count].raw[1] )
> +        if ( iommu->ctrl.ga_en
> +             ? !tbl.ptr128[count].raw[0] && !tbl.ptr128[count].raw[1]
> +             : !tbl.ptr32[count].raw )
>                   continue;
> +
> +        if ( ivrs_mapping )
> +        {
> +            printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
> +                   PCI_BUS(ivrs_mapping->dte_requestor_id),
> +                   PCI_SLOT(ivrs_mapping->dte_requestor_id),
> +                   PCI_FUNC(ivrs_mapping->dte_requestor_id));
> +            ivrs_mapping = NULL;
> +        }
> +
> +        if ( iommu->ctrl.ga_en )
>               printk("    IRTE[%03x] %016lx_%016lx\n",
>                      count, tbl.ptr128[count].raw[1], tbl.ptr128[count].raw[0]);
> -        }
>           else
> -        {
> -            if ( !tbl.ptr32[count].raw )
> -                continue;
>               printk("    IRTE[%03x] %08x\n", count, tbl.ptr32[count].raw);
> -        }
>       }
>   }
>   
> @@ -916,13 +923,8 @@ static int dump_intremap_mapping(const s
>       if ( !ivrs_mapping )
>           return 0;
>   
> -    printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
> -           PCI_BUS(ivrs_mapping->dte_requestor_id),
> -           PCI_SLOT(ivrs_mapping->dte_requestor_id),
> -           PCI_FUNC(ivrs_mapping->dte_requestor_id));
> -
>       spin_lock_irqsave(&(ivrs_mapping->intremap_lock), flags);
> -    dump_intremap_table(iommu, ivrs_mapping->intremap_table);
> +    dump_intremap_table(iommu, ivrs_mapping->intremap_table, ivrs_mapping);
>       spin_unlock_irqrestore(&(ivrs_mapping->intremap_lock), flags);
>   
>       process_pending_softirqs();
> @@ -932,17 +934,22 @@ static int dump_intremap_mapping(const s
>   
>   static void dump_intremap_tables(unsigned char key)
>   {
> -    unsigned long flags;
> -
> -    printk("--- Dumping Per-dev IOMMU Interrupt Remapping Table ---\n");
> +    if ( !shared_intremap_table )
> +    {
> +        printk("--- Dumping Per-dev IOMMU Interrupt Remapping Table ---\n");
>   
> -    iterate_ivrs_entries(dump_intremap_mapping);
> +        iterate_ivrs_entries(dump_intremap_mapping);
> +    }
> +    else
> +    {
> +        unsigned long flags;
>   
> -    printk("--- Dumping Shared IOMMU Interrupt Remapping Table ---\n");
> +        printk("--- Dumping Shared IOMMU Interrupt Remapping Table ---\n");
>   
> -    spin_lock_irqsave(&shared_intremap_lock, flags);
> -    dump_intremap_table(list_first_entry(&amd_iommu_head, struct amd_iommu,
> -                                         list),
> -                        shared_intremap_table);
> -    spin_unlock_irqrestore(&shared_intremap_lock, flags);
> +        spin_lock_irqsave(&shared_intremap_lock, flags);
> +        dump_intremap_table(list_first_entry(&amd_iommu_head, struct amd_iommu,
> +                                             list),
> +                            shared_intremap_table, NULL);
> +        spin_unlock_irqrestore(&shared_intremap_lock, flags);
> +    }
>   }
>
diff mbox series

Patch

--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -883,7 +883,8 @@  int __init amd_setup_hpet_msi(struct msi
  }
  
  static void dump_intremap_table(const struct amd_iommu *iommu,
-                                union irte_cptr tbl)
+                                union irte_cptr tbl,
+                                const struct ivrs_mappings *ivrs_mapping)
  {
      unsigned int count;
  
@@ -892,19 +893,25 @@  static void dump_intremap_table(const st
  
      for ( count = 0; count < INTREMAP_ENTRIES; count++ )
      {
-        if ( iommu->ctrl.ga_en )
-        {
-            if ( !tbl.ptr128[count].raw[0] && !tbl.ptr128[count].raw[1] )
+        if ( iommu->ctrl.ga_en
+             ? !tbl.ptr128[count].raw[0] && !tbl.ptr128[count].raw[1]
+             : !tbl.ptr32[count].raw )
                  continue;
+
+        if ( ivrs_mapping )
+        {
+            printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
+                   PCI_BUS(ivrs_mapping->dte_requestor_id),
+                   PCI_SLOT(ivrs_mapping->dte_requestor_id),
+                   PCI_FUNC(ivrs_mapping->dte_requestor_id));
+            ivrs_mapping = NULL;
+        }
+
+        if ( iommu->ctrl.ga_en )
              printk("    IRTE[%03x] %016lx_%016lx\n",
                     count, tbl.ptr128[count].raw[1], tbl.ptr128[count].raw[0]);
-        }
          else
-        {
-            if ( !tbl.ptr32[count].raw )
-                continue;
              printk("    IRTE[%03x] %08x\n", count, tbl.ptr32[count].raw);
-        }
      }
  }
  
@@ -916,13 +923,8 @@  static int dump_intremap_mapping(const s
      if ( !ivrs_mapping )
          return 0;
  
-    printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
-           PCI_BUS(ivrs_mapping->dte_requestor_id),
-           PCI_SLOT(ivrs_mapping->dte_requestor_id),
-           PCI_FUNC(ivrs_mapping->dte_requestor_id));
-
      spin_lock_irqsave(&(ivrs_mapping->intremap_lock), flags);
-    dump_intremap_table(iommu, ivrs_mapping->intremap_table);
+    dump_intremap_table(iommu, ivrs_mapping->intremap_table, ivrs_mapping);
      spin_unlock_irqrestore(&(ivrs_mapping->intremap_lock), flags);
  
      process_pending_softirqs();
@@ -932,17 +934,22 @@  static int dump_intremap_mapping(const s
  
  static void dump_intremap_tables(unsigned char key)
  {
-    unsigned long flags;
-
-    printk("--- Dumping Per-dev IOMMU Interrupt Remapping Table ---\n");
+    if ( !shared_intremap_table )
+    {
+        printk("--- Dumping Per-dev IOMMU Interrupt Remapping Table ---\n");
  
-    iterate_ivrs_entries(dump_intremap_mapping);
+        iterate_ivrs_entries(dump_intremap_mapping);
+    }
+    else
+    {
+        unsigned long flags;
  
-    printk("--- Dumping Shared IOMMU Interrupt Remapping Table ---\n");
+        printk("--- Dumping Shared IOMMU Interrupt Remapping Table ---\n");
  
-    spin_lock_irqsave(&shared_intremap_lock, flags);
-    dump_intremap_table(list_first_entry(&amd_iommu_head, struct amd_iommu,
-                                         list),
-                        shared_intremap_table);
-    spin_unlock_irqrestore(&shared_intremap_lock, flags);
+        spin_lock_irqsave(&shared_intremap_lock, flags);
+        dump_intremap_table(list_first_entry(&amd_iommu_head, struct amd_iommu,
+                                             list),
+                            shared_intremap_table, NULL);
+        spin_unlock_irqrestore(&shared_intremap_lock, flags);
+    }
  }