Re: [PATCH v2 06/13] iommu: Add extra use_iommu argument to iommu_domain_init()

[Oleksandr Tyshchenko <olekstysh@gmail.com>]

Hi, all.

Any comments?

On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
<olekstysh@gmail.com> wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> The presence of this flag lets us know that the guest domain has statically
> assigned devices which will most likely be used for passthrough
> and as the result the IOMMU is expected to be used for this domain.
>
> Taking into the account this hint when dealing with non-shared IOMMUs
> we can populate IOMMU page tables before hand avoid going through
> the list of pages at the first assigned device.
> As this flag doesn't cover hotplug case, we will continue to populate
> IOMMU page tables on the fly.
>
> Extend corresponding platform callback with extra argument as well and
> pass thought incoming flag to the IOMMU drivers followed by updating
> "d->need_iommu" flag for any domains. But, it must be an additional logic before
> updating this flag for hardware domains which the next patch is introducing.
>
> As iommu_domain_init() is called with "use_iommu" flag being forced
> to false for now, no functional change is intended for both ARM and x86.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> ---
>    Changes in v1:
>       - Clarify patch subject/description.
>       - s/bool_t/bool/
>
>    Changes in v2:
>       - Extend "init" callback with extra argument too.
>       - Clarify patch description.
>       - Add maintainers in CC
> ---
>  xen/arch/arm/domain.c                       |  2 +-
>  xen/arch/x86/domain.c                       |  2 +-
>  xen/drivers/passthrough/amd/pci_amd_iommu.c |  2 +-
>  xen/drivers/passthrough/arm/smmu.c          |  2 +-
>  xen/drivers/passthrough/iommu.c             | 10 ++++++++--
>  xen/drivers/passthrough/vtd/iommu.c         |  2 +-
>  xen/include/xen/iommu.h                     |  4 ++--
>  7 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 76310ed..ec19310 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -569,7 +569,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>      ASSERT(config != NULL);
>
>      /* p2m_init relies on some value initialized by the IOMMU subsystem */
> -    if ( (rc = iommu_domain_init(d)) != 0 )
> +    if ( (rc = iommu_domain_init(d, false)) != 0 )
>          goto fail;
>
>      if ( (rc = p2m_init(d)) != 0 )
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index d7e6992..1ffe76c 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -641,7 +641,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>          if ( (rc = init_domain_irq_mapping(d)) != 0 )
>              goto fail;
>
> -        if ( (rc = iommu_domain_init(d)) != 0 )
> +        if ( (rc = iommu_domain_init(d, false)) != 0 )
>              goto fail;
>      }
>      spin_lock_init(&d->arch.e820_lock);
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index fe744d2..2491e8c 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -261,7 +261,7 @@ static int get_paging_mode(unsigned long entries)
>      return level;
>  }
>
> -static int amd_iommu_domain_init(struct domain *d)
> +static int amd_iommu_domain_init(struct domain *d, bool use_iommu)
>  {
>      struct domain_iommu *hd = dom_iommu(d);
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index e828308..652b58c 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2705,7 +2705,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
>         return 0;
>  }
>
> -static int arm_smmu_iommu_domain_init(struct domain *d)
> +static int arm_smmu_iommu_domain_init(struct domain *d, bool use_iommu)
>  {
>         struct arm_smmu_xen_domain *xen_domain;
>
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 3e9e4c3..19c87d1 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -129,7 +129,7 @@ static void __init parse_iommu_param(char *s)
>      } while ( ss );
>  }
>
> -int iommu_domain_init(struct domain *d)
> +int iommu_domain_init(struct domain *d, bool use_iommu)
>  {
>      struct domain_iommu *hd = dom_iommu(d);
>      int ret = 0;
> @@ -142,7 +142,13 @@ int iommu_domain_init(struct domain *d)
>          return 0;
>
>      hd->platform_ops = iommu_get_ops();
> -    return hd->platform_ops->init(d);
> +    ret = hd->platform_ops->init(d, use_iommu);
> +    if ( ret )
> +        return ret;
> +
> +    d->need_iommu = use_iommu;
> +
> +    return 0;
>  }
>
>  static void __hwdom_init check_hwdom_reqs(struct domain *d)
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index b4e8c89..45d1f36 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1277,7 +1277,7 @@ void __init iommu_free(struct acpi_drhd_unit *drhd)
>          agaw = 64;                              \
>      agaw; })
>
> -static int intel_iommu_domain_init(struct domain *d)
> +static int intel_iommu_domain_init(struct domain *d, bool use_iommu)
>  {
>      dom_iommu(d)->arch.agaw = width_to_agaw(DEFAULT_DOMAIN_ADDRESS_WIDTH);
>
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 3297998..f4d489e 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -56,7 +56,7 @@ int iommu_setup(void);
>  int iommu_add_device(struct pci_dev *pdev);
>  int iommu_enable_device(struct pci_dev *pdev);
>  int iommu_remove_device(struct pci_dev *pdev);
> -int iommu_domain_init(struct domain *d);
> +int iommu_domain_init(struct domain *d, bool use_iommu);
>  void iommu_hwdom_init(struct domain *d);
>  void iommu_domain_destroy(struct domain *d);
>  int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn);
> @@ -155,7 +155,7 @@ struct page_info;
>  typedef int iommu_grdm_t(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt);
>
>  struct iommu_ops {
> -    int (*init)(struct domain *d);
> +    int (*init)(struct domain *d, bool use_iommu);
>      void (*hwdom_init)(struct domain *d);
>      int (*add_device)(u8 devfn, device_t *dev);
>      int (*enable_device)(device_t *dev);
> --
> 2.7.4
>



-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel