On Tue, May 21, 2019 at 06:43:34PM +0100, Robin Murphy wrote:
> On 21/05/2019 17:13, Jordan Crouse wrote:
> >Allow IOMMU enabled devices specified on an opt-in list to create a
> >default identity domain for a new IOMMU group and bypass the DMA
> >domain created by the IOMMU core. This allows the group to be properly
> >set up but otherwise skips touching the hardware until the client
> >device attaches a unmanaged domain of its own.
> 
> All the cool kids are using iommu_request_dm_for_dev() to force an identity
> domain for particular devices, won't that suffice for this case too? There
> is definite scope for improvement in this area, so I'd really like to keep
> things as consistent as possible to make that easier in future.

I initially rejected iommu_request_dm_for_dev() since it still allowed the DMA
domain to consume the context bank but now that I look at it again as long as
the domain free returns the context bank to the pool it might work. Let me give
it a shot and see if it does what we need.

Jordan

> >Signed-off-by: Jordan Crouse <jcro...@codeaurora.org>
> >---
> >
> >  drivers/iommu/arm-smmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/iommu/iommu.c    | 29 +++++++++++++++++++++++------
> >  include/linux/iommu.h    |  3 +++
> >  3 files changed, 68 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >index 5e54cc0..a795ada 100644
> >--- a/drivers/iommu/arm-smmu.c
> >+++ b/drivers/iommu/arm-smmu.c
> >@@ -1235,6 +1235,35 @@ static int arm_smmu_domain_add_master(struct 
> >arm_smmu_domain *smmu_domain,
> >     return 0;
> >  }
> >+struct arm_smmu_client_match_data {
> >+    bool use_identity_domain;
> >+};
> >+
> >+static const struct arm_smmu_client_match_data qcom_adreno = {
> >+    .use_identity_domain = true,
> >+};
> >+
> >+static const struct arm_smmu_client_match_data qcom_mdss = {
> >+    .use_identity_domain = true,
> >+};
> >+
> >+static const struct of_device_id arm_smmu_client_of_match[] = {
> >+    { .compatible = "qcom,adreno", .data = &qcom_adreno },
> >+    { .compatible = "qcom,mdp4", .data = &qcom_mdss },
> >+    { .compatible = "qcom,mdss", .data = &qcom_mdss },
> >+    { .compatible = "qcom,sdm845-mdss", .data = &qcom_mdss },
> >+    {},
> >+};
> >+
> >+static const struct arm_smmu_client_match_data *
> >+arm_smmu_client_data(struct device *dev)
> >+{
> >+    const struct of_device_id *match =
> >+            of_match_device(arm_smmu_client_of_match, dev);
> >+
> >+    return match ? match->data : NULL;
> >+}
> >+
> >  static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device 
> > *dev)
> >  {
> >     int ret;
> >@@ -1552,6 +1581,7 @@ static struct iommu_group 
> >*arm_smmu_device_group(struct device *dev)
> >  {
> >     struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> >     struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
> >+    const struct arm_smmu_client_match_data *client;
> >     struct iommu_group *group = NULL;
> >     int i, idx;
> >@@ -1573,6 +1603,18 @@ static struct iommu_group 
> >*arm_smmu_device_group(struct device *dev)
> >     else
> >             group = generic_device_group(dev);
> >+    client = arm_smmu_client_data(dev);
> >+
> >+    /*
> >+     * If the client chooses to bypass the dma domain, create a identity
> >+     * domain as a default placeholder. This will give the device a
> >+     * default domain but skip DMA operations and not consume a context
> >+     * bank
> >+     */
> >+    if (client && client->no_dma_domain)
> >+            iommu_group_set_default_domain(group, dev,
> >+                    IOMMU_DOMAIN_IDENTITY);
> >+
> >     return group;
> >  }
> >diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> >index 67ee662..af3e1ed 100644
> >--- a/drivers/iommu/iommu.c
> >+++ b/drivers/iommu/iommu.c
> >@@ -1062,6 +1062,24 @@ struct iommu_group *fsl_mc_device_group(struct device 
> >*dev)
> >     return group;
> >  }
> >+struct iommu_domain *iommu_group_set_default_domain(struct iommu_group 
> >*group,
> >+            struct device *dev, unsigned int type)
> >+{
> >+    struct iommu_domain *dom;
> >+
> >+    dom = __iommu_domain_alloc(dev->bus, type);
> >+    if (!dom)
> >+            return NULL;
> >+
> >+    /* FIXME: Error if the default domain is already set? */
> >+    group->default_domain = dom;
> >+    if (!group->domain)
> >+            group->domain = dom;
> >+
> >+    return dom;
> >+}
> >+EXPORT_SYMBOL_GPL(iommu_group_set_default_domain);
> >+
> >  /**
> >   * iommu_group_get_for_dev - Find or create the IOMMU group for a device
> >   * @dev: target device
> >@@ -1099,9 +1117,12 @@ struct iommu_group *iommu_group_get_for_dev(struct 
> >device *dev)
> >     if (!group->default_domain) {
> >             struct iommu_domain *dom;
> >-            dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> >+            dom = iommu_group_set_default_domain(group, dev,
> >+                    iommu_def_domain_type);
> >+
> >             if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
> >-                    dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
> >+                    dom = iommu_group_set_default_domain(group, dev,
> >+                            IOMMU_DOMAIN_DMA);
> >                     if (dom) {
> >                             dev_warn(dev,
> >                                      "failed to allocate default IOMMU 
> > domain of type %u; falling back to IOMMU_DOMAIN_DMA",
> >@@ -1109,10 +1130,6 @@ struct iommu_group *iommu_group_get_for_dev(struct 
> >device *dev)
> >                     }
> >             }
> >-            group->default_domain = dom;
> >-            if (!group->domain)
> >-                    group->domain = dom;
> >-
> >             if (dom && !iommu_dma_strict) {
> >                     int attr = 1;
> >                     iommu_domain_set_attr(dom,
> >diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >index a815cf6..4ef8bd5 100644
> >--- a/include/linux/iommu.h
> >+++ b/include/linux/iommu.h
> >@@ -394,6 +394,9 @@ extern int iommu_group_id(struct iommu_group *group);
> >  extern struct iommu_group *iommu_group_get_for_dev(struct device *dev);
> >  extern struct iommu_domain *iommu_group_default_domain(struct iommu_group 
> > *);
> >+struct iommu_domain *iommu_group_set_default_domain(struct iommu_group 
> >*group,
> >+            struct device *dev, unsigned int type);
> >+
> >  extern int iommu_domain_get_attr(struct iommu_domain *domain, enum 
> > iommu_attr,
> >                              void *data);
> >  extern int iommu_domain_set_attr(struct iommu_domain *domain, enum 
> > iommu_attr,
> >

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to