On Tue, Jul 16, 2019 at 04:37:04PM +0000, Jan Beulich wrote:
> Both users will want to know IOMMU properties (specifically the IRTE
> size) subsequently. Leverage this to avoid pointless calls to the
> callback when IVRS mapping table entries are unpopulated. To avoid
> leaking interrupt remapping tables (bogusly) allocated for IOMMUs
> themselves, this requires suppressing their allocation in the first
> place, taking a step further what commit 757122c0cf ('AMD/IOMMU: don't
> "add" IOMMUs') had done.
>
> Additionally suppress the call for alias entries, as again both users
> don't care about these anyway. In fact this eliminates a fair bit of
> redundancy from dump output.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Acked-by: Brian Woods <brian.wo...@amd.com>
> ---
> v3: New.
> ---
> TBD: Along the lines of avoiding the IRT allocation for the IOMMUs, is
> there a way to recognize the many CPU-provided devices many of
> which can't generate interrupts anyway, and avoid allocations for
> them as well? It's 32k per device, after all. Another option might
> be on-demand allocation of the tables, but quite possibly we'd get
> into trouble with error handling there.
>
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -65,7 +65,11 @@ static void __init add_ivrs_mapping_entr
> /* override flags for range of devices */
> ivrs_mappings[bdf].device_flags = flags;
>
> - if (ivrs_mappings[alias_id].intremap_table == NULL )
> + /* Don't map an IOMMU by itself. */
> + if ( iommu->bdf == bdf )
> + return;
> +
> + if ( !ivrs_mappings[alias_id].intremap_table )
> {
> /* allocate per-device interrupt remapping table */
> if ( amd_iommu_perdev_intremap )
> @@ -81,8 +85,9 @@ static void __init add_ivrs_mapping_entr
> ivrs_mappings[alias_id].intremap_inuse = shared_intremap_inuse;
> }
> }
> - /* Assign IOMMU hardware, but don't map an IOMMU by itself. */
> - ivrs_mappings[bdf].iommu = iommu->bdf != bdf ? iommu : NULL;
> +
> + /* Assign IOMMU hardware. */
> + ivrs_mappings[bdf].iommu = iommu;
> }
>
> static struct amd_iommu * __init find_iommu_from_bdf_cap(
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1069,7 +1069,8 @@ int iterate_ivrs_mappings(int (*handler)
> return rc;
> }
>
> -int iterate_ivrs_entries(int (*handler)(u16 seg, struct ivrs_mappings *))
> +int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *,
> + struct ivrs_mappings *))
> {
> u16 seg = 0;
> int rc = 0;
> @@ -1082,7 +1083,12 @@ int iterate_ivrs_entries(int (*handler)(
> break;
> seg = IVRS_MAPPINGS_SEG(map);
> for ( bdf = 0; !rc && bdf < ivrs_bdf_entries; ++bdf )
> - rc = handler(seg, map + bdf);
> + {
> + const struct amd_iommu *iommu = map[bdf].iommu;
> +
> + if ( iommu && map[bdf].dte_requestor_id == bdf )
> + rc = handler(iommu, &map[bdf]);
> + }
> } while ( !rc && ++seg );
>
> return rc;
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -617,7 +617,7 @@ void amd_iommu_read_msi_from_ire(
> }
>
> int __init amd_iommu_free_intremap_table(
> - u16 seg, struct ivrs_mappings *ivrs_mapping)
> + const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping)
> {
> void *tb = ivrs_mapping->intremap_table;
>
> @@ -693,14 +693,15 @@ static void dump_intremap_table(const u3
> }
> }
>
> -static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)
> +static int dump_intremap_mapping(const struct amd_iommu *iommu,
> + struct ivrs_mappings *ivrs_mapping)
> {
> unsigned long flags;
>
> if ( !ivrs_mapping )
> return 0;
>
> - printk(" %04x:%02x:%02x:%u:\n", seg,
> + 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));
> --- a/xen/include/asm-x86/amd-iommu.h
> +++ b/xen/include/asm-x86/amd-iommu.h
> @@ -129,7 +129,8 @@ extern u8 ivhd_type;
>
> struct ivrs_mappings *get_ivrs_mappings(u16 seg);
> int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
> -int iterate_ivrs_entries(int (*)(u16 seg, struct ivrs_mappings *));
> +int iterate_ivrs_entries(int (*)(const struct amd_iommu *,
> + struct ivrs_mappings *));
>
> /* iommu tables in guest space */
> struct mmio_reg {
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -98,7 +98,8 @@ struct amd_iommu *find_iommu_for_device(
> /* interrupt remapping */
> int amd_iommu_setup_ioapic_remapping(void);
> void *amd_iommu_alloc_intremap_table(unsigned long **);
> -int amd_iommu_free_intremap_table(u16 seg, struct ivrs_mappings *);
> +int amd_iommu_free_intremap_table(
> + const struct amd_iommu *, struct ivrs_mappings *);
> void amd_iommu_ioapic_update_ire(
> unsigned int apic, unsigned int reg, unsigned int value);
> unsigned int amd_iommu_read_ioapic_from_ire(
>
--
Brian Woods
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
