[PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laura Abbott]

Hi,

On the heels of Exynos integrating SMMU in to the DT for probing,
this series attempts to add support to make SMMU drivers work with
deferred probing. This has been referenced before[1] but this is
some actual code to use as a starting point for discussion.

The requirement for this is based on a previous patch to add clock
support to the ARM SMMU driver[2]. Once we have clock support, it's
possible that the driver itself may need to be defered which breaks
a bunch of assumptions about how SMMU probing is supposed to work.
The concept here is to have the driver call of_dma_configure which
may return -EPROBE_DEFER. of_dma_configure could possibly be moved
up to probe level. The existing method of initialization still needs
to be done as well which might mean we have the worst of both worlds.

Open questions:
- This still doesn't really address Arnd's concerns[3] about disabling
IOMMUs
- Currently tested where we knew the driver was going to be deferring.
Probably need some logic for calling of_dma_configure again.

This is based on Robin Murphy's work for dma mapping[4] and a few
patches from Murali Kaicheri[5] for of_dma_configure.


[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-January/011764.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279036.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311579.html
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315459.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/319390.html

Laura Abbott (3):
  dma-mapping: Make arch_setup_dma_ops return an error code
  of: Return error codes from of_dma_configure
  iommu/arm-smmu: Support deferred probing

Mitchel Humpherys (1):
  iommu/arm-smmu: add support for specifying clocks

 arch/arm/include/asm/dma-mapping.h   |   2 +-
 arch/arm/mm/dma-mapping.c            |   4 +-
 arch/arm64/include/asm/dma-mapping.h |   2 +-
 arch/arm64/mm/dma-mapping.c          |  16 +--
 drivers/iommu/arm-smmu.c             | 186 +++++++++++++++++++++++++++++++++--
 drivers/iommu/iommu.c                |  49 ++++++++-
 drivers/iommu/of_iommu.c             |  14 ++-
 drivers/of/device.c                  |   9 +-
 include/linux/dma-mapping.h          |   7 +-
 include/linux/iommu.h                |   2 +
 include/linux/of_device.h            |   4 +-
 11 files changed, 268 insertions(+), 27 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH/RFC 1/4] dma-mapping: Make arch_setup_dma_ops return an error code

[Laura Abbott]

Setting up the DMA operations may trigger a probe deferral.
Return a proper error code to let probe deferral do its thing.

Signed-off-by: Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 arch/arm/include/asm/dma-mapping.h   |  2 +-
 arch/arm/mm/dma-mapping.c            |  4 +++-
 arch/arm64/include/asm/dma-mapping.h |  2 +-
 arch/arm64/mm/dma-mapping.c          | 16 ++++++++++------
 include/linux/dma-mapping.h          |  7 +++++--
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index b52101d..68bff4e 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -122,7 +122,7 @@ static inline unsigned long dma_max_pfn(struct device *dev)
 #define dma_max_pfn(dev) dma_max_pfn(dev)
 
 #define arch_setup_dma_ops arch_setup_dma_ops
-extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+extern int arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 			       struct iommu_ops *iommu, bool coherent);
 
 #define arch_teardown_dma_ops arch_teardown_dma_ops
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7864797..ba20196 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2048,7 +2048,7 @@ static struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
 	return coherent ? &arm_coherent_dma_ops : &arm_dma_ops;
 }
 
-void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+int arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 			struct iommu_ops *iommu, bool coherent)
 {
 	struct dma_map_ops *dma_ops;
@@ -2060,6 +2060,8 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		dma_ops = arm_get_dma_map_ops(coherent);
 
 	set_dma_ops(dev, dma_ops);
+
+	return 0;
 }
 
 void arch_teardown_dma_ops(struct device *dev)
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 0791a78..f7968af 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -45,7 +45,7 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 		return __generic_dma_ops(dev);
 }
 
-void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+int arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 			struct iommu_ops *iommu, bool coherent);
 #define arch_setup_dma_ops	arch_setup_dma_ops
 
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index d52175d..1e3c8f9 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -704,22 +704,24 @@ static struct dma_map_ops iommu_dma_ops = {
 	.mapping_error = iommu_dma_mapping_error,
 };
 
-static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+static int __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 				  struct iommu_ops *ops)
 {
 	struct iommu_dma_mapping *mapping;
+	int ret;
 
 	if (!ops)
-		return;
+		return 0;
 
 	mapping = iommu_dma_create_mapping(ops, dma_base, size);
 	if (!mapping) {
 		pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
 				size, dev_name(dev));
-		return;
+		return -EINVAL;
 	}
 
-	if (iommu_dma_attach_device(dev, mapping))
+	ret = iommu_dma_attach_device(dev, mapping);
+	if (ret)
 		pr_warn("Failed to attach device %s to IOMMU mapping\n",
 				dev_name(dev));
 	else
@@ -727,6 +729,8 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 
 	/* drop the initial mapping refcount */
 	iommu_dma_release_mapping(mapping);
+
+	return ret;
 }
 
 static void __iommu_teardown_dma_ops(struct device *dev)
@@ -747,11 +751,11 @@ static void __iommu_teardown_dma_ops(struct device *dev) { }
 
 #endif  /* CONFIG_IOMMU_DMA */
 
-void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+int arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 			struct iommu_ops *iommu, bool coherent)
 {
 	dev->archdata.dma_coherent = coherent;
-	__iommu_setup_dma_ops(dev, dma_base, size, iommu);
+	return __iommu_setup_dma_ops(dev, dma_base, size, iommu);
 }
 
 void arch_teardown_dma_ops(struct device *dev)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index c3007cb..dfd8060 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,9 +130,12 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask)
 extern u64 dma_get_required_mask(struct device *dev);
 
 #ifndef arch_setup_dma_ops
-static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
+static inline int arch_setup_dma_ops(struct device *dev, u64 dma_base,
 				      u64 size, struct iommu_ops *iommu,
-				      bool coherent) { }
+				      bool coherent)
+{
+	return 0;
+}
 #endif
 
 #ifndef arch_teardown_dma_ops
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH/RFC 2/4] of: Return error codes from of_dma_configure

[Laura Abbott]

of_dma_configure is currently a void function. IOMMU configuration may
need to defer probing so return appropriate values.

Signed-off-by: Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/iommu/of_iommu.c  | 14 +++++++++++---
 drivers/of/device.c       |  9 +++++++--
 include/linux/of_device.h |  4 ++--
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 396bc77..01cd540 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -138,7 +138,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
 {
 	struct of_phandle_args iommu_spec;
 	struct device_node *np;
-	struct iommu_ops *ops = NULL;
+	struct iommu_ops *ops = ERR_PTR(-ENODEV);
 	int idx = 0;
 
 	if (dev_is_pci(dev)) {
@@ -154,11 +154,19 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
 	while (!of_parse_phandle_with_args(node, "iommus",
 					   "#iommu-cells", idx,
 					   &iommu_spec)) {
+		int ret;
+
 		np = iommu_spec.np;
 		ops = of_iommu_get_ops(np);
 
-		if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+		if (!ops || !ops->of_xlate)
+			goto err_put_node;
+
+		ret = ops->of_xlate(dev, &iommu_spec);
+		if (ret) {
+			ops = ERR_PTR(ret);
 			goto err_put_node;
+		}
 
 		of_node_put(np);
 		idx++;
@@ -168,7 +176,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
 
 err_put_node:
 	of_node_put(np);
-	return NULL;
+	return ops;
 }
 
 void __init of_iommu_init(void)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index ccbc958..1ff7a7a 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -81,7 +81,7 @@ int of_device_add(struct platform_device *ofdev)
  * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event
  * to fix up DMA configuration.
  */
-void of_dma_configure(struct device *dev, struct device_node *np)
+int of_dma_configure(struct device *dev, struct device_node *np)
 {
 	u64 dma_addr, paddr, size;
 	int ret;
@@ -117,10 +117,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 		coherent ? " " : " not ");
 
 	iommu = of_iommu_configure(dev, np);
+	if (PTR_ERR(iommu) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+	else if (IS_ERR(iommu))
+		iommu = NULL;
+
 	dev_dbg(dev, "device is%sbehind an iommu\n",
 		iommu ? " " : " not ");
 
-	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+	return arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
 
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index c661496..c0821c0 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -53,7 +53,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 	return of_node_get(cpu_dev->of_node);
 }
 
-void of_dma_configure(struct device *dev, struct device_node *np);
+int of_dma_configure(struct device *dev, struct device_node *np);
 #else /* CONFIG_OF */
 
 static inline int of_driver_match_device(struct device *dev,
@@ -91,7 +91,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 {
 	return NULL;
 }
-void of_dma_configure(struct device *dev, struct device_node *np) { }
+int of_dma_configure(struct device *dev, struct device_node *np) { return -ENXIO; }
 #endif /* CONFIG_OF */
 
 #endif /* _LINUX_OF_DEVICE_H */
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC 3/4] iommu/arm-smmu: add support for specifying clocks

[Laura Abbott]

From: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On some platforms with tight power constraints it is polite to only
leave your clocks on for as long as you absolutely need them. Currently
we assume that all clocks necessary for SMMU register access are always
on.

Add some optional device tree properties to specify any clocks that are
necessary for SMMU register access and turn them on and off as needed.

If no clocks are specified in the device tree things continue to work
the way they always have: we assume all necessary clocks are always
turned on.

Signed-off-by: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
This is a rework of the patch to add clocks and it attempted to address
previous comments. I sent it with this series to give the rest of the
RFC some series but I'll probably resend later versions of this one separately
from the rest of the series.
---
 .../devicetree/bindings/iommu/arm,smmu.txt         |  10 ++
 drivers/iommu/arm-smmu.c                           | 149 ++++++++++++++++++++-
 drivers/iommu/iommu.c                              |  49 ++++++-
 include/linux/iommu.h                              |   2 +
 4 files changed, 203 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 0676050..9200e0b 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -48,6 +48,16 @@ conditions.
                   SMMU configuration registers. In this case non-secure
                   aliases of secure registers have to be used during
                   SMMU configuration.
+- clocks        : List of clocks to be used during SMMU register access. See
+                  Documentation/devicetree/bindings/clock/clock-bindings.txt
+                  for information about the format. For each clock specified
+                  here, there must be a corresponding entery in clock-names
+                  (see below).
+
+- clock-names   : List of clock names corresponding to the clocks specified in
+                  the "clocks" property (above). See
+                  Documentation/devicetree/bindings/clock/clock-bindings.txt
+                  for more info.
 
 Example:
 
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6cd47b7..d9f7cf48 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -391,6 +391,10 @@ struct arm_smmu_device {
 
 	struct list_head		list;
 	struct rb_root			masters;
+
+	int				num_clocks;
+	struct clk			**clocks;
+	atomic_t			clk_enable;
 };
 
 struct arm_smmu_cfg {
@@ -596,6 +600,36 @@ static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
 	clear_bit(idx, map);
 }
 
+static int arm_smmu_enable_clocks(struct arm_smmu_device *smmu)
+{
+	int i, ret = 0;
+
+	atomic_add(1, &smmu->clk_enable);
+
+	for (i = 0; i < smmu->num_clocks; ++i) {
+		ret = clk_prepare_enable(smmu->clocks[i]);
+		if (ret) {
+			dev_err(smmu->dev, "Couldn't enable clock #%d\n", i);
+			while (i--)
+				clk_disable_unprepare(smmu->clocks[i]);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static void arm_smmu_disable_clocks(struct arm_smmu_device *smmu)
+{
+	int i;
+
+	if (!atomic_add_unless(&smmu->clk_enable, -1, 0))
+		return;
+
+	for (i = 0; i < smmu->num_clocks; ++i)
+		clk_disable_unprepare(smmu->clocks[i]);
+}
+
 /* Wait for any pending TLB invalidations to complete */
 static void arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
 {
@@ -1259,6 +1293,13 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	}
 
 	/*
+	 * Need the explicit clock enable calls because the context
+	 * isn't set up for this to work with enable_resource/
+	 * disable resource
+	 */
+	arm_smmu_enable_clocks(smmu);
+
+	/*
 	 * Sanity check the domain. We don't support domains across
 	 * different SMMUs.
 	 */
@@ -1267,7 +1308,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		/* Now that we have a master, we can finalise the domain */
 		ret = arm_smmu_init_domain_context(domain, smmu);
 		if (IS_ERR_VALUE(ret))
-			return ret;
+			goto out;
 
 		dom_smmu = smmu_domain->smmu;
 	}
@@ -1276,17 +1317,22 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		dev_err(dev,
 			"cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n",
 			dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	/* Looks ok, so add the device to the domain */
 	cfg = find_smmu_master_cfg(dev);
-	if (!cfg)
-		return -ENODEV;
+	if (!cfg) {
+		ret = -ENODEV;
+		goto out;
+	}
 
 	ret = arm_smmu_domain_add_master(smmu_domain, cfg);
 	if (!ret)
 		dev->archdata.iommu = domain;
+out:
+	arm_smmu_disable_clocks(smmu);
 	return ret;
 }
 
@@ -1715,6 +1761,42 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	}
 }
 
+static int arm_smmu_enable_resources(struct iommu_domain *domain)
+{
+	struct arm_smmu_domain *smmu_domain = domain->priv;
+	struct arm_smmu_device *smmu;
+
+	if (!smmu_domain)
+		return 0;
+
+	smmu = smmu_domain->smmu;
+
+	if (!smmu)
+		return 0;
+
+	arm_smmu_enable_clocks(smmu);
+
+	return 0;
+}
+
+static int arm_smmu_disable_resources(struct iommu_domain *domain)
+{
+	struct arm_smmu_domain *smmu_domain = domain->priv;
+	struct arm_smmu_device *smmu;
+
+	if (!smmu_domain)
+		return 0;
+
+	smmu = smmu_domain->smmu;
+
+	if (!smmu)
+		return 0;
+
+	arm_smmu_disable_clocks(smmu);
+
+	return 0;
+}
+
 static const struct iommu_ops arm_smmu_ops = {
 	.capable		= arm_smmu_capable,
 	.domain_init		= arm_smmu_domain_init,
@@ -1732,6 +1814,8 @@ static const struct iommu_ops arm_smmu_ops = {
 	.pgsize_bitmap		= (SECTION_SIZE |
 				   ARM_SMMU_PTE_CONT_SIZE |
 				   PAGE_SIZE),
+	.enable_resources	= arm_smmu_enable_resources,
+	.disable_resources	= arm_smmu_disable_resources,
 };
 
 static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
@@ -1805,6 +1889,51 @@ static int arm_smmu_id_size_to_bits(int size)
 	}
 }
 
+static int arm_smmu_init_clocks(struct arm_smmu_device *smmu)
+{
+	const char *cname;
+	struct property *prop;
+	int i;
+	struct device *dev = smmu->dev;
+
+	smmu->num_clocks = of_property_count_strings(dev->of_node,
+						"clock-names");
+
+	if (!smmu->num_clocks)
+		return 0;
+
+	smmu->clocks = devm_kzalloc(
+		dev, sizeof(*smmu->clocks) * smmu->num_clocks,
+		GFP_KERNEL);
+
+	if (!smmu->clocks) {
+		dev_err(dev,
+			"Failed to allocate memory for clocks\n");
+		return -ENODEV;
+	}
+
+	i = 0;
+	of_property_for_each_string(dev->of_node, "clock-names",
+				prop, cname) {
+		struct clk *c = devm_clk_get(dev, cname);
+		if (IS_ERR(c)) {
+			dev_err(dev, "Couldn't get clock: %s",
+				cname);
+			return PTR_ERR(c);
+		}
+
+		if (clk_get_rate(c) == 0) {
+			long rate = clk_round_rate(c, 1000);
+			clk_set_rate(c, rate);
+		}
+
+		smmu->clocks[i] = c;
+
+		++i;
+	}
+	return 0;
+}
+
 static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 {
 	unsigned long size;
@@ -2031,10 +2160,16 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		smmu->irqs[i] = irq;
 	}
 
-	err = arm_smmu_device_cfg_probe(smmu);
+	err = arm_smmu_init_clocks(smmu);
 	if (err)
 		return err;
 
+	arm_smmu_enable_clocks(smmu);
+
+	err = arm_smmu_device_cfg_probe(smmu);
+	if (err)
+		goto out_disable_clocks;
+
 	i = 0;
 	smmu->masters = RB_ROOT;
 	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
@@ -2081,6 +2216,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	spin_unlock(&arm_smmu_devices_lock);
 
 	arm_smmu_device_reset(smmu);
+	arm_smmu_disable_clocks(smmu);
 	return 0;
 
 out_free_irqs:
@@ -2094,6 +2230,9 @@ out_put_masters:
 		of_node_put(master->of_node);
 	}
 
+out_disable_clocks:
+	arm_smmu_disable_clocks(smmu);
+
 	return err;
 }
 
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f7718d7..236b1e1 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -927,9 +927,17 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
 
 void iommu_domain_free(struct iommu_domain *domain)
 {
-	if (likely(domain->ops->domain_destroy != NULL))
+	if (likely(domain->ops->domain_destroy != NULL)) {
+		if (domain->ops->enable_resources)
+			domain->ops->enable_resources(domain);
+
 		domain->ops->domain_destroy(domain);
 
+		if (domain->ops->disable_resources)
+			domain->ops->disable_resources(domain);
+
+	}
+
 	kfree(domain);
 }
 EXPORT_SYMBOL_GPL(iommu_domain_free);
@@ -940,9 +948,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 	if (unlikely(domain->ops->attach_dev == NULL))
 		return -ENODEV;
 
+	if (domain->ops->enable_resources)
+		domain->ops->enable_resources(domain);
+
 	ret = domain->ops->attach_dev(domain, dev);
 	if (!ret)
 		trace_attach_device_to_domain(dev);
+
+	if (domain->ops->disable_resources)
+		domain->ops->disable_resources(domain);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
@@ -952,8 +967,15 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 	if (unlikely(domain->ops->detach_dev == NULL))
 		return;
 
+	if (domain->ops->enable_resources)
+		domain->ops->enable_resources(domain);
+
 	domain->ops->detach_dev(domain, dev);
 	trace_detach_device_from_domain(dev);
+
+	if (domain->ops->disable_resources)
+		domain->ops->disable_resources(domain);
+
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device);
 
@@ -998,10 +1020,20 @@ EXPORT_SYMBOL_GPL(iommu_detach_group);
 
 phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
 {
+	phys_addr_t phys;
+
 	if (unlikely(domain->ops->iova_to_phys == NULL))
 		return 0;
 
-	return domain->ops->iova_to_phys(domain, iova);
+	if (domain->ops->enable_resources)
+		domain->ops->enable_resources(domain);
+
+	phys = domain->ops->iova_to_phys(domain, iova);
+
+	if (domain->ops->disable_resources)
+		domain->ops->disable_resources(domain);
+
+	return phys;
 }
 EXPORT_SYMBOL_GPL(iommu_iova_to_phys);
 
@@ -1065,6 +1097,9 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
 
 	pr_debug("map: iova 0x%lx pa %pa size 0x%zx\n", iova, &paddr, size);
 
+	if (domain->ops->enable_resources)
+		domain->ops->enable_resources(domain);
+
 	while (size) {
 		size_t pgsize = iommu_pgsize(domain, iova | paddr, size);
 
@@ -1080,6 +1115,9 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		size -= pgsize;
 	}
 
+	if (domain->ops->disable_resources)
+		domain->ops->disable_resources(domain);
+
 	/* unroll mapping in case something went wrong */
 	if (ret)
 		iommu_unmap(domain, orig_iova, orig_size - size);
@@ -1115,6 +1153,10 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 
 	pr_debug("unmap this: iova 0x%lx size 0x%zx\n", iova, size);
 
+
+	if (domain->ops->enable_resources)
+		domain->ops->enable_resources(domain);
+
 	/*
 	 * Keep iterating until we either unmap 'size' bytes (or more)
 	 * or we hit an area that isn't mapped.
@@ -1133,6 +1175,9 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 		unmapped += unmapped_page;
 	}
 
+	if (domain->ops->disable_resources)
+		domain->ops->disable_resources(domain);
+
 	trace_unmap(iova, 0, size);
 	return unmapped;
 }
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 38daa45..79cc5af 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -145,6 +145,8 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 #endif
 
+	int (*enable_resources)(struct iommu_domain *domain);
+	int (*disable_resources)(struct iommu_domain *domain);
 	unsigned long pgsize_bitmap;
 	void *priv;
 };
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH/RFC 4/4] iommu/arm-smmu: Support deferred probing

[Laura Abbott]

With the addition of clocks in the SMMU driver, the driver
now may need to be deferred if the clocks are not ready. Apart from
just the probe function though, we may need to defer attachment as
well. Support both of these.

Signed-off-by: Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
I went with the simplest approach ('started_probe') to indicate
we should defer probing. Another possibility I considered was to have
a 'masters added before probe completed' list and keep all masters
there until probe completes at which point we can bind the masters
to the SMMUs.
---
 drivers/iommu/arm-smmu.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index d9f7cf48..7e8194c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -43,6 +43,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
 
 #include <linux/amba/bus.h>
 
@@ -330,6 +332,7 @@ static int force_stage;
 module_param_named(force_stage, force_stage, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(force_stage,
 	"Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation.");
+static bool started_probe;
 
 enum arm_smmu_arch_version {
 	ARM_SMMU_V1 = 1,
@@ -1284,7 +1287,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	smmu = find_smmu_for_device(dev);
 	if (!smmu) {
 		dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
-		return -ENXIO;
+		return started_probe ? -EPROBE_DEFER : -ENXIO;
 	}
 
 	if (dev->archdata.iommu) {
@@ -1677,7 +1680,7 @@ static int arm_smmu_add_device(struct device *dev)
 
 	smmu = find_smmu_for_device(dev);
 	if (!smmu)
-		return -ENODEV;
+		return started_probe ? -EPROBE_DEFER : -ENODEV;
 
 	group = iommu_group_alloc();
 	if (IS_ERR(group)) {
@@ -1761,6 +1764,11 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	}
 }
 
+static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *spec)
+{
+	return arm_smmu_add_device(dev);
+}
+
 static int arm_smmu_enable_resources(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = domain->priv;
@@ -1814,6 +1822,7 @@ static const struct iommu_ops arm_smmu_ops = {
 	.pgsize_bitmap		= (SECTION_SIZE |
 				   ARM_SMMU_PTE_CONT_SIZE |
 				   PAGE_SIZE),
+	.of_xlate		= arm_smmu_of_xlate,
 	.enable_resources	= arm_smmu_enable_resources,
 	.disable_resources	= arm_smmu_disable_resources,
 };
@@ -2108,6 +2117,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	struct of_phandle_args masterspec;
 	int num_irqs, i, err;
 
+	started_probe = true;
+
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
 	if (!smmu) {
 		dev_err(dev, "failed to allocate arm_smmu_device\n");
@@ -2282,6 +2293,8 @@ static struct platform_driver arm_smmu_driver = {
 	.remove	= arm_smmu_device_remove,
 };
 
+static int init_done;
+
 static int __init arm_smmu_init(void)
 {
 	struct device_node *np;
@@ -2316,6 +2329,7 @@ static int __init arm_smmu_init(void)
 		bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
 #endif
 
+	init_done = true;
 	return 0;
 }
 
@@ -2327,6 +2341,25 @@ static void __exit arm_smmu_exit(void)
 subsys_initcall(arm_smmu_init);
 module_exit(arm_smmu_exit);
 
+static int __init arm_smmu_of_setup(struct device_node *np)
+{
+	struct platform_device *pdev;
+
+	if (!init_done)
+		arm_smmu_init();
+
+	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	of_iommu_set_ops(np, &arm_smmu_ops);
+	return 0;
+}
+
+IOMMU_OF_DECLARE(arm_smmu_of, "arm,mmu-500",
+		 arm_smmu_of_setup);
+
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH/RFC 4/4] iommu/arm-smmu: Support deferred probing

[Robin Murphy]

Hi Laura,

As a heads up, I'm still vainly hoping to move the ARM SMMU driver 
entirely over to the generic framework - there's an iommu/dev branch on 
top of the iommu/dma branch I pushed earlier[1] which you might want to 
take a peek at to check if we're likely to end up pulling in different 
directions.

[1]:http://article.gmane.org/gmane.linux.kernel.iommu/8773

On 06/02/15 00:32, Laura Abbott wrote:
>
> With the addition of clocks in the SMMU driver, the driver
> now may need to be deferred if the clocks are not ready. Apart from
> just the probe function though, we may need to defer attachment as
> well. Support both of these.
>
> Signed-off-by: Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> I went with the simplest approach ('started_probe') to indicate
> we should defer probing. Another possibility I considered was to have
> a 'masters added before probe completed' list and keep all masters
> there until probe completes at which point we can bind the masters
> to the SMMUs.

I think this looks OK for the single-distributed-SMMU case MMU-500 
allows; the Juno case with multiple MMU-401 instances is more awkward, 
but I have a feeling there's a really neat way to slot this approach to 
deferral into my per-instance stuff. I'll give this series a spin and 
see what falls out.

Thanks,
Robin.

> ---
>   drivers/iommu/arm-smmu.c | 37 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index d9f7cf48..7e8194c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -43,6 +43,8 @@
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/of_iommu.h>
> +#include <linux/of_platform.h>
>
>   #include <linux/amba/bus.h>
>
> @@ -330,6 +332,7 @@ static int force_stage;
>   module_param_named(force_stage, force_stage, int, S_IRUGO | S_IWUSR);
>   MODULE_PARM_DESC(force_stage,
>   	"Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation.");
> +static bool started_probe;
>
>   enum arm_smmu_arch_version {
>   	ARM_SMMU_V1 = 1,
> @@ -1284,7 +1287,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>   	smmu = find_smmu_for_device(dev);
>   	if (!smmu) {
>   		dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
> -		return -ENXIO;
> +		return started_probe ? -EPROBE_DEFER : -ENXIO;
>   	}
>
>   	if (dev->archdata.iommu) {
> @@ -1677,7 +1680,7 @@ static int arm_smmu_add_device(struct device *dev)
>
>   	smmu = find_smmu_for_device(dev);
>   	if (!smmu)
> -		return -ENODEV;
> +		return started_probe ? -EPROBE_DEFER : -ENODEV;
>
>   	group = iommu_group_alloc();
>   	if (IS_ERR(group)) {
> @@ -1761,6 +1764,11 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>   	}
>   }
>
> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *spec)
> +{
> +	return arm_smmu_add_device(dev);
> +}
> +
>   static int arm_smmu_enable_resources(struct iommu_domain *domain)
>   {
>   	struct arm_smmu_domain *smmu_domain = domain->priv;
> @@ -1814,6 +1822,7 @@ static const struct iommu_ops arm_smmu_ops = {
>   	.pgsize_bitmap		= (SECTION_SIZE |
>   				   ARM_SMMU_PTE_CONT_SIZE |
>   				   PAGE_SIZE),
> +	.of_xlate		= arm_smmu_of_xlate,
>   	.enable_resources	= arm_smmu_enable_resources,
>   	.disable_resources	= arm_smmu_disable_resources,
>   };
> @@ -2108,6 +2117,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>   	struct of_phandle_args masterspec;
>   	int num_irqs, i, err;
>
> +	started_probe = true;
> +
>   	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
>   	if (!smmu) {
>   		dev_err(dev, "failed to allocate arm_smmu_device\n");
> @@ -2282,6 +2293,8 @@ static struct platform_driver arm_smmu_driver = {
>   	.remove	= arm_smmu_device_remove,
>   };
>
> +static int init_done;
> +
>   static int __init arm_smmu_init(void)
>   {
>   	struct device_node *np;
> @@ -2316,6 +2329,7 @@ static int __init arm_smmu_init(void)
>   		bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>   #endif
>
> +	init_done = true;
>   	return 0;
>   }
>
> @@ -2327,6 +2341,25 @@ static void __exit arm_smmu_exit(void)
>   subsys_initcall(arm_smmu_init);
>   module_exit(arm_smmu_exit);
>
> +static int __init arm_smmu_of_setup(struct device_node *np)
> +{
> +	struct platform_device *pdev;
> +
> +	if (!init_done)
> +		arm_smmu_init();
> +
> +	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	of_iommu_set_ops(np, &arm_smmu_ops);
> +	return 0;
> +}
> +
> +IOMMU_OF_DECLARE(arm_smmu_of, "arm,mmu-500",
> +		 arm_smmu_of_setup);
> +
> +
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>   MODULE_LICENSE("GPL v2");
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[arnd-r2nGTMty4D4]

Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> hat am 6. Februar 2015 um 01:31
geschrieben:
>
> The requirement for this is based on a previous patch to add clock
> support to the ARM SMMU driver[2]. Once we have clock support, it's
> possible that the driver itself may need to be defered which breaks
> a bunch of assumptions about how SMMU probing is supposed to work.
 
Hi Laura,
 
I was hoping that we would not need this, and instead treat the iommu in
the same way as timers and SMP initialization, both
of which need to be run early at boot time but may rely on clock controllers
to be initialized first.
 
Is there a specific requirement that makes this impossible here, or is your
intention to solve the problem more nicely by allowing deferred probing
over forcing the input clocks of the iommu to be early?
 
      Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laura Abbott]

On 2/7/2015 2:41 PM, arnd@arndb.de wrote:
> Laura Abbott <lauraa@codeaurora.org> hat am 6. Februar 2015 um 01:31
> geschrieben:
>>
>> The requirement for this is based on a previous patch to add clock
>> support to the ARM SMMU driver[2]. Once we have clock support, it's
>> possible that the driver itself may need to be defered which breaks
>> a bunch of assumptions about how SMMU probing is supposed to work.
>
> Hi Laura,
>
> I was hoping that we would not need this, and instead treat the iommu in
> the same way as timers and SMP initialization, both
> of which need to be run early at boot time but may rely on clock controllers
> to be initialized first.
>
> Is there a specific requirement that makes this impossible here, or is your
> intention to solve the problem more nicely by allowing deferred probing
> over forcing the input clocks of the iommu to be early?
>
>        Arnd
>

The current clock driver for qcom targets doesn't support the early
initialization needed for timers and SMP because neither of those depend
on the clocksources. I discussed this with Stephen some and adding the
early support would not mesh well with the device/driver design of the
current clock driver so that's not really an option right now.

I do think the deferred probing design is cleaner. Even cleaner would
be a proper bus type but that's a different can of worms.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Marek Szyprowski]

Hello,

On 2015-02-07 23:41, arnd-r2nGTMty4D4@public.gmane.org wrote:
> Laura Abbott <lauraa-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> hat am 6. Februar 2015 um 01:31
> geschrieben:
>> The requirement for this is based on a previous patch to add clock
>> support to the ARM SMMU driver[2]. Once we have clock support, it's
>> possible that the driver itself may need to be defered which breaks
>> a bunch of assumptions about how SMMU probing is supposed to work.
>   
> Hi Laura,
>   
> I was hoping that we would not need this, and instead treat the iommu in
> the same way as timers and SMP initialization, both
> of which need to be run early at boot time but may rely on clock controllers
> to be initialized first.
>   
> Is there a specific requirement that makes this impossible here, or is your
> intention to solve the problem more nicely by allowing deferred probing
> over forcing the input clocks of the iommu to be early?

I case of Exynos SoCs there is also a dependency on power domains (some 
might
be disabled by the bootloader). It is convenient to use the whole device
infrastructure for this although it still doesn't provide any methods of
modelling the real power management dependencies. Right now I simply ignored
this problem and left it for the future.

I will check if this patchset helps in our case.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laurent Pinchart]

Hi Laura and all,

On Thursday 05 February 2015 16:31:58 Laura Abbott wrote:
> Hi,
> 
> On the heels of Exynos integrating SMMU in to the DT for probing,
> this series attempts to add support to make SMMU drivers work with
> deferred probing. This has been referenced before[1] but this is
> some actual code to use as a starting point for discussion.
> 
> The requirement for this is based on a previous patch to add clock
> support to the ARM SMMU driver[2]. Once we have clock support, it's
> possible that the driver itself may need to be defered which breaks
> a bunch of assumptions about how SMMU probing is supposed to work.
> The concept here is to have the driver call of_dma_configure which
> may return -EPROBE_DEFER. of_dma_configure could possibly be moved
> up to probe level. The existing method of initialization still needs
> to be done as well which might mean we have the worst of both worlds.
> 
> Open questions:
> - This still doesn't really address Arnd's concerns[3] about disabling
> IOMMUs

Arnd, Will and I have discussed IOMMU probe deferral last week. Here's a 
summary of the discussion, before the details blur away.

The discussion covered both higher level concepts and lower level details, in 
various directions. I'll try to make the summary clear by filling the gaps 
between pieces where needed, so some of the information below might not be a 
direct result of the discussions. Arnd and Will, please feel free to correct 
me.

The first question we've discussed was whether probe deferral for IOMMUs is 
really needed. Various dependencies between IOMMU devices and other devices 
exist, in particular on clocks (as you have mentioned above) and on power 
domains (as mentioned by Marek in http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html). While there are mechanism to handle 
some of them with probe deferral (for instance by using the OF_DECLARE macros 
to register clock drivers), generalizing those mechanisms would essentially 
recreate a probe ordering mechanism similar to link order probe ordering and 
couldn't really scale.

Additionally, IOMMUs could also be present hot-pluggable devices and depend on 
resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case. 
For all those reasons we have concluded that probe deferral for IOMMUs is 
desired if it can be implemented cleanly.

The second question we've discussed was how to implement probe deferral 
cleanly :-)

The current implementation configures DMA at device creation time and sets the 
following properties:

- dma_map_ops (IOMMU, SWIOTLB, linear mapping)
- initial DMA mask
- DMA offset
- DMA coherency

Additionally the IOMMU group (when an IOMMU is present) will also be 
configured at the same time (patches are under review).

The base idea is to defer probing of bus master devices when their IOMMU isn't 
present. Three cases need to be handled.

1. No IOMMU is declared by the firmware (through DT, ACPI, ...) for the bus 
master device. The bus master device probe doesn't need to be deferred due to 
the IOMMU. dma_map_ops must be set to SWIOTLB or linear mapping (the later 
should likely be implemented through SWIOTLB).

2. An IOMMU is declared for the bus master device and the IOMMU has been 
successfully probed and registered. The bus master device probe doesn't need 
to be deferred due to the IOMMU. dma_map_ops must be set to IOMMU ops.

3. An IOMMU is declared for the bus master device but the IOMMU isn't 
registered. This can be caused by different reasons:

- a. No driver is loaded for this IOMMU (for instance because DT describes the 
IOMMU connection, but the IOMMU driver hasn't been developed yet, or an older 
kernel is used). If the IOMMU is optional the bus master device probe should 
succeed, and dma_map_ops should be set to linear. If the IOMMU is mandatory 
the bus master device probe should fail.

Note that, as we require IOMMU drivers to be compiled in, we don't need to 
handle the case of loadable IOMMU drivers that haven't been loaded yet.

- b. A driver is loaded for this IOMMU but the IOMMU hasn't been probed yet, 
or its probe has been deferred. The bus master device probe should be 
deferred.

- c. A driver is loaded for this IOMMU but the IOMMU probe has failed 
permanently. It's not clear at the moment whether we should try to recover 
from this automatically using the same mechanism as case 3.a, or if we can 
considered this as an abnormal failure and disable the bus master device.

Assuming that we should try to recover from such an error, we can't predict 
this case when the device is instantiated (even if IOMMUs are registered 
before bus master devices are added, for instance using the OF_DECLARE 
mechanism that Will has implemented). We thus need to setup the dma_map_ops 
and IOMMU group at bus master device probe time.

Furthermore, we can't distinguish cases 3.a and 3.b at bus master probe time 
without early registration of IOMMU drivers. Case 3.a would instead be 
considered as 3.b, leading to infinite probe deferral of bus master devices.

For those reasons we have concluded that IOMMU probe deferral needs to be 
implemented with a combination of several mechanisms. The following steps 
should happen at bus master device probe time.

1. The IOMMU device referenced by the firmware with the bus master device is 
looked up. On DT-based systems, this will be the IOMMU DT node referenced by 
the iommus property. If no IOMMU device is associated, dma_map_ops will be set 
to linear mapping or SWIOTLB and device probe will continue.

2. An IOMMU device is referenced for the bus master device.

The corresponding IOMMU instance is looked up. This requires a new IOMMU 
registration system. IOMMU drivers will create IOMMU instances at probe time 
and register them with the IOMMU core.

If an IOMMU instance is found for the referenced IOMMU device, the IOMMU 
instance's of_xlate function will be called to setup the IOMMU.

If the of_xlate call succeeds dma_map_ops will be set to IOMMU ops and device 
probe will continue. If the call fails we can either fail the bus master 
device probe, or fall back to non-IOMMU dma_map_ops (to be discussed).

3. The IOMMU device referenced for the bus master device isn't present, due to 
the IOMMU device probe not having been performed yet, having been deferred, or 
having failed.

The IOMMU driver associated with the IOMMU device is looked up. This was 
initially thought to require an early registration mechanism for IOMMU drivers 
(using an OF_DECLARE mechanism for DT-based systems for instance), but on 
second thought it might be possible to implement this based on normal driver 
registration (to be researched).

If an IOMMU driver is found for the referenced IOMMU device, a callback 
function of the IOMMU driver is called to check whether an IOMMU instance is 
expected to be registered later (most IOMMU drivers will just return true, so 
we could skip this callback function until an IOMMU driver requires it). If an 
IOMMU instance is expected to be registered later the bus master device probe 
is deferred. Otherwise dma_map_ops will be set to linear mapping/SWIOTLB and 
device probe will continue.


The initial DMA mask and the DMA offset can still be configured at device 
instantiation time if desired.

> - Currently tested where we knew the driver was going to be deferring.
> Probably need some logic for calling of_dma_configure again.
> 
> This is based on Robin Murphy's work for dma mapping[4] and a few
> patches from Murali Kaicheri[5] for of_dma_configure.
> 
> 
> [1]
> http://lists.linuxfoundation.org/pipermail/iommu/2015-January/011764.html
> [2]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279036.ht
> ml [3]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311579.
> html [4]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315459.h
> tml [5]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/319390.h
> tml
> 
> Laura Abbott (3):
>   dma-mapping: Make arch_setup_dma_ops return an error code
>   of: Return error codes from of_dma_configure
>   iommu/arm-smmu: Support deferred probing
> 
> Mitchel Humpherys (1):
>   iommu/arm-smmu: add support for specifying clocks
> 
>  arch/arm/include/asm/dma-mapping.h   |   2 +-
>  arch/arm/mm/dma-mapping.c            |   4 +-
>  arch/arm64/include/asm/dma-mapping.h |   2 +-
>  arch/arm64/mm/dma-mapping.c          |  16 +--
>  drivers/iommu/arm-smmu.c             | 186 ++++++++++++++++++++++++++++++--
>  drivers/iommu/iommu.c                |  49 ++++++++-
>  drivers/iommu/of_iommu.c             |  14 ++-
>  drivers/of/device.c                  |   9 +-
>  include/linux/dma-mapping.h          |   7 +-
>  include/linux/iommu.h                |   2 +
>  include/linux/of_device.h            |   4 +-
>  11 files changed, 268 insertions(+), 27 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laurent Pinchart]

Hello,

I haven't seen any reply to this e-mail. I know that the combination of IOMMU, 
DMA mapping and DT doesn't exactly sound like fun, but I think we still need 
to move on :-)

Will and Arnd, could you please confirm that my summary below matches your 
memories of our discussion ?

On Monday 16 February 2015 18:14:45 Laurent Pinchart wrote:
> On Thursday 05 February 2015 16:31:58 Laura Abbott wrote:
> > Hi,
> > 
> > On the heels of Exynos integrating SMMU in to the DT for probing,
> > this series attempts to add support to make SMMU drivers work with
> > deferred probing. This has been referenced before[1] but this is
> > some actual code to use as a starting point for discussion.
> > 
> > The requirement for this is based on a previous patch to add clock
> > support to the ARM SMMU driver[2]. Once we have clock support, it's
> > possible that the driver itself may need to be defered which breaks
> > a bunch of assumptions about how SMMU probing is supposed to work.
> > The concept here is to have the driver call of_dma_configure which
> > may return -EPROBE_DEFER. of_dma_configure could possibly be moved
> > up to probe level. The existing method of initialization still needs
> > to be done as well which might mean we have the worst of both worlds.
> > 
> > Open questions:
> > - This still doesn't really address Arnd's concerns[3] about disabling
> > IOMMUs
> 
> Arnd, Will and I have discussed IOMMU probe deferral last week. Here's a
> summary of the discussion, before the details blur away.
> 
> The discussion covered both higher level concepts and lower level details,
> in various directions. I'll try to make the summary clear by filling the
> gaps between pieces where needed, so some of the information below might
> not be a direct result of the discussions. Arnd and Will, please feel free
> to correct me.
> 
> The first question we've discussed was whether probe deferral for IOMMUs is
> really needed. Various dependencies between IOMMU devices and other devices
> exist, in particular on clocks (as you have mentioned above) and on power
> domains (as mentioned by Marek in
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.
> html). While there are mechanism to handle some of them with probe deferral
> (for instance by using the OF_DECLARE macros to register clock drivers),
> generalizing those mechanisms would essentially recreate a probe ordering
> mechanism similar to link order probe ordering and couldn't really scale.
> 
> Additionally, IOMMUs could also be present hot-pluggable devices and depend
> on resources that are thus hot-plugged. OF_DECLARE wouldn't help in that
> case. For all those reasons we have concluded that probe deferral for
> IOMMUs is desired if it can be implemented cleanly.
> 
> The second question we've discussed was how to implement probe deferral
> cleanly :-)
> 
> The current implementation configures DMA at device creation time and sets
> the following properties:
> 
> - dma_map_ops (IOMMU, SWIOTLB, linear mapping)
> - initial DMA mask
> - DMA offset
> - DMA coherency
> 
> Additionally the IOMMU group (when an IOMMU is present) will also be
> configured at the same time (patches are under review).
> 
> The base idea is to defer probing of bus master devices when their IOMMU
> isn't present. Three cases need to be handled.
> 
> 1. No IOMMU is declared by the firmware (through DT, ACPI, ...) for the bus
> master device. The bus master device probe doesn't need to be deferred due
> to the IOMMU. dma_map_ops must be set to SWIOTLB or linear mapping (the
> later should likely be implemented through SWIOTLB).
> 
> 2. An IOMMU is declared for the bus master device and the IOMMU has been
> successfully probed and registered. The bus master device probe doesn't need
> to be deferred due to the IOMMU. dma_map_ops must be set to IOMMU ops.
> 
> 3. An IOMMU is declared for the bus master device but the IOMMU isn't
> registered. This can be caused by different reasons:
> 
> - a. No driver is loaded for this IOMMU (for instance because DT describes
> the IOMMU connection, but the IOMMU driver hasn't been developed yet, or an
> older kernel is used). If the IOMMU is optional the bus master device probe
> should succeed, and dma_map_ops should be set to linear. If the IOMMU is
> mandatory the bus master device probe should fail.
> 
> Note that, as we require IOMMU drivers to be compiled in, we don't need to
> handle the case of loadable IOMMU drivers that haven't been loaded yet.
> 
> - b. A driver is loaded for this IOMMU but the IOMMU hasn't been probed yet,
> or its probe has been deferred. The bus master device probe should be
> deferred.
> 
> - c. A driver is loaded for this IOMMU but the IOMMU probe has failed
> permanently. It's not clear at the moment whether we should try to recover
> from this automatically using the same mechanism as case 3.a, or if we can
> considered this as an abnormal failure and disable the bus master device.
> 
> Assuming that we should try to recover from such an error, we can't predict
> this case when the device is instantiated (even if IOMMUs are registered
> before bus master devices are added, for instance using the OF_DECLARE
> mechanism that Will has implemented). We thus need to setup the dma_map_ops
> and IOMMU group at bus master device probe time.
> 
> Furthermore, we can't distinguish cases 3.a and 3.b at bus master probe time
> without early registration of IOMMU drivers. Case 3.a would instead be
> considered as 3.b, leading to infinite probe deferral of bus master
> devices.
> 
> For those reasons we have concluded that IOMMU probe deferral needs to be
> implemented with a combination of several mechanisms. The following steps
> should happen at bus master device probe time.
> 
> 1. The IOMMU device referenced by the firmware with the bus master device is
> looked up. On DT-based systems, this will be the IOMMU DT node referenced
> by the iommus property. If no IOMMU device is associated, dma_map_ops will
> be set to linear mapping or SWIOTLB and device probe will continue.
> 
> 2. An IOMMU device is referenced for the bus master device.
> 
> The corresponding IOMMU instance is looked up. This requires a new IOMMU
> registration system. IOMMU drivers will create IOMMU instances at probe time
> and register them with the IOMMU core.
> 
> If an IOMMU instance is found for the referenced IOMMU device, the IOMMU
> instance's of_xlate function will be called to setup the IOMMU.
> 
> If the of_xlate call succeeds dma_map_ops will be set to IOMMU ops and
> device probe will continue. If the call fails we can either fail the bus
> master device probe, or fall back to non-IOMMU dma_map_ops (to be
> discussed).
> 
> 3. The IOMMU device referenced for the bus master device isn't present, due
> to the IOMMU device probe not having been performed yet, having been
> deferred, or having failed.
> 
> The IOMMU driver associated with the IOMMU device is looked up. This was
> initially thought to require an early registration mechanism for IOMMU
> drivers (using an OF_DECLARE mechanism for DT-based systems for instance),
> but on second thought it might be possible to implement this based on
> normal driver registration (to be researched).
> 
> If an IOMMU driver is found for the referenced IOMMU device, a callback
> function of the IOMMU driver is called to check whether an IOMMU instance is
> expected to be registered later (most IOMMU drivers will just return true,
> so we could skip this callback function until an IOMMU driver requires it).
> If an IOMMU instance is expected to be registered later the bus master
> device probe is deferred. Otherwise dma_map_ops will be set to linear
> mapping/SWIOTLB and device probe will continue.
> 
> 
> The initial DMA mask and the DMA offset can still be configured at device
> instantiation time if desired.
> 
> > - Currently tested where we knew the driver was going to be deferring.
> > Probably need some logic for calling of_dma_configure again.
> > 
> > This is based on Robin Murphy's work for dma mapping[4] and a few
> > patches from Murali Kaicheri[5] for of_dma_configure.
> > 
> > 
> > [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-> > January/011764.html
> > [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-> > August/279036.html
> > [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-> > December/311579.html
> > [4] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-> > January/315459.html
> > [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-> > January/319390.html
> > 
> > Laura Abbott (3):
> >   dma-mapping: Make arch_setup_dma_ops return an error code
> >   of: Return error codes from of_dma_configure
> >   iommu/arm-smmu: Support deferred probing
> > 
> > Mitchel Humpherys (1):
> >   iommu/arm-smmu: add support for specifying clocks
> >  
> >  arch/arm/include/asm/dma-mapping.h   |   2 +-
> >  arch/arm/mm/dma-mapping.c            |   4 +-
> >  arch/arm64/include/asm/dma-mapping.h |   2 +-
> >  arch/arm64/mm/dma-mapping.c          |  16 +--
> >  drivers/iommu/arm-smmu.c             | 186 ++++++++++++++++++++++++++++--
> >  drivers/iommu/iommu.c                |  49 ++++++++-
> >  drivers/iommu/of_iommu.c             |  14 ++-
> >  drivers/of/device.c                  |   9 +-
> >  include/linux/dma-mapping.h          |   7 +-
> >  include/linux/iommu.h                |   2 +
> >  include/linux/of_device.h            |   4 +-
> >  11 files changed, 268 insertions(+), 27 deletions(-)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Will Deacon]

On Tue, Mar 03, 2015 at 11:54:46PM +0000, Laurent Pinchart wrote:
> Hello,

Hi Laurent,

> I haven't seen any reply to this e-mail. I know that the combination of IOMMU, 
> DMA mapping and DT doesn't exactly sound like fun, but I think we still need 
> to move on :-)

Yup, and thanks for taking the time to write this up!

> Will and Arnd, could you please confirm that my summary below matches your 
> memories of our discussion ?

It certainly matches my recollection, but I suspect the devil is in the
details and we'll have to have some discussions once somebody has a crack at
implementing this.

Will

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laurent Pinchart]

Hi Will,

(CC'ing Thierry Reding who was missing. Thierry, you'll have to look at the 
archive for the discussion, but here's at least a start pointer)

On Wednesday 04 March 2015 15:25:05 Will Deacon wrote:
> On Tue, Mar 03, 2015 at 11:54:46PM +0000, Laurent Pinchart wrote:
> > Hello,
> 
> Hi Laurent,
> 
> > I haven't seen any reply to this e-mail. I know that the combination of
> > IOMMU, DMA mapping and DT doesn't exactly sound like fun, but I think we
> > still need to move on :-)
> 
> Yup, and thanks for taking the time to write this up!
> 
> > Will and Arnd, could you please confirm that my summary below matches your
> > memories of our discussion ?
> 
> It certainly matches my recollection, but I suspect the devil is in the
> details and we'll have to have some discussions once somebody has a crack at
> implementing this.

You're not volunteering, are you ? ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Marek Szyprowski]

Hello,

On 2015-02-16 17:14, Laurent Pinchart wrote:
> Hi Laura and all,
>
> On Thursday 05 February 2015 16:31:58 Laura Abbott wrote:
>> Hi,
>>
>> On the heels of Exynos integrating SMMU in to the DT for probing,
>> this series attempts to add support to make SMMU drivers work with
>> deferred probing. This has been referenced before[1] but this is
>> some actual code to use as a starting point for discussion.
>>
>> The requirement for this is based on a previous patch to add clock
>> support to the ARM SMMU driver[2]. Once we have clock support, it's
>> possible that the driver itself may need to be defered which breaks
>> a bunch of assumptions about how SMMU probing is supposed to work.
>> The concept here is to have the driver call of_dma_configure which
>> may return -EPROBE_DEFER. of_dma_configure could possibly be moved
>> up to probe level. The existing method of initialization still needs
>> to be done as well which might mean we have the worst of both worlds.
>>
>> Open questions:
>> - This still doesn't really address Arnd's concerns[3] about disabling
>> IOMMUs
> Arnd, Will and I have discussed IOMMU probe deferral last week. Here's a
> summary of the discussion, before the details blur away.
>
> The discussion covered both higher level concepts and lower level details, in
> various directions. I'll try to make the summary clear by filling the gaps
> between pieces where needed, so some of the information below might not be a
> direct result of the discussions. Arnd and Will, please feel free to correct
> me.
>
> The first question we've discussed was whether probe deferral for IOMMUs is
> really needed. Various dependencies between IOMMU devices and other devices
> exist, in particular on clocks (as you have mentioned above) and on power
> domains (as mentioned by Marek in http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html). While there are mechanism to handle
> some of them with probe deferral (for instance by using the OF_DECLARE macros
> to register clock drivers), generalizing those mechanisms would essentially
> recreate a probe ordering mechanism similar to link order probe ordering and
> couldn't really scale.
>
> Additionally, IOMMUs could also be present hot-pluggable devices and depend on
> resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case.
> For all those reasons we have concluded that probe deferral for IOMMUs is
> desired if it can be implemented cleanly.
>
> The second question we've discussed was how to implement probe deferral
> cleanly :-)
>
> The current implementation configures DMA at device creation time and sets the
> following properties:
>
> - dma_map_ops (IOMMU, SWIOTLB, linear mapping)
> - initial DMA mask
> - DMA offset
> - DMA coherency
>
> Additionally the IOMMU group (when an IOMMU is present) will also be
> configured at the same time (patches are under review).
>
> The base idea is to defer probing of bus master devices when their IOMMU isn't
> present. Three cases need to be handled.
>
> 1. No IOMMU is declared by the firmware (through DT, ACPI, ...) for the bus
> master device. The bus master device probe doesn't need to be deferred due to
> the IOMMU. dma_map_ops must be set to SWIOTLB or linear mapping (the later
> should likely be implemented through SWIOTLB).
>
> 2. An IOMMU is declared for the bus master device and the IOMMU has been
> successfully probed and registered. The bus master device probe doesn't need
> to be deferred due to the IOMMU. dma_map_ops must be set to IOMMU ops.
>
> 3. An IOMMU is declared for the bus master device but the IOMMU isn't
> registered. This can be caused by different reasons:
>
> - a. No driver is loaded for this IOMMU (for instance because DT describes the
> IOMMU connection, but the IOMMU driver hasn't been developed yet, or an older
> kernel is used). If the IOMMU is optional the bus master device probe should
> succeed, and dma_map_ops should be set to linear. If the IOMMU is mandatory
> the bus master device probe should fail.
>
> Note that, as we require IOMMU drivers to be compiled in, we don't need to
> handle the case of loadable IOMMU drivers that haven't been loaded yet.
>
> - b. A driver is loaded for this IOMMU but the IOMMU hasn't been probed yet,
> or its probe has been deferred. The bus master device probe should be
> deferred.
>
> - c. A driver is loaded for this IOMMU but the IOMMU probe has failed
> permanently. It's not clear at the moment whether we should try to recover
> from this automatically using the same mechanism as case 3.a, or if we can
> considered this as an abnormal failure and disable the bus master device.
>
> Assuming that we should try to recover from such an error, we can't predict
> this case when the device is instantiated (even if IOMMUs are registered
> before bus master devices are added, for instance using the OF_DECLARE
> mechanism that Will has implemented). We thus need to setup the dma_map_ops
> and IOMMU group at bus master device probe time.
>
> Furthermore, we can't distinguish cases 3.a and 3.b at bus master probe time
> without early registration of IOMMU drivers. Case 3.a would instead be
> considered as 3.b, leading to infinite probe deferral of bus master devices.
>
> For those reasons we have concluded that IOMMU probe deferral needs to be
> implemented with a combination of several mechanisms. The following steps
> should happen at bus master device probe time.
>
> 1. The IOMMU device referenced by the firmware with the bus master device is
> looked up. On DT-based systems, this will be the IOMMU DT node referenced by
> the iommus property. If no IOMMU device is associated, dma_map_ops will be set
> to linear mapping or SWIOTLB and device probe will continue.
>
> 2. An IOMMU device is referenced for the bus master device.
>
> The corresponding IOMMU instance is looked up. This requires a new IOMMU
> registration system. IOMMU drivers will create IOMMU instances at probe time
> and register them with the IOMMU core.
>
> If an IOMMU instance is found for the referenced IOMMU device, the IOMMU
> instance's of_xlate function will be called to setup the IOMMU.
>
> If the of_xlate call succeeds dma_map_ops will be set to IOMMU ops and device
> probe will continue. If the call fails we can either fail the bus master
> device probe, or fall back to non-IOMMU dma_map_ops (to be discussed).
>
> 3. The IOMMU device referenced for the bus master device isn't present, due to
> the IOMMU device probe not having been performed yet, having been deferred, or
> having failed.
>
> The IOMMU driver associated with the IOMMU device is looked up. This was
> initially thought to require an early registration mechanism for IOMMU drivers
> (using an OF_DECLARE mechanism for DT-based systems for instance), but on
> second thought it might be possible to implement this based on normal driver
> registration (to be researched).
>
> If an IOMMU driver is found for the referenced IOMMU device, a callback
> function of the IOMMU driver is called to check whether an IOMMU instance is
> expected to be registered later (most IOMMU drivers will just return true, so
> we could skip this callback function until an IOMMU driver requires it). If an
> IOMMU instance is expected to be registered later the bus master device probe
> is deferred. Otherwise dma_map_ops will be set to linear mapping/SWIOTLB and
> device probe will continue.
>
>
> The initial DMA mask and the DMA offset can still be configured at device
> instantiation time if desired.

I'm sorry for not participating in the discussions, I was terribly busy 
with our
internal stuff. The approach you have described looks fine although I 
would like
also to know a bit more about the roadmap of development. The IOMMU 
integration
is being discussed for over 2 years and right now we are STILL discussing.

Do you plan to post any patches implementing this approach? I would 
really like
to merge something simple, maybe not fully optimized and then resolve 
all the
corner cases and possible integration details.

>
>> - Currently tested where we knew the driver was going to be deferring.
>> Probably need some logic for calling of_dma_configure again.
>>
>> This is based on Robin Murphy's work for dma mapping[4] and a few
>> patches from Murali Kaicheri[5] for of_dma_configure.
>>
>>
>> [1]
>> http://lists.linuxfoundation.org/pipermail/iommu/2015-January/011764.html
>> [2]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279036.ht
>> ml [3]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311579.
>> html [4]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315459.h
>> tml [5]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/319390.h
>> tml
>>
>> Laura Abbott (3):
>>    dma-mapping: Make arch_setup_dma_ops return an error code
>>    of: Return error codes from of_dma_configure
>>    iommu/arm-smmu: Support deferred probing
>>
>> Mitchel Humpherys (1):
>>    iommu/arm-smmu: add support for specifying clocks
>>
>>   arch/arm/include/asm/dma-mapping.h   |   2 +-
>>   arch/arm/mm/dma-mapping.c            |   4 +-
>>   arch/arm64/include/asm/dma-mapping.h |   2 +-
>>   arch/arm64/mm/dma-mapping.c          |  16 +--
>>   drivers/iommu/arm-smmu.c             | 186 ++++++++++++++++++++++++++++++--
>>   drivers/iommu/iommu.c                |  49 ++++++++-
>>   drivers/iommu/of_iommu.c             |  14 ++-
>>   drivers/of/device.c                  |   9 +-
>>   include/linux/dma-mapping.h          |   7 +-
>>   include/linux/iommu.h                |   2 +
>>   include/linux/of_device.h            |   4 +-
>>   11 files changed, 268 insertions(+), 27 deletions(-)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH/RFC 0/4] Probe deferral for IOMMU DT integration

[Laurent Pinchart]

Hi Marek,

On Wednesday 04 March 2015 10:20:36 Marek Szyprowski wrote:
> On 2015-02-16 17:14, Laurent Pinchart wrote:
> > On Thursday 05 February 2015 16:31:58 Laura Abbott wrote:
> >> Hi,
> >> 
> >> On the heels of Exynos integrating SMMU in to the DT for probing,
> >> this series attempts to add support to make SMMU drivers work with
> >> deferred probing. This has been referenced before[1] but this is
> >> some actual code to use as a starting point for discussion.
> >> 
> >> The requirement for this is based on a previous patch to add clock
> >> support to the ARM SMMU driver[2]. Once we have clock support, it's
> >> possible that the driver itself may need to be defered which breaks
> >> a bunch of assumptions about how SMMU probing is supposed to work.
> >> The concept here is to have the driver call of_dma_configure which
> >> may return -EPROBE_DEFER. of_dma_configure could possibly be moved
> >> up to probe level. The existing method of initialization still needs
> >> to be done as well which might mean we have the worst of both worlds.
> >> 
> >> Open questions:
> >> - This still doesn't really address Arnd's concerns[3] about disabling
> >> IOMMUs
> > 
> > Arnd, Will and I have discussed IOMMU probe deferral last week. Here's a
> > summary of the discussion, before the details blur away.
> > 
> > The discussion covered both higher level concepts and lower level details,
> > in various directions. I'll try to make the summary clear by filling the
> > gaps between pieces where needed, so some of the information below might
> > not be a direct result of the discussions. Arnd and Will, please feel
> > free to correct me.
> > 
> > The first question we've discussed was whether probe deferral for IOMMUs
> > is really needed. Various dependencies between IOMMU devices and other
> > devices exist, in particular on clocks (as you have mentioned above) and
> > on power domains (as mentioned by Marek in
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/32323
> > 8.html). While there are mechanism to handle some of them with probe
> > deferral (for instance by using the OF_DECLARE macros to register clock
> > drivers), generalizing those mechanisms would essentially recreate a
> > probe ordering mechanism similar to link order probe ordering and
> > couldn't really scale.
> > 
> > Additionally, IOMMUs could also be present hot-pluggable devices and
> > depend on resources that are thus hot-plugged. OF_DECLARE wouldn't help
> > in that case. For all those reasons we have concluded that probe deferral
> > for IOMMUs is desired if it can be implemented cleanly.
> > 
> > The second question we've discussed was how to implement probe deferral
> > cleanly :-)
> > 
> > The current implementation configures DMA at device creation time and sets
> > the following properties:
> > 
> > - dma_map_ops (IOMMU, SWIOTLB, linear mapping)
> > - initial DMA mask
> > - DMA offset
> > - DMA coherency
> > 
> > Additionally the IOMMU group (when an IOMMU is present) will also be
> > configured at the same time (patches are under review).
> > 
> > The base idea is to defer probing of bus master devices when their IOMMU
> > isn't present. Three cases need to be handled.
> > 
> > 1. No IOMMU is declared by the firmware (through DT, ACPI, ...) for the
> > bus master device. The bus master device probe doesn't need to be deferred
> > due to the IOMMU. dma_map_ops must be set to SWIOTLB or linear mapping
> > (the later should likely be implemented through SWIOTLB).
> > 
> > 2. An IOMMU is declared for the bus master device and the IOMMU has been
> > successfully probed and registered. The bus master device probe doesn't
> > need to be deferred due to the IOMMU. dma_map_ops must be set to IOMMU
> > ops.
> > 
> > 3. An IOMMU is declared for the bus master device but the IOMMU isn't
> > registered. This can be caused by different reasons:
> > 
> > - a. No driver is loaded for this IOMMU (for instance because DT describes
> > the IOMMU connection, but the IOMMU driver hasn't been developed yet, or
> > an older kernel is used). If the IOMMU is optional the bus master device
> > probe should succeed, and dma_map_ops should be set to linear. If the
> > IOMMU is mandatory the bus master device probe should fail.
> > 
> > Note that, as we require IOMMU drivers to be compiled in, we don't need to
> > handle the case of loadable IOMMU drivers that haven't been loaded yet.
> > 
> > - b. A driver is loaded for this IOMMU but the IOMMU hasn't been probed
> > yet, or its probe has been deferred. The bus master device probe should
> > be deferred.
> > 
> > - c. A driver is loaded for this IOMMU but the IOMMU probe has failed
> > permanently. It's not clear at the moment whether we should try to recover
> > from this automatically using the same mechanism as case 3.a, or if we can
> > considered this as an abnormal failure and disable the bus master device.
> > 
> > Assuming that we should try to recover from such an error, we can't
> > predict this case when the device is instantiated (even if IOMMUs are
> > registered before bus master devices are added, for instance using the
> > OF_DECLARE mechanism that Will has implemented). We thus need to setup the
> > dma_map_ops and IOMMU group at bus master device probe time.
> > 
> > Furthermore, we can't distinguish cases 3.a and 3.b at bus master probe
> > time without early registration of IOMMU drivers. Case 3.a would instead
> > be considered as 3.b, leading to infinite probe deferral of bus master
> > devices.
> > 
> > For those reasons we have concluded that IOMMU probe deferral needs to be
> > implemented with a combination of several mechanisms. The following steps
> > should happen at bus master device probe time.
> > 
> > 1. The IOMMU device referenced by the firmware with the bus master device
> > is looked up. On DT-based systems, this will be the IOMMU DT node
> > referenced by the iommus property. If no IOMMU device is associated,
> > dma_map_ops will be set to linear mapping or SWIOTLB and device probe
> > will continue.
> > 
> > 2. An IOMMU device is referenced for the bus master device.
> > 
> > The corresponding IOMMU instance is looked up. This requires a new IOMMU
> > registration system. IOMMU drivers will create IOMMU instances at probe
> > time and register them with the IOMMU core.
> > 
> > If an IOMMU instance is found for the referenced IOMMU device, the IOMMU
> > instance's of_xlate function will be called to setup the IOMMU.
> > 
> > If the of_xlate call succeeds dma_map_ops will be set to IOMMU ops and
> > device probe will continue. If the call fails we can either fail the bus
> > master device probe, or fall back to non-IOMMU dma_map_ops (to be
> > discussed).
> > 
> > 3. The IOMMU device referenced for the bus master device isn't present,
> > due to the IOMMU device probe not having been performed yet, having been
> > deferred, or having failed.
> > 
> > The IOMMU driver associated with the IOMMU device is looked up. This was
> > initially thought to require an early registration mechanism for IOMMU
> > drivers (using an OF_DECLARE mechanism for DT-based systems for
> > instance), but on second thought it might be possible to implement this
> > based on normal driver registration (to be researched).
> > 
> > If an IOMMU driver is found for the referenced IOMMU device, a callback
> > function of the IOMMU driver is called to check whether an IOMMU instance
> > is expected to be registered later (most IOMMU drivers will just return
> > true, so we could skip this callback function until an IOMMU driver
> > requires it). If an IOMMU instance is expected to be registered later the
> > bus master device probe is deferred. Otherwise dma_map_ops will be set to
> > linear mapping/SWIOTLB and device probe will continue.
> > 
> > 
> > The initial DMA mask and the DMA offset can still be configured at device
> > instantiation time if desired.
> 
> I'm sorry for not participating in the discussions, I was terribly busy
> with our internal stuff.

No worries, I know the feeling. My reply is two months later :-/

> The approach you have described looks fine although I would like also to
> know a bit more about the roadmap of development. The IOMMU integration is
> being discussed for over 2 years and right now we are STILL discussing.
> 
> Do you plan to post any patches implementing this approach? I would
> really like to merge something simple, maybe not fully optimized and then
> resolve all the corner cases and possible integration details.

I secretly hoped that someone would beat me to it but that wasn't the case. I 
started giving it a go, and I'll try to post patches soon. It will be an RFC 
though.

> >> - Currently tested where we knew the driver was going to be deferring.
> >> Probably need some logic for calling of_dma_configure again.
> >> 
> >> This is based on Robin Murphy's work for dma mapping[4] and a few
> >> patches from Murali Kaicheri[5] for of_dma_configure.
> >> 
> >> 
> >> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-> >> January/011764.html
> >> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-> >> August/279036.html
> >> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-> >> December/311579.html
> >> [4] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-> >> January/315459.html
> >> [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-> >> January/319390.html
> >> 
> >> Laura Abbott (3):
> >>    dma-mapping: Make arch_setup_dma_ops return an error code
> >>    of: Return error codes from of_dma_configure
> >>    iommu/arm-smmu: Support deferred probing
> >> 
> >> Mitchel Humpherys (1):
> >>    iommu/arm-smmu: add support for specifying clocks
> >>   
> >>   arch/arm/include/asm/dma-mapping.h   |   2 +-
> >>   arch/arm/mm/dma-mapping.c            |   4 +-
> >>   arch/arm64/include/asm/dma-mapping.h |   2 +-
> >>   arch/arm64/mm/dma-mapping.c          |  16 +--
> >>   drivers/iommu/arm-smmu.c             | 186 ++++++++++++++++++++++++++--
> >>   drivers/iommu/iommu.c                |  49 ++++++++-
> >>   drivers/iommu/of_iommu.c             |  14 ++-
> >>   drivers/of/device.c                  |   9 +-
> >>   include/linux/dma-mapping.h          |   7 +-
> >>   include/linux/iommu.h                |   2 +
> >>   include/linux/of_device.h            |   4 +-
> >>   11 files changed, 268 insertions(+), 27 deletions(-)

-- 
Regards,

Laurent Pinchart