Re: [Xen-devel] [PATCH 6/6] x86/vtd: Drop struct intel_iommu

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich 
> <JBeulich@xxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>
> Subject: [PATCH 6/6] x86/vtd: Drop struct intel_iommu
> 
> The sole remaining member of struct intel_iommu is the drhd backpointer.  Move
> this into struct vtd_iommu, replacing the the 'intel' pointer.
> 
> This removes one dynamic memory allocation per IOMMU on the system.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/drivers/passthrough/vtd/iommu.c  | 33 +++++----------------------------
>  xen/drivers/passthrough/vtd/iommu.h  |  6 +-----
>  xen/drivers/passthrough/vtd/quirks.c |  9 +++------
>  xen/drivers/passthrough/vtd/utils.c  |  2 +-
>  4 files changed, 10 insertions(+), 40 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
> b/xen/drivers/passthrough/vtd/iommu.c
> index 7fc6fe0..01e2574 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -139,22 +139,6 @@ static int context_get_domain_id(struct context_entry 
> *context,
>      return domid;
>  }
> 
> -static struct intel_iommu *__init alloc_intel_iommu(void)
> -{
> -    struct intel_iommu *intel;
> -
> -    intel = xzalloc(struct intel_iommu);
> -    if ( intel == NULL )
> -        return NULL;
> -
> -    return intel;
> -}
> -
> -static void __init free_intel_iommu(struct intel_iommu *intel)
> -{
> -    xfree(intel);
> -}
> -
>  static int iommus_incoherent;
>  static void __iommu_flush_cache(void *addr, unsigned int size)
>  {
> @@ -869,7 +853,7 @@ static int iommu_page_fault_do_one(struct vtd_iommu 
> *iommu, int type,
>  {
>      const char *reason, *kind;
>      enum faulttype fault_type;
> -    u16 seg = iommu->intel->drhd->segment;
> +    u16 seg = iommu->drhd->segment;
> 
>      reason = iommu_get_fault_reason(fault_reason, &fault_type);
>      switch ( fault_type )
> @@ -982,7 +966,7 @@ static void __do_iommu_page_fault(struct vtd_iommu *iommu)
>          iommu_page_fault_do_one(iommu, type, fault_reason,
>                                  source_id, guest_addr);
> 
> -        pci_check_disable_device(iommu->intel->drhd->segment,
> +        pci_check_disable_device(iommu->drhd->segment,
>                                   PCI_BUS(source_id), PCI_DEVFN2(source_id));
> 
>          fault_index++;
> @@ -1180,13 +1164,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
>      INIT_LIST_HEAD(&iommu->ats_devices);
>      spin_lock_init(&iommu->iremap_lock);
> 
> -    iommu->intel = alloc_intel_iommu();
> -    if ( iommu->intel == NULL )
> -    {
> -        xfree(iommu);
> -        return -ENOMEM;
> -    }
> -    iommu->intel->drhd = drhd;
> +    iommu->drhd = drhd;
>      drhd->iommu = iommu;
> 
>      if ( !(iommu->root_maddr = alloc_pgtable_maddr(drhd, 1)) )
> @@ -1279,7 +1257,6 @@ void __init iommu_free(struct acpi_drhd_unit *drhd)
>      xfree(iommu->domid_bitmap);
>      xfree(iommu->domid_map);
> 
> -    free_intel_iommu(iommu->intel);
>      if ( iommu->msi.irq >= 0 )
>          destroy_irq(iommu->msi.irq);
>      xfree(iommu);
> @@ -1329,7 +1306,7 @@ int domain_context_mapping_one(
>      struct domain_iommu *hd = dom_iommu(domain);
>      struct context_entry *context, *context_entries;
>      u64 maddr, pgd_maddr;
> -    u16 seg = iommu->intel->drhd->segment;
> +    u16 seg = iommu->drhd->segment;

u16.

>      int agaw, rc, ret;
>      bool_t flush_dev_iotlb;
> 
> @@ -1618,7 +1595,7 @@ int domain_context_unmap_one(
>      spin_unlock(&iommu->lock);
>      unmap_vtd_domain_page(context_entries);
> 
> -    if ( !iommu->intel->drhd->segment && !rc )
> +    if ( !iommu->drhd->segment && !rc )
>          rc = me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
> 
>      return rc;
> diff --git a/xen/drivers/passthrough/vtd/iommu.h 
> b/xen/drivers/passthrough/vtd/iommu.h
> index a8cffba..808dfcd 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -506,10 +506,6 @@ extern struct list_head acpi_drhd_units;
>  extern struct list_head acpi_rmrr_units;
>  extern struct list_head acpi_ioapic_units;
> 
> -struct intel_iommu {
> -    struct acpi_drhd_unit *drhd;
> -};
> -
>  struct vtd_iommu {
>      struct list_head list;
>      void __iomem *reg; /* Pointer to hardware regs, virtual addr */
> @@ -521,7 +517,7 @@ struct vtd_iommu {
>      spinlock_t register_lock; /* protect iommu register handling */
>      u64 root_maddr; /* root entry machine address */

Perhaps clean this since it is in context...

>      struct msi_desc msi;
> -    struct intel_iommu *intel;
> +    struct acpi_drhd_unit *drhd;
> 
>      uint64_t qinval_maddr;   /* queue invalidation page machine address */

... so it is consistent with this ^

> 
> diff --git a/xen/drivers/passthrough/vtd/quirks.c 
> b/xen/drivers/passthrough/vtd/quirks.c
> index 79209f3..f3a617f 100644
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -139,8 +139,7 @@ static void __init map_igd_reg(void)
>   */
>  static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
>  {
> -    struct intel_iommu *intel = iommu->intel;
> -    struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> +    struct acpi_drhd_unit *drhd = iommu->drhd;
> 
>      if ( !is_igd_drhd(drhd) || !is_cantiga_b3 )
>          return 0;
> @@ -174,8 +173,7 @@ static int cantiga_vtd_ops_preamble(struct vtd_iommu 
> *iommu)
>   */
>  static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
>  {
> -    struct intel_iommu *intel = iommu->intel;
> -    struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> +    struct acpi_drhd_unit *drhd = iommu->drhd;
>      s_time_t start_time;
> 
>      if ( !is_igd_drhd(drhd) || !is_snb_gfx )
> @@ -204,8 +202,7 @@ static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
> 
>  static void snb_vtd_ops_postamble(struct vtd_iommu *iommu)
>  {
> -    struct intel_iommu *intel = iommu->intel;
> -    struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> +    struct acpi_drhd_unit *drhd = iommu->drhd;
> 
>      if ( !is_igd_drhd(drhd) || !is_snb_gfx )
>          return;
> diff --git a/xen/drivers/passthrough/vtd/utils.c 
> b/xen/drivers/passthrough/vtd/utils.c
> index 72d2235..ee9e6cb 100644
> --- a/xen/drivers/passthrough/vtd/utils.c
> +++ b/xen/drivers/passthrough/vtd/utils.c
> @@ -96,7 +96,7 @@ void print_vtd_entries(struct vtd_iommu *iommu, int bus, 
> int devfn, u64 gmfn)

u64 (and maybe rename).

  Paul

>      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,
> +           iommu->index, iommu->drhd->segment, bus,
>             PCI_SLOT(devfn), PCI_FUNC(devfn), gmfn);
> 
>      if ( iommu->root_maddr == 0 )
> --
> 2.1.4


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