Re: [Xen-devel] [PATCH v4 11/12] AMD/IOMMU: don't needlessly log headers when dumping IRTs

["Woods, Brian" <Brian.Woods@xxxxxxx>]

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@xxxxxxxx>

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

> ---
> 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);
> +    }
>   }
> 

-- 
Brian Woods

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