[PATCH v4 00/10] IOMMU probe deferral support

[PATCH v4 00/10] IOMMU probe deferral support

[Sricharan R]

This series calls the dma ops configuration for the devices
at a generic place so that it works for all busses.
The dma_configure_ops for a device is now called during
the device_attach callback just before the probe of the
bus/driver is called. Similarly dma_deconfigure is called during
device/driver_detach path.

pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
       |                         |
pci_bus_add_device     (device_add/driver_register)
       |                         |
device_attach           device_initial_probe
       |                         |
__device_attach_driver    __device_attach_driver
       |
driver_probe_device
       |
really_probe
       |
dma_configure

Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.

Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].

Tested with platform and pci devices for probe deferral
and reprobe on arm64 based platform. Added the patches [9,10],
    drivers: acpi: Configure acpi devices dma operation at probe time
    drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
for doing the dma ops configuration at probe time for acpi based platform
as well. Did not have a acpi based platform to test the changes and with
my still catching up knowledge on acpi/enumeration those two patches needs
to be reviewed/tested more.

Previous post of this series [5].

 [V4]
     * Took the reworked patches [2] from Robin's branch and
       rebased on top of Lorenzo's ACPI IORT ARM support series [3].

     * Added the patches for moving the dma ops configuration of
       acpi based devices to probe time as well.
 [V3]
     * Removed the patch to split dma_masks/dma_ops configuration
       separately based on review comments that both masks and ops are
       required only during the device probe time.

     * Reworked the series based on Generic DT bindings series.

     * Added call to iommu's remove_device in the cleanup path for arm and
       arm64.

     * Removed the notifier trick in arm64 to handle early device
       registration.

     * Added reset of dma_ops in cleanup path for arm based on comments.

     * Fixed the pci_iommu_configure path and tested with PCI device as
       well.
 
     * Fixed a bug to return the correct iommu_ops from patch 7 [4] in
       last post.

     * Fixed few other cosmetic comments.
  
 [V2]
     * Updated the Initial post to call dma_configure/deconfigure from
       generic code
 
     * Added iommu add_device callback from of_iommu_configure path

 [V1]
     * Initial post from Laurent Pinchart [1]

[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
[2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
[3] https://lkml.org/lkml/2016/11/21/141
[4] https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13940.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html

Laurent Pinchart (3):
  of: dma: Move range size workaround to of_dma_get_range()
  of: dma: Make of_dma_deconfigure() public
  iommu: of: Handle IOMMU lookup failure with deferred probing or error

Robin Murphy (3):
  iommu/of: Refactor of_iommu_configure() for error handling
  iommu/of: Prepare for deferred IOMMU configuration
  iommu/arm-smmu: Clean up early-probing workarounds

Sricharan R (4):
  drivers: platform: Configure dma operations at probe time
  arm64: dma-mapping: Remove the notifier trick to handle early setting
    of dma_ops
  drivers: acpi: Configure acpi devices dma operation at probe time
  drivers: acpi: Handle IOMMU lookup failure with deferred probing or
    error

 arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
 drivers/acpi/arm64/iort.c   |  17 +++++-
 drivers/acpi/glue.c         |   6 --
 drivers/acpi/scan.c         |   7 ++-
 drivers/base/dd.c           |  10 ++++
 drivers/base/dma-mapping.c  |  32 +++++++++++
 drivers/iommu/arm-smmu-v3.c |  35 +-----------
 drivers/iommu/arm-smmu.c    |  58 +++----------------
 drivers/iommu/dma-iommu.c   |   1 +
 drivers/iommu/of_iommu.c    | 114 +++++++++++++++++++++++++++-----------
 drivers/of/address.c        |  20 ++++++-
 drivers/of/device.c         |  36 ++++++------
 drivers/of/platform.c       |  10 +---
 drivers/pci/probe.c         |  17 +++---
 include/acpi/acpi_bus.h     |   2 +-
 include/linux/acpi.h        |   7 ++-
 include/linux/dma-mapping.h |   3 +
 include/linux/of_device.h   |  10 +++-
 include/linux/pci.h         |   5 ++
 19 files changed, 238 insertions(+), 284 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 01/10] iommu/of: Refactor of_iommu_configure() for error handling

[Sricharan R]

From: Robin Murphy <robin.murphy@arm.com>

In preparation for some upcoming cleverness, rework the control flow in
of_iommu_configure() to minimise duplication and improve the propogation
of errors. It's also as good a time as any to switch over from the
now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic
IOMMU instance interface directly.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/of_iommu.c | 83 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 0f57ddc..ee49081 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
+static const struct iommu_ops
+*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
+{
+	const struct iommu_ops *ops;
+	struct fwnode_handle *fwnode = &iommu_spec->np->fwnode;
+	int err;
+
+	ops = iommu_get_instance(fwnode);
+	if (!ops || !ops->of_xlate)
+		return NULL;
+
+	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
+	if (err)
+		return ERR_PTR(err);
+
+	err = ops->of_xlate(dev, iommu_spec);
+	if (err)
+		return ERR_PTR(err);
+
+	return ops;
+}
+
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 {
 	struct of_phandle_args *iommu_spec = data;
@@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 }
 
 static const struct iommu_ops
-*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node *bridge_np)
+*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np)
 {
 	const struct iommu_ops *ops;
 	struct of_phandle_args iommu_spec;
+	int err;
 
 	/*
 	 * Start by tracing the RID alias down the PCI topology as
@@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 	 * bus into the system beyond, and which IOMMU it ends up at.
 	 */
 	iommu_spec.np = NULL;
-	if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
-			   "iommu-map-mask", &iommu_spec.np, iommu_spec.args))
-		return NULL;
+	err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
+			     "iommu-map-mask", &iommu_spec.np,
+			     iommu_spec.args);
+	if (err)
+		return ERR_PTR(err);
 
-	ops = of_iommu_get_ops(iommu_spec.np);
-	if (!ops || !ops->of_xlate ||
-	    iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) ||
-	    ops->of_xlate(&pdev->dev, &iommu_spec))
-		ops = NULL;
+	ops = of_iommu_xlate(&pdev->dev, &iommu_spec);
 
 	of_node_put(iommu_spec.np);
 	return ops;
 }
 
-const struct iommu_ops *of_iommu_configure(struct device *dev,
-					   struct device_node *master_np)
+static const struct iommu_ops
+*of_platform_iommu_init(struct device *dev, struct device_node *np)
 {
 	struct of_phandle_args iommu_spec;
-	struct device_node *np;
 	const struct iommu_ops *ops = NULL;
 	int idx = 0;
 
-	if (dev_is_pci(dev))
-		return of_pci_iommu_configure(to_pci_dev(dev), master_np);
-
 	/*
 	 * We don't currently walk up the tree looking for a parent IOMMU.
 	 * See the `Notes:' section of
 	 * Documentation/devicetree/bindings/iommu/iommu.txt
 	 */
-	while (!of_parse_phandle_with_args(master_np, "iommus",
-					   "#iommu-cells", idx,
-					   &iommu_spec)) {
-		np = iommu_spec.np;
-		ops = of_iommu_get_ops(np);
-
-		if (!ops || !ops->of_xlate ||
-		    iommu_fwspec_init(dev, &np->fwnode, ops) ||
-		    ops->of_xlate(dev, &iommu_spec))
-			goto err_put_node;
-
-		of_node_put(np);
+	while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells",
+					   idx, &iommu_spec)) {
+		ops = of_iommu_xlate(dev, &iommu_spec);
+		of_node_put(iommu_spec.np);
 		idx++;
+		if (IS_ERR_OR_NULL(ops))
+			break;
 	}
 
 	return ops;
+}
+
+const struct iommu_ops *of_iommu_configure(struct device *dev,
+					   struct device_node *master_np)
+{
+	const struct iommu_ops *ops;
+
+	if (!master_np)
+		return NULL;
+
+	if (dev_is_pci(dev))
+		ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
+	else
+		ops = of_platform_iommu_init(dev, master_np);
 
-err_put_node:
-	of_node_put(np);
-	return NULL;
+	return IS_ERR(ops) ? NULL : ops;
 }
 
 static int __init of_iommu_init(void)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Sricharan R]

From: Robin Murphy <robin.murphy@arm.com>

IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.

Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 	int err;
 
 	ops = iommu_get_instance(fwnode);
-	if (!ops || !ops->of_xlate)
+	if ((ops && !ops->of_xlate) ||
+	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
 		return NULL;
 
 	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
 	if (err)
 		return ERR_PTR(err);
+	/*
+	 * The otherwise-empty fwspec handily serves to indicate the specific
+	 * IOMMU device we're waiting for, which will be useful if we ever get
+	 * a proper probe-ordering dependency mechanism in future.
+	 */
+	if (!ops)
+		return ERR_PTR(-EPROBE_DEFER);
 
 	err = ops->of_xlate(dev, iommu_spec);
 	if (err)
@@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
 					   struct device_node *master_np)
 {
 	const struct iommu_ops *ops;
+	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
 
 	if (!master_np)
 		return NULL;
 
+	if (fwspec) {
+		if (fwspec->ops)
+			return fwspec->ops;
+
+		/* In the deferred case, start again from scratch */
+		iommu_fwspec_free(dev);
+	}
+
 	if (dev_is_pci(dev))
 		ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
 	else
 		ops = of_platform_iommu_init(dev, master_np);
+	/*
+	 * If we have reason to believe the IOMMU driver missed the initial
+	 * add_device callback for dev, replay it to get things in order.
+	 */
+	if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+	    dev->bus && !dev->iommu_group) {
+		int err = ops->add_device(dev);
+
+		if (err)
+			ops = ERR_PTR(err);
+	}
 
 	return IS_ERR(ops) ? NULL : ops;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 03/10] of: dma: Move range size workaround to of_dma_get_range()

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Invalid dma-ranges values should be worked around when retrieving the
DMA range in of_dma_get_range(), not by all callers of the function.
This isn't much of a problem now that we have a single caller, but that
situation will change when moving DMA configuration to device probe
time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/address.c | 20 ++++++++++++++++++--
 drivers/of/device.c  | 15 ---------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..6aeb816 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -819,8 +819,8 @@ void __iomem *of_io_request_and_map(struct device_node *np, int index,
  *	CPU addr (phys_addr_t)	: pna cells
  *	size			: nsize cells
  *
- * It returns -ENODEV if "dma-ranges" property was not found
- * for this device in DT.
+ * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
+ * this device in DT, or -EINVAL if the CPU address or size is invalid.
  */
 int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
 {
@@ -880,6 +880,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
 	*dma_addr = dmaaddr;
 
 	*size = of_read_number(ranges + naddr + pna, nsize);
+	/*
+	 * DT nodes sometimes incorrectly set the size as a mask. Work around
+	 * those incorrect DT by computing the size as mask + 1.
+	 */
+	if (*size & 1) {
+		pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
+			__func__, *size, np->full_name);
+		*size = *size + 1;
+	}
+
+	if (!*size) {
+		pr_err("%s: invalid size zero for dma-range in node(%s)\n",
+		       __func__, np->full_name);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
 		 *dma_addr, *paddr, *size);
diff --git a/drivers/of/device.c b/drivers/of/device.c
index fd5cfad..d9898d9 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -110,21 +110,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 		size = dev->coherent_dma_mask + 1;
 	} else {
 		offset = PFN_DOWN(paddr - dma_addr);
-
-		/*
-		 * Add a work around to treat the size as mask + 1 in case
-		 * it is defined in DT as a mask.
-		 */
-		if (size & 1) {
-			dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
-				 size);
-			size = size + 1;
-		}
-
-		if (!size) {
-			dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
-			return;
-		}
 		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
 	}
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 04/10] of: dma: Make of_dma_deconfigure() public

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

As part of moving DMA initializing to probe time the
of_dma_deconfigure() function will need to be called from different
source files. Make it public and move it to drivers/of/device.c where
the of_dma_configure() function is.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/device.c       | 12 ++++++++++++
 drivers/of/platform.c     |  5 -----
 include/linux/of_device.h |  3 +++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index d9898d9..1c843e2 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -136,6 +136,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
 
+/**
+ * of_dma_deconfigure - Clean up DMA configuration
+ * @dev:	Device for which to clean up DMA configuration
+ *
+ * Clean up all configuration performed by of_dma_configure_ops() and free all
+ * resources that have been allocated.
+ */
+void of_dma_deconfigure(struct device *dev)
+{
+	arch_teardown_dma_ops(dev);
+}
+
 int of_device_register(struct platform_device *pdev)
 {
 	device_initialize(&pdev->dev);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e4bf07d..fdb6f8e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -155,11 +155,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
 }
 EXPORT_SYMBOL(of_device_alloc);
 
-static void of_dma_deconfigure(struct device *dev)
-{
-	arch_teardown_dma_ops(dev);
-}
-
 /**
  * of_platform_device_create_pdata - Alloc, initialize and register an of_device
  * @np: pointer to node to create device for
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687..d20a31a 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -56,6 +56,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 }
 
 void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_deconfigure(struct device *dev);
 #else /* CONFIG_OF */
 
 static inline int of_driver_match_device(struct device *dev,
@@ -100,6 +101,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 }
 static inline void of_dma_configure(struct device *dev, struct device_node *np)
 {}
+static inline void of_dma_deconfigure(struct device *dev)
+{}
 #endif /* CONFIG_OF */
 
 #endif /* _LINUX_OF_DEVICE_H */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 05/10] drivers: platform: Configure dma operations at probe time

[Sricharan R]

Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet. The dma_configure for the device is now called
from the generic device_attach callback just before the bus/driver probe
is called. This way, configuring the DMA ops for the device would be called
at the same place for all bus_types, hence the deferred probing mechanism
should work for all buses as well.

pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
       |                         |
pci_bus_add_device     (device_add/driver_register)
       |                         |
device_attach           device_initial_probe
       |                         |
__device_attach_driver    __device_attach_driver
       |
driver_probe_device
       |
really_probe
       |
dma_configure

Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
[rm: PCI hacks]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/base/dd.c           | 10 ++++++++++
 drivers/base/dma-mapping.c  | 20 ++++++++++++++++++++
 drivers/of/platform.c       |  5 +----
 drivers/pci/probe.c         | 15 +++++++--------
 include/linux/dma-mapping.h |  3 +++
 include/linux/pci.h         |  5 +++++
 6 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d76cd97..6c0a885 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -19,6 +19,7 @@
 
 #include <linux/device.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/wait.h>
@@ -351,6 +352,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	if (ret)
 		goto pinctrl_bind_failed;
 
+	ret = dma_configure(dev);
+	if (ret)
+		goto dma_failed;
+
 	if (driver_sysfs_add(dev)) {
 		printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
 			__func__, dev_name(dev));
@@ -412,6 +417,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	goto done;
 
 probe_failed:
+	dma_deconfigure(dev);
+dma_failed:
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
@@ -796,6 +803,9 @@ static void __device_release_driver(struct device *dev)
 			dev->bus->remove(dev);
 		else if (drv->remove)
 			drv->remove(dev);
+
+		dma_deconfigure(dev);
+
 		devres_release_all(dev);
 		dev->driver = NULL;
 		dev_set_drvdata(dev, NULL);
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 8f8b68c..b2a5629 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -10,6 +10,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/gfp.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 
@@ -341,3 +342,22 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	vunmap(cpu_addr);
 }
 #endif
+
+/*
+ * Common configuration to enable DMA API use for a device
+ */
+#include <linux/pci.h>
+
+int dma_configure(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		pci_dma_configure(dev);
+	else if (dev->of_node)
+		of_dma_configure(dev, dev->of_node);
+	return 0;
+}
+
+void dma_deconfigure(struct device *dev)
+{
+	of_dma_deconfigure(dev);
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index fdb6f8e..b0a2f9e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/of_iommu.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
@@ -183,11 +184,9 @@ static struct platform_device *of_platform_device_create_pdata(
 
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
-	of_dma_configure(&dev->dev, dev->dev.of_node);
 	of_msi_configure(&dev->dev, dev->dev.of_node);
 
 	if (of_device_add(dev) != 0) {
-		of_dma_deconfigure(&dev->dev);
 		platform_device_put(dev);
 		goto err_clear_flag;
 	}
@@ -245,7 +244,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 		dev_set_name(&dev->dev, "%s", bus_id);
 	else
 		of_device_make_bus_id(&dev->dev);
-	of_dma_configure(&dev->dev, dev->dev.of_node);
 
 	/* Allow the HW Peripheral ID to be overridden */
 	prop = of_get_property(node, "arm,primecell-periphid", NULL);
@@ -539,7 +537,6 @@ static int of_platform_device_destroy(struct device *dev, void *data)
 		amba_device_unregister(to_amba_device(dev));
 #endif
 
-	of_dma_deconfigure(dev);
 	of_node_clear_flag(dev->of_node, OF_POPULATED);
 	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
 	return 0;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c29e07a..04af770 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1719,26 +1719,26 @@ static void pci_set_msi_domain(struct pci_dev *dev)
 
 /**
  * pci_dma_configure - Setup DMA configuration
- * @dev: ptr to pci_dev struct of the PCI device
+ * @dev: ptr to device struct of the PCI device
  *
  * Function to update PCI devices's DMA configuration using the same
  * info from the OF node or ACPI node of host bridge's parent (if any).
  */
-static void pci_dma_configure(struct pci_dev *dev)
+void pci_dma_configure(struct device *dev)
 {
-	struct device *bridge = pci_get_host_bridge_device(dev);
+	struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
 
 	if (IS_ENABLED(CONFIG_OF) &&
-		bridge->parent && bridge->parent->of_node) {
-			of_dma_configure(&dev->dev, bridge->parent->of_node);
+	    bridge->parent && bridge->parent->of_node) {
+		of_dma_configure(dev, bridge->parent->of_node);
 	} else if (has_acpi_companion(bridge)) {
 		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
 		enum dev_dma_attr attr = acpi_get_dma_attr(adev);
 
 		if (attr == DEV_DMA_NOT_SUPPORTED)
-			dev_warn(&dev->dev, "DMA not supported.\n");
+			dev_warn(dev, "DMA not supported.\n");
 		else
-			acpi_dma_configure(&dev->dev, attr);
+			acpi_dma_configure(dev, attr);
 	}
 
 	pci_put_host_bridge_device(bridge);
@@ -1757,7 +1757,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	dev->dev.dma_mask = &dev->dma_mask;
 	dev->dev.dma_parms = &dev->dma_parms;
 	dev->dev.coherent_dma_mask = 0xffffffffull;
-	pci_dma_configure(dev);
 
 	pci_set_dma_max_seg_size(dev, 65536);
 	pci_set_dma_seg_boundary(dev, 0xffffffff);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 08528af..6f3e6ca 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -702,6 +702,9 @@ void *dma_mark_declared_memory_occupied(struct device *dev,
 }
 #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
 
+int dma_configure(struct device *dev);
+void dma_deconfigure(struct device *dev);
+
 /*
  * Managed DMA API
  */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0e49f70..d04f651 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,6 +870,8 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
 #define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
 
+void pci_dma_configure(struct device *dev);
+
 /* Generic PCI functions exported to card drivers */
 
 enum pci_lost_interrupt_reason {
@@ -1601,6 +1603,9 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
 #define dev_is_pci(d) (false)
 #define dev_is_pf(d) (false)
 #define dev_num_vf(d) (0)
+
+static inline void pci_dma_configure(struct device *dev) { }
+
 #endif /* CONFIG_PCI */
 
 /* Include architecture-dependent settings and functions */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.

The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.

The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.

The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.

The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.

Signed-off-by: Laurent Pichart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
[rm: massive PCI hacks]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/base/dma-mapping.c | 4 ++--
 drivers/iommu/dma-iommu.c  | 1 +
 drivers/iommu/of_iommu.c   | 5 +++--
 drivers/of/device.c        | 9 +++++++--
 drivers/pci/probe.c        | 6 ++++--
 include/linux/of_device.h  | 9 ++++++---
 include/linux/pci.h        | 4 ++--
 7 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 int dma_configure(struct device *dev)
 {
 	if (dev_is_pci(dev))
-		pci_dma_configure(dev);
+		return pci_dma_configure(dev);
 	else if (dev->of_node)
-		of_dma_configure(dev, dev->of_node);
+		return of_dma_configure(dev, dev->of_node);
 	return 0;
 }
 
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	base_pfn = max_t(unsigned long, 1, base >> order);
 	end_pfn = (base + size - 1) >> order;
 
+	dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain->geometry.aperture_end, *dev->dma_mask, dev->coherent_dma_mask);
 	/* Check the domain allows at least some access to the device... */
 	if (domain->geometry.force_aperture) {
 		if (base > domain->geometry.aperture_end ||
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 349bd1d..9529d6c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_iommu.h>
 #include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <linux/slab.h>
 
 static const struct of_device_id __iommu_of_table_sentinel
@@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
 			ops = ERR_PTR(err);
 	}
 
-	return IS_ERR(ops) ? NULL : ops;
+	return ops;
 }
 
 static int __init of_iommu_init(void)
@@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
 	for_each_matching_node_and_match(np, matches, &match) {
 		const of_iommu_init_fn init_fn = match->data;
 
-		if (init_fn(np))
+		if (init_fn && init_fn(np))
 			pr_err("Failed to initialise IOMMU %s\n",
 				of_node_full_name(np));
 	}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 1c843e2..d58595c 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
  * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
  * 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;
@@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 	ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
 	if (ret < 0) {
 		dma_addr = offset = 0;
-		size = dev->coherent_dma_mask + 1;
+		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
 	} else {
 		offset = PFN_DOWN(paddr - dma_addr);
 		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
@@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 		coherent ? " " : " not ");
 
 	iommu = of_iommu_configure(dev, np);
+	if (IS_ERR(iommu))
+		return PTR_ERR(iommu);
+
 	dev_dbg(dev, "device is%sbehind an iommu\n",
 		iommu ? " " : " not ");
 
 	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 04af770..6316cae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
  * Function to update PCI devices's DMA configuration using the same
  * info from the OF node or ACPI node of host bridge's parent (if any).
  */
-void pci_dma_configure(struct device *dev)
+int pci_dma_configure(struct device *dev)
 {
 	struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
+	int ret = 0;
 
 	if (IS_ENABLED(CONFIG_OF) &&
 	    bridge->parent && bridge->parent->of_node) {
-		of_dma_configure(dev, bridge->parent->of_node);
+		ret = of_dma_configure(dev, bridge->parent->of_node);
 	} else if (has_acpi_companion(bridge)) {
 		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
 		enum dev_dma_attr attr = acpi_get_dma_attr(adev);
@@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
 	}
 
 	pci_put_host_bridge_device(bridge);
+	return ret;
 }
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index d20a31a..6dca65c 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -55,7 +55,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);
 void of_dma_deconfigure(struct device *dev);
 #else /* CONFIG_OF */
 
@@ -99,8 +99,11 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 {
 	return NULL;
 }
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
-{}
+
+static inline int of_dma_configure(struct device *dev, struct device_node *np)
+{
+	return 0;
+}
 static inline void of_dma_deconfigure(struct device *dev)
 {}
 #endif /* CONFIG_OF */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d04f651..989ca44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
 #define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
 
-void pci_dma_configure(struct device *dev);
+int pci_dma_configure(struct device *dev);
 
 /* Generic PCI functions exported to card drivers */
 
@@ -1604,7 +1604,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
 #define dev_is_pf(d) (false)
 #define dev_num_vf(d) (0)
 
-static inline void pci_dma_configure(struct device *dev) { }
+static inline int pci_dma_configure(struct device *dev) { return 0; }
 
 #endif /* CONFIG_PCI */
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 07/10] arm64: dma-mapping: Remove the notifier trick to handle early setting of dma_ops

[Sricharan R]

With arch_setup_dma_ops now being called late during device's probe after the
device's iommu is probed, the notifier trick required to handle the early
setup of dma_ops before the iommu group gets created is not required.
So removing the notifier's here.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
[rm: clean up even more]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
 1 file changed, 12 insertions(+), 120 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index aa6c8f8..401f79a 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -800,140 +800,32 @@ static void __iommu_unmap_sg_attrs(struct device *dev,
 	.mapping_error = iommu_dma_mapping_error,
 };
 
-/*
- * TODO: Right now __iommu_setup_dma_ops() gets called too early to do
- * everything it needs to - the device is only partially created and the
- * IOMMU driver hasn't seen it yet, so it can't have a group. Thus we
- * need this delayed attachment dance. Once IOMMU probe ordering is sorted
- * to move the arch_setup_dma_ops() call later, all the notifier bits below
- * become unnecessary, and will go away.
- */
-struct iommu_dma_notifier_data {
-	struct list_head list;
-	struct device *dev;
-	const struct iommu_ops *ops;
-	u64 dma_base;
-	u64 size;
-};
-static LIST_HEAD(iommu_dma_masters);
-static DEFINE_MUTEX(iommu_dma_notifier_lock);
-
-static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
-			   u64 dma_base, u64 size)
-{
-	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
-	/*
-	 * If the IOMMU driver has the DMA domain support that we require,
-	 * then the IOMMU core will have already configured a group for this
-	 * device, and allocated the default domain for that group.
-	 */
-	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
-		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
-			dev_name(dev));
-		return false;
-	}
-
-	dev->archdata.dma_ops = &iommu_dma_ops;
-	return true;
-}
-
-static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
-			      u64 dma_base, u64 size)
-{
-	struct iommu_dma_notifier_data *iommudata;
-
-	iommudata = kzalloc(sizeof(*iommudata), GFP_KERNEL);
-	if (!iommudata)
-		return;
-
-	iommudata->dev = dev;
-	iommudata->ops = ops;
-	iommudata->dma_base = dma_base;
-	iommudata->size = size;
-
-	mutex_lock(&iommu_dma_notifier_lock);
-	list_add(&iommudata->list, &iommu_dma_masters);
-	mutex_unlock(&iommu_dma_notifier_lock);
-}
-
-static int __iommu_attach_notifier(struct notifier_block *nb,
-				   unsigned long action, void *data)
-{
-	struct iommu_dma_notifier_data *master, *tmp;
-
-	if (action != BUS_NOTIFY_BIND_DRIVER)
-		return 0;
-
-	mutex_lock(&iommu_dma_notifier_lock);
-	list_for_each_entry_safe(master, tmp, &iommu_dma_masters, list) {
-		if (data == master->dev && do_iommu_attach(master->dev,
-				master->ops, master->dma_base, master->size)) {
-			list_del(&master->list);
-			kfree(master);
-			break;
-		}
-	}
-	mutex_unlock(&iommu_dma_notifier_lock);
-	return 0;
-}
-
-static int __init register_iommu_dma_ops_notifier(struct bus_type *bus)
-{
-	struct notifier_block *nb = kzalloc(sizeof(*nb), GFP_KERNEL);
-	int ret;
-
-	if (!nb)
-		return -ENOMEM;
-
-	nb->notifier_call = __iommu_attach_notifier;
-
-	ret = bus_register_notifier(bus, nb);
-	if (ret) {
-		pr_warn("Failed to register DMA domain notifier; IOMMU DMA ops unavailable on bus '%s'\n",
-			bus->name);
-		kfree(nb);
-	}
-	return ret;
-}
-
 static int __init __iommu_dma_init(void)
 {
-	int ret;
-
-	ret = iommu_dma_init();
-	if (!ret)
-		ret = register_iommu_dma_ops_notifier(&platform_bus_type);
-	if (!ret)
-		ret = register_iommu_dma_ops_notifier(&amba_bustype);
-#ifdef CONFIG_PCI
-	if (!ret)
-		ret = register_iommu_dma_ops_notifier(&pci_bus_type);
-#endif
-	return ret;
+	return iommu_dma_init();
 }
 arch_initcall(__iommu_dma_init);
 
 static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 				  const struct iommu_ops *ops)
 {
-	struct iommu_group *group;
+	struct iommu_domain *domain;
 
 	if (!ops)
 		return;
+
 	/*
-	 * TODO: As a concession to the future, we're ready to handle being
-	 * called both early and late (i.e. after bus_add_device). Once all
-	 * the platform bus code is reworked to call us late and the notifier
-	 * junk above goes away, move the body of do_iommu_attach here.
+	 * The IOMMU core code allocates the default DMA domain, which the
+	 * underlying IOMMU driver needs to support via the dma-iommu layer.
 	 */
-	group = iommu_group_get(dev);
-	if (group) {
-		do_iommu_attach(dev, ops, dma_base, size);
-		iommu_group_put(group);
-	} else {
-		queue_iommu_attach(dev, ops, dma_base, size);
+	domain = iommu_get_domain_for_dev(dev);
+	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
+		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+			dev_name(dev));
+		return;
 	}
+
+	dev->archdata.dma_ops = &iommu_dma_ops;
 }
 
 void arch_teardown_dma_ops(struct device *dev)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 08/10] iommu/arm-smmu: Clean up early-probing workarounds

[Sricharan R]

From: Robin Murphy <robin.murphy@arm.com>

Now that the appropriate ordering is enforced via profe-deferral of
masters in core code, rip it all out and bask in the simplicity.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
[sricharan: Rebased on top of ACPI IORT SMMU series]
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
[v4] Removed arm_smmu_acpi_init function and let the
     module_platform_driver to take care of the driver
     registration.

 drivers/iommu/arm-smmu-v3.c | 35 ++--------------------------
 drivers/iommu/arm-smmu.c    | 56 ++++++++-------------------------------------
 2 files changed, 11 insertions(+), 80 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d22c428..257a6a3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2729,40 +2729,9 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
 	.probe	= arm_smmu_device_probe,
 	.remove	= arm_smmu_device_remove,
 };
+module_platform_driver(arm_smmu_driver);
 
-static int __init arm_smmu_init(void)
-{
-	static bool registered;
-	int ret = 0;
-
-	if (!registered) {
-		ret = platform_driver_register(&arm_smmu_driver);
-		registered = !ret;
-	}
-	return ret;
-}
-
-static void __exit arm_smmu_exit(void)
-{
-	return platform_driver_unregister(&arm_smmu_driver);
-}
-
-subsys_initcall(arm_smmu_init);
-module_exit(arm_smmu_exit);
-
-static int __init arm_smmu_of_init(struct device_node *np)
-{
-	int ret = arm_smmu_init();
-
-	if (ret)
-		return ret;
-
-	if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
-		return -ENODEV;
-
-	return 0;
-}
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL);
 
 #ifdef CONFIG_ACPI
 static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8e66b16..eaa8f44 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2127,57 +2127,19 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
 	.probe	= arm_smmu_device_probe,
 	.remove	= arm_smmu_device_remove,
 };
-
-static int __init arm_smmu_init(void)
-{
-	static bool registered;
-	int ret = 0;
-
-	if (!registered) {
-		ret = platform_driver_register(&arm_smmu_driver);
-		registered = !ret;
-	}
-	return ret;
-}
-
-static void __exit arm_smmu_exit(void)
-{
-	return platform_driver_unregister(&arm_smmu_driver);
-}
-
-subsys_initcall(arm_smmu_init);
-module_exit(arm_smmu_exit);
-
-static int __init arm_smmu_of_init(struct device_node *np)
-{
-	int ret = arm_smmu_init();
-
-	if (ret)
-		return ret;
-
-	if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
-		return -ENODEV;
-
-	return 0;
-}
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", arm_smmu_of_init);
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", arm_smmu_of_init);
+module_platform_driver(arm_smmu_driver);
 
 #ifdef CONFIG_ACPI
-static int __init arm_smmu_acpi_init(struct acpi_table_header *table)
-{
-	if (iort_node_match(ACPI_IORT_NODE_SMMU))
-		return arm_smmu_init();
-
-	return 0;
-}
-IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, arm_smmu_acpi_init);
+IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, NULL);
 #endif
 
+IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", NULL);
+IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", NULL);
+IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", NULL);
+IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", NULL);
+IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", NULL);
+IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", NULL);
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 09/10] drivers: acpi: Configure acpi devices dma operation at probe time

[Sricharan R]

With all the DT based devices getting their dma ops configured
during probe time to have the right iommu setup, let us do the
same for acpi based devices as well.

Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet. The dma_configure for the device is now called
from the generic device_attach callback just before the bus/driver probe
is called. This way, configuring the DMA ops for the device would be called
at the same place for all bus_types, hence the deferred probing mechanism
should work for all buses as well.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/acpi/glue.c        |  6 ------
 drivers/base/dma-mapping.c | 12 ++++++++++++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index f8d6564..458445f 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -168,7 +168,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	struct list_head *physnode_list;
 	unsigned int node_id;
 	int retval = -EINVAL;
-	enum dev_dma_attr attr;
 
 	if (has_acpi_companion(dev)) {
 		if (acpi_dev) {
@@ -225,10 +224,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	if (!has_acpi_companion(dev))
 		ACPI_COMPANION_SET(dev, acpi_dev);
 
-	attr = acpi_get_dma_attr(acpi_dev);
-	if (attr != DEV_DMA_NOT_SUPPORTED)
-		acpi_dma_configure(dev, attr);
-
 	acpi_physnode_link_name(physical_node_name, node_id);
 	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
 				   physical_node_name);
@@ -250,7 +245,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	return 0;
 
  err:
-	acpi_dma_deconfigure(dev);
 	ACPI_COMPANION_SET(dev, NULL);
 	put_device(dev);
 	put_device(&acpi_dev->dev);
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 576fdfb..2ec4dae 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -347,17 +347,29 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
  * Common configuration to enable DMA API use for a device
  */
 #include <linux/pci.h>
+#include <linux/acpi.h>
 
 int dma_configure(struct device *dev)
 {
+	struct acpi_device *adev;
+	enum dev_dma_attr attr;
+
 	if (dev_is_pci(dev))
 		return pci_dma_configure(dev);
 	else if (dev->of_node)
 		return of_dma_configure(dev, dev->of_node);
+	else if (has_acpi_companion(dev)) {
+		adev = to_acpi_device_node(dev->fwnode);
+		attr = acpi_get_dma_attr(adev);
+		if (attr != DEV_DMA_NOT_SUPPORTED)
+			return acpi_dma_configure(dev, attr);
+	}
+
 	return 0;
 }
 
 void dma_deconfigure(struct device *dev)
 {
 	of_dma_deconfigure(dev);
+	acpi_dma_deconfigure(dev);
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 10/10] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error

[Sricharan R]

This is an equivalent to the DT's handling of the iommu master's probe
with deferred probing when the corrsponding iommu is not probed yet.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.

The first case occurs when the firmware describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.

The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.

The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/acpi/arm64/iort.c | 17 ++++++++++++++++-
 drivers/acpi/scan.c       |  7 +++++--
 drivers/pci/probe.c       |  2 +-
 include/acpi/acpi_bus.h   |  2 +-
 include/linux/acpi.h      |  7 +++++--
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 47bace8..18ad4d9 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -537,8 +537,9 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
 			return NULL;
 
 		ops = iommu_get_instance(iort_fwnode);
+		/* This means that the iommu driver is still not probed */
 		if (!ops)
-			return NULL;
+			return ERR_PTR(-EPROBE_DEFER);
 
 		ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
 	}
@@ -590,12 +591,26 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
 
 		while (parent) {
 			ops = iort_iommu_xlate(dev, parent, streamid);
+			if (IS_ERR_OR_NULL(ops))
+				return ops;
 
 			parent = iort_node_get_id(node, &streamid,
 						  IORT_IOMMU_TYPE, i++);
 		}
 	}
 
+	/*
+	 * If we have reason to believe the IOMMU driver missed the initial
+	 * add_device callback for dev, replay it to get things in order.
+	 */
+	if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+	    dev->bus && !dev->iommu_group) {
+		int err = ops->add_device(dev);
+
+		if (err)
+			ops = ERR_PTR(err);
+	}
+
 	return ops;
 }
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 80698d3..c80e51e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1376,7 +1376,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
  * @dev: The pointer to the device
  * @attr: device dma attributes
  */
-void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
+int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
 {
 	const struct iommu_ops *iommu;
 
@@ -1395,13 +1395,16 @@ void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
 		dev->dma_mask = &dev->coherent_dma_mask;
 
 	iommu = iort_iommu_configure(dev);
-
+	if (IS_ERR(iommu))
+		return PTR_ERR(iommu);
 	/*
 	 * Assume dma valid range starts at 0 and covers the whole
 	 * coherent_dma_mask.
 	 */
 	arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
 			   attr == DEV_DMA_COHERENT);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(acpi_dma_configure);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6316cae..e32c7e5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1739,7 +1739,7 @@ int pci_dma_configure(struct device *dev)
 		if (attr == DEV_DMA_NOT_SUPPORTED)
 			dev_warn(dev, "DMA not supported.\n");
 		else
-			acpi_dma_configure(dev, attr);
+			ret = acpi_dma_configure(dev, attr);
 	}
 
 	pci_put_host_bridge_device(bridge);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..9aa762f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -573,7 +573,7 @@ struct acpi_pci_root {
 
 bool acpi_dma_supported(struct acpi_device *adev);
 enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
-void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
+int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
 void acpi_dma_deconfigure(struct device *dev);
 
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 8d15fc5..0e8d33f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -765,8 +765,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 	return DEV_DMA_NOT_SUPPORTED;
 }
 
-static inline void acpi_dma_configure(struct device *dev,
-				      enum dev_dma_attr attr) { }
+static inline int acpi_dma_configure(struct device *dev,
+				     enum dev_dma_attr attr)
+{
+	return 0;
+}
 
 static inline void acpi_dma_deconfigure(struct device *dev) { }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Marek Szyprowski]

Hi Sricharan and Robin,


On 2016-11-30 01:22, Sricharan R wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>
> Failures to look up an IOMMU when parsing the DT iommus property need to
> be handled separately from the .of_xlate() failures to support deferred
> probing.
>
> The lack of a registered IOMMU can be caused by the lack of a driver for
> the IOMMU, the IOMMU device probe not having been performed yet, having
> been deferred, or having failed.
>
> The first case occurs when the device tree describes the bus master and
> IOMMU topology correctly but no device driver exists for the IOMMU yet
> or the device driver has not been compiled in. Return NULL, the caller
> will configure the device without an IOMMU.
>
> The second and third cases are handled by deferring the probe of the bus
> master device which will eventually get reprobed after the IOMMU.
>
> The last case is currently handled by deferring the probe of the bus
> master device as well. A mechanism to either configure the bus master
> device without an IOMMU or to fail the bus master device probe depending
> on whether the IOMMU is optional or mandatory would be a good
> enhancement.
>
> Signed-off-by: Laurent Pichart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> [rm: massive PCI hacks]
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>   drivers/base/dma-mapping.c | 4 ++--
>   drivers/iommu/dma-iommu.c  | 1 +
>   drivers/iommu/of_iommu.c   | 5 +++--
>   drivers/of/device.c        | 9 +++++++--
>   drivers/pci/probe.c        | 6 ++++--
>   include/linux/of_device.h  | 9 ++++++---
>   include/linux/pci.h        | 4 ++--
>   7 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index b2a5629..576fdfb 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>   int dma_configure(struct device *dev)
>   {
>   	if (dev_is_pci(dev))
> -		pci_dma_configure(dev);
> +		return pci_dma_configure(dev);
>   	else if (dev->of_node)
> -		of_dma_configure(dev, dev->of_node);
> +		return of_dma_configure(dev, dev->of_node);
>   	return 0;
>   }
>   
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c5ab866..d2a7a46 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   	base_pfn = max_t(unsigned long, 1, base >> order);
>   	end_pfn = (base + size - 1) >> order;
>   
> +	dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain->geometry.aperture_end,

This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or 
debugging stage.

> *dev->dma_mask, dev->coherent_dma_mask);
>   	/* Check the domain allows at least some access to the device... */
>   	if (domain->geometry.force_aperture) {
>   		if (base > domain->geometry.aperture_end ||
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 349bd1d..9529d6c 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -23,6 +23,7 @@
>   #include <linux/of.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_pci.h>
> +#include <linux/pci.h>
>   #include <linux/slab.h>
>   
>   static const struct of_device_id __iommu_of_table_sentinel
> @@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>   			ops = ERR_PTR(err);
>   	}
>   
> -	return IS_ERR(ops) ? NULL : ops;
> +	return ops;
>   }
>   
>   static int __init of_iommu_init(void)
> @@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
>   	for_each_matching_node_and_match(np, matches, &match) {
>   		const of_iommu_init_fn init_fn = match->data;
>   
> -		if (init_fn(np))
> +		if (init_fn && init_fn(np))
>   			pr_err("Failed to initialise IOMMU %s\n",
>   				of_node_full_name(np));
>   	}
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 1c843e2..d58595c 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
>    * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
>    * 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;
> @@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>   	ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>   	if (ret < 0) {
>   		dma_addr = offset = 0;
> -		size = dev->coherent_dma_mask + 1;
> +		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
>   	} else {
>   		offset = PFN_DOWN(paddr - dma_addr);
>   		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> @@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>   		coherent ? " " : " not ");
>   
>   	iommu = of_iommu_configure(dev, np);
> +	if (IS_ERR(iommu))
> +		return PTR_ERR(iommu);
> +
>   	dev_dbg(dev, "device is%sbehind an iommu\n",
>   		iommu ? " " : " not ");
>   
>   	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> +
> +	return 0;
>   }
>   EXPORT_SYMBOL_GPL(of_dma_configure);
>   
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 04af770..6316cae 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
>    * Function to update PCI devices's DMA configuration using the same
>    * info from the OF node or ACPI node of host bridge's parent (if any).
>    */
> -void pci_dma_configure(struct device *dev)
> +int pci_dma_configure(struct device *dev)
>   {
>   	struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> +	int ret = 0;
>   
>   	if (IS_ENABLED(CONFIG_OF) &&
>   	    bridge->parent && bridge->parent->of_node) {
> -		of_dma_configure(dev, bridge->parent->of_node);
> +		ret = of_dma_configure(dev, bridge->parent->of_node);
>   	} else if (has_acpi_companion(bridge)) {
>   		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>   		enum dev_dma_attr attr = acpi_get_dma_attr(adev);
> @@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
>   	}
>   
>   	pci_put_host_bridge_device(bridge);
> +	return ret;
>   }
>   
>   void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index d20a31a..6dca65c 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -55,7 +55,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);
>   void of_dma_deconfigure(struct device *dev);
>   #else /* CONFIG_OF */
>   
> @@ -99,8 +99,11 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>   {
>   	return NULL;
>   }
> -static inline void of_dma_configure(struct device *dev, struct device_node *np)
> -{}
> +
> +static inline int of_dma_configure(struct device *dev, struct device_node *np)
> +{
> +	return 0;
> +}
>   static inline void of_dma_deconfigure(struct device *dev)
>   {}
>   #endif /* CONFIG_OF */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d04f651..989ca44 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
>   #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
>   #define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
>   
> -void pci_dma_configure(struct device *dev);
> +int pci_dma_configure(struct device *dev);
>   
>   /* Generic PCI functions exported to card drivers */
>   
> @@ -1604,7 +1604,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
>   #define dev_is_pf(d) (false)
>   #define dev_num_vf(d) (0)
>   
> -static inline void pci_dma_configure(struct device *dev) { }
> +static inline int pci_dma_configure(struct device *dev) { return 0; }
>   
>   #endif /* CONFIG_PCI */
>   

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

Re: [PATCH v4 00/10] IOMMU probe deferral support

[Marek Szyprowski]

Hi All,


On 2016-11-30 01:22, Sricharan R wrote:
> This series calls the dma ops configuration for the devices
> at a generic place so that it works for all busses.
> The dma_configure_ops for a device is now called during
> the device_attach callback just before the probe of the
> bus/driver is called. Similarly dma_deconfigure is called during
> device/driver_detach path.
>
> pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
>         |                         |
> pci_bus_add_device     (device_add/driver_register)
>         |                         |
> device_attach           device_initial_probe
>         |                         |
> __device_attach_driver    __device_attach_driver
>         |
> driver_probe_device
>         |
> really_probe
>         |
> dma_configure
>
> Similarly on the device/driver_unregister path __device_release_driver is
> called which inturn calls dma_deconfigure.
>
> Took the reworked patches [2] from Robin's branch and
> rebased on top of Lorenzo's ACPI IORT ARM support series [3].
>
> Tested with platform and pci devices for probe deferral
> and reprobe on arm64 based platform. Added the patches [9,10],
>      drivers: acpi: Configure acpi devices dma operation at probe time
>      drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
> for doing the dma ops configuration at probe time for acpi based platform
> as well. Did not have a acpi based platform to test the changes and with
> my still catching up knowledge on acpi/enumeration those two patches needs
> to be reviewed/tested more.

I've checked this patchset on my development boards: Odroid U3 
(ARM/Exynos4412),
XU3 (ARM/Exynos5422) and TM2 (ARM64/Exynos5433) and my queued patches 
for Exynos
IOMMU driver [1]. Besides the issue with an excessive dev_info, which 
causes NULL
pointer dereference (reported in reply to patch #6), it works fine on 
all those
platforms! Please add (again) my:

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

[1] https://www.spinics.net/lists/linux-samsung-soc/msg56354.html

> Previous post of this series [5].
>
>   [V4]
>       * Took the reworked patches [2] from Robin's branch and
>         rebased on top of Lorenzo's ACPI IORT ARM support series [3].
>
>       * Added the patches for moving the dma ops configuration of
>         acpi based devices to probe time as well.
>   [V3]
>       * Removed the patch to split dma_masks/dma_ops configuration
>         separately based on review comments that both masks and ops are
>         required only during the device probe time.
>
>       * Reworked the series based on Generic DT bindings series.
>
>       * Added call to iommu's remove_device in the cleanup path for arm and
>         arm64.
>
>       * Removed the notifier trick in arm64 to handle early device
>         registration.
>
>       * Added reset of dma_ops in cleanup path for arm based on comments.
>
>       * Fixed the pci_iommu_configure path and tested with PCI device as
>         well.
>   
>       * Fixed a bug to return the correct iommu_ops from patch 7 [4] in
>         last post.
>
>       * Fixed few other cosmetic comments.
>    
>   [V2]
>       * Updated the Initial post to call dma_configure/deconfigure from
>         generic code
>   
>       * Added iommu add_device callback from of_iommu_configure path
>
>   [V1]
>       * Initial post from Laurent Pinchart [1]
>
> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
> [2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
> [3] https://lkml.org/lkml/2016/11/21/141
> [4] https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13940.html
> [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html
>
> Laurent Pinchart (3):
>    of: dma: Move range size workaround to of_dma_get_range()
>    of: dma: Make of_dma_deconfigure() public
>    iommu: of: Handle IOMMU lookup failure with deferred probing or error
>
> Robin Murphy (3):
>    iommu/of: Refactor of_iommu_configure() for error handling
>    iommu/of: Prepare for deferred IOMMU configuration
>    iommu/arm-smmu: Clean up early-probing workarounds
>
> Sricharan R (4):
>    drivers: platform: Configure dma operations at probe time
>    arm64: dma-mapping: Remove the notifier trick to handle early setting
>      of dma_ops
>    drivers: acpi: Configure acpi devices dma operation at probe time
>    drivers: acpi: Handle IOMMU lookup failure with deferred probing or
>      error
>
>   arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
>   drivers/acpi/arm64/iort.c   |  17 +++++-
>   drivers/acpi/glue.c         |   6 --
>   drivers/acpi/scan.c         |   7 ++-
>   drivers/base/dd.c           |  10 ++++
>   drivers/base/dma-mapping.c  |  32 +++++++++++
>   drivers/iommu/arm-smmu-v3.c |  35 +-----------
>   drivers/iommu/arm-smmu.c    |  58 +++----------------
>   drivers/iommu/dma-iommu.c   |   1 +
>   drivers/iommu/of_iommu.c    | 114 +++++++++++++++++++++++++++-----------
>   drivers/of/address.c        |  20 ++++++-
>   drivers/of/device.c         |  36 ++++++------
>   drivers/of/platform.c       |  10 +---
>   drivers/pci/probe.c         |  17 +++---
>   include/acpi/acpi_bus.h     |   2 +-
>   include/linux/acpi.h        |   7 ++-
>   include/linux/dma-mapping.h |   3 +
>   include/linux/of_device.h   |  10 +++-
>   include/linux/pci.h         |   5 ++
>   19 files changed, 238 insertions(+), 284 deletions(-)
>

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

RE: [PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Sricharan]

Hi,

>Hi Sricharan and Robin,
>
>
>On 2016-11-30 01:22, Sricharan R wrote:
>> From: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>
>> Failures to look up an IOMMU when parsing the DT iommus property need to
>> be handled separately from the .of_xlate() failures to support deferred
>> probing.
>>
>> The lack of a registered IOMMU can be caused by the lack of a driver for
>> the IOMMU, the IOMMU device probe not having been performed yet, having
>> been deferred, or having failed.
>>
>> The first case occurs when the device tree describes the bus master and
>> IOMMU topology correctly but no device driver exists for the IOMMU yet
>> or the device driver has not been compiled in. Return NULL, the caller
>> will configure the device without an IOMMU.
>>
>> The second and third cases are handled by deferring the probe of the bus
>> master device which will eventually get reprobed after the IOMMU.
>>
>> The last case is currently handled by deferring the probe of the bus
>> master device as well. A mechanism to either configure the bus master
>> device without an IOMMU or to fail the bus master device probe depending
>> on whether the IOMMU is optional or mandatory would be a good
>> enhancement.
>>
>> Signed-off-by: Laurent Pichart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> [rm: massive PCI hacks]
>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>> ---
>>   drivers/base/dma-mapping.c | 4 ++--
>>   drivers/iommu/dma-iommu.c  | 1 +
>>   drivers/iommu/of_iommu.c   | 5 +++--
>>   drivers/of/device.c        | 9 +++++++--
>>   drivers/pci/probe.c        | 6 ++++--
>>   include/linux/of_device.h  | 9 ++++++---
>>   include/linux/pci.h        | 4 ++--
>>   7 files changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index b2a5629..576fdfb 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>>   int dma_configure(struct device *dev)
>>   {
>>   	if (dev_is_pci(dev))
>> -		pci_dma_configure(dev);
>> +		return pci_dma_configure(dev);
>>   	else if (dev->of_node)
>> -		of_dma_configure(dev, dev->of_node);
>> +		return of_dma_configure(dev, dev->of_node);
>>   	return 0;
>>   }
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index c5ab866..d2a7a46 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>   	base_pfn = max_t(unsigned long, 1, base >> order);
>>   	end_pfn = (base + size - 1) >> order;
>>
>> +	dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain-
>>geometry.aperture_end,
>
>This causes a NULL pointer dereference if caller passes NULL device pointer.
>There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
>Trivial to fix as it looks like a leftover from developement or
>debugging stage.
>
My bad, i should have removed it before posting. Noticed this, but missed to remove it.
Will do so.

Regards,
 Sricharan

RE: [PATCH v4 00/10] IOMMU probe deferral support

[Sricharan]

Hi Marek,

>
>On 2016-11-30 01:22, Sricharan R wrote:
>> This series calls the dma ops configuration for the devices
>> at a generic place so that it works for all busses.
>> The dma_configure_ops for a device is now called during
>> the device_attach callback just before the probe of the
>> bus/driver is called. Similarly dma_deconfigure is called during
>> device/driver_detach path.
>>
>> pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
>>         |                         |
>> pci_bus_add_device     (device_add/driver_register)
>>         |                         |
>> device_attach           device_initial_probe
>>         |                         |
>> __device_attach_driver    __device_attach_driver
>>         |
>> driver_probe_device
>>         |
>> really_probe
>>         |
>> dma_configure
>>
>> Similarly on the device/driver_unregister path __device_release_driver is
>> called which inturn calls dma_deconfigure.
>>
>> Took the reworked patches [2] from Robin's branch and
>> rebased on top of Lorenzo's ACPI IORT ARM support series [3].
>>
>> Tested with platform and pci devices for probe deferral
>> and reprobe on arm64 based platform. Added the patches [9,10],
>>      drivers: acpi: Configure acpi devices dma operation at probe time
>>      drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
>> for doing the dma ops configuration at probe time for acpi based platform
>> as well. Did not have a acpi based platform to test the changes and with
>> my still catching up knowledge on acpi/enumeration those two patches needs
>> to be reviewed/tested more.
>
>I've checked this patchset on my development boards: Odroid U3
>(ARM/Exynos4412),
>XU3 (ARM/Exynos5422) and TM2 (ARM64/Exynos5433) and my queued patches
>for Exynos
>IOMMU driver [1]. Besides the issue with an excessive dev_info, which
>causes NULL
>pointer dereference (reported in reply to patch #6), it works fine on
>all those
>platforms! Please add (again) my:
>
>Tested-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
>[1] https://www.spinics.net/lists/linux-samsung-soc/msg56354.html
>
>> Previous post of this series [5].

Thanks for the testing every time.

Regards,
 Sricharan

>>
>>   [V4]
>>       * Took the reworked patches [2] from Robin's branch and
>>         rebased on top of Lorenzo's ACPI IORT ARM support series [3].
>>
>>       * Added the patches for moving the dma ops configuration of
>>         acpi based devices to probe time as well.
>>   [V3]
>>       * Removed the patch to split dma_masks/dma_ops configuration
>>         separately based on review comments that both masks and ops are
>>         required only during the device probe time.
>>
>>       * Reworked the series based on Generic DT bindings series.
>>
>>       * Added call to iommu's remove_device in the cleanup path for arm and
>>         arm64.
>>
>>       * Removed the notifier trick in arm64 to handle early device
>>         registration.
>>
>>       * Added reset of dma_ops in cleanup path for arm based on comments.
>>
>>       * Fixed the pci_iommu_configure path and tested with PCI device as
>>         well.
>>
>>       * Fixed a bug to return the correct iommu_ops from patch 7 [4] in
>>         last post.
>>
>>       * Fixed few other cosmetic comments.
>>
>>   [V2]
>>       * Updated the Initial post to call dma_configure/deconfigure from
>>         generic code
>>
>>       * Added iommu add_device callback from of_iommu_configure path
>>
>>   [V1]
>>       * Initial post from Laurent Pinchart [1]
>>
>> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
>> [2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
>> [3] https://lkml.org/lkml/2016/11/21/141
>> [4] https://www.mail-archive.com/iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org/msg13940.html
>> [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html
>>
>> Laurent Pinchart (3):
>>    of: dma: Move range size workaround to of_dma_get_range()
>>    of: dma: Make of_dma_deconfigure() public
>>    iommu: of: Handle IOMMU lookup failure with deferred probing or error
>>
>> Robin Murphy (3):
>>    iommu/of: Refactor of_iommu_configure() for error handling
>>    iommu/of: Prepare for deferred IOMMU configuration
>>    iommu/arm-smmu: Clean up early-probing workarounds
>>
>> Sricharan R (4):
>>    drivers: platform: Configure dma operations at probe time
>>    arm64: dma-mapping: Remove the notifier trick to handle early setting
>>      of dma_ops
>>    drivers: acpi: Configure acpi devices dma operation at probe time
>>    drivers: acpi: Handle IOMMU lookup failure with deferred probing or
>>      error
>>
>>   arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
>>   drivers/acpi/arm64/iort.c   |  17 +++++-
>>   drivers/acpi/glue.c         |   6 --
>>   drivers/acpi/scan.c         |   7 ++-
>>   drivers/base/dd.c           |  10 ++++
>>   drivers/base/dma-mapping.c  |  32 +++++++++++
>>   drivers/iommu/arm-smmu-v3.c |  35 +-----------
>>   drivers/iommu/arm-smmu.c    |  58 +++----------------
>>   drivers/iommu/dma-iommu.c   |   1 +
>>   drivers/iommu/of_iommu.c    | 114 +++++++++++++++++++++++++++-----------
>>   drivers/of/address.c        |  20 ++++++-
>>   drivers/of/device.c         |  36 ++++++------
>>   drivers/of/platform.c       |  10 +---
>>   drivers/pci/probe.c         |  17 +++---
>>   include/acpi/acpi_bus.h     |   2 +-
>>   include/linux/acpi.h        |   7 ++-
>>   include/linux/dma-mapping.h |   3 +
>>   include/linux/of_device.h   |  10 +++-
>>   include/linux/pci.h         |   5 ++
>>   19 files changed, 238 insertions(+), 284 deletions(-)
>>
>
>Best regards
>--
>Marek Szyprowski, PhD
>Samsung R&D Institute Poland
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Robin Murphy]

On 30/11/16 07:54, Marek Szyprowski wrote:
> Hi Sricharan and Robin,
> 
> 
> On 2016-11-30 01:22, Sricharan R wrote:
>> From: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>
>> Failures to look up an IOMMU when parsing the DT iommus property need to
>> be handled separately from the .of_xlate() failures to support deferred
>> probing.
>>
>> The lack of a registered IOMMU can be caused by the lack of a driver for
>> the IOMMU, the IOMMU device probe not having been performed yet, having
>> been deferred, or having failed.
>>
>> The first case occurs when the device tree describes the bus master and
>> IOMMU topology correctly but no device driver exists for the IOMMU yet
>> or the device driver has not been compiled in. Return NULL, the caller
>> will configure the device without an IOMMU.
>>
>> The second and third cases are handled by deferring the probe of the bus
>> master device which will eventually get reprobed after the IOMMU.
>>
>> The last case is currently handled by deferring the probe of the bus
>> master device as well. A mechanism to either configure the bus master
>> device without an IOMMU or to fail the bus master device probe depending
>> on whether the IOMMU is optional or mandatory would be a good
>> enhancement.
>>
>> Signed-off-by: Laurent Pichart
>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> [rm: massive PCI hacks]
>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>> ---
>>   drivers/base/dma-mapping.c | 4 ++--
>>   drivers/iommu/dma-iommu.c  | 1 +
>>   drivers/iommu/of_iommu.c   | 5 +++--
>>   drivers/of/device.c        | 9 +++++++--
>>   drivers/pci/probe.c        | 6 ++++--
>>   include/linux/of_device.h  | 9 ++++++---
>>   include/linux/pci.h        | 4 ++--
>>   7 files changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index b2a5629..576fdfb 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t
>> size, unsigned long vm_flags)
>>   int dma_configure(struct device *dev)
>>   {
>>       if (dev_is_pci(dev))
>> -        pci_dma_configure(dev);
>> +        return pci_dma_configure(dev);
>>       else if (dev->of_node)
>> -        of_dma_configure(dev, dev->of_node);
>> +        return of_dma_configure(dev, dev->of_node);
>>       return 0;
>>   }
>>   diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index c5ab866..d2a7a46 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain
>> *domain, dma_addr_t base,
>>       base_pfn = max_t(unsigned long, 1, base >> order);
>>       end_pfn = (base + size - 1) >> order;
>>   +    dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n",
>> base, size, domain->geometry.aperture_start,
>> domain->geometry.aperture_end,
> 
> This causes a NULL pointer dereference if caller passes NULL device
> pointer.
> There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
> Trivial to fix as it looks like a leftover from developement or
> debugging stage.

Yes, this is some development crap which was never intended to go
upstream. Hence "massive PCI hacks" ;)

Other than the first two patches, the rest of the stuff from me here was
just an experiment which I'm not entirely convinced by the outcome of -
I don't particularly like the resulting fragmentation of having
pci_dma_configure() awkwardly floating around on its own in pci.c.

Robin.

>> *dev->dma_mask, dev->coherent_dma_mask);
>>       /* Check the domain allows at least some access to the device... */
>>       if (domain->geometry.force_aperture) {
>>           if (base > domain->geometry.aperture_end ||
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 349bd1d..9529d6c 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/of.h>
>>   #include <linux/of_iommu.h>
>>   #include <linux/of_pci.h>
>> +#include <linux/pci.h>
>>   #include <linux/slab.h>
>>     static const struct of_device_id __iommu_of_table_sentinel
>> @@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct
>> device *dev,
>>               ops = ERR_PTR(err);
>>       }
>>   -    return IS_ERR(ops) ? NULL : ops;
>> +    return ops;
>>   }
>>     static int __init of_iommu_init(void)
>> @@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
>>       for_each_matching_node_and_match(np, matches, &match) {
>>           const of_iommu_init_fn init_fn = match->data;
>>   -        if (init_fn(np))
>> +        if (init_fn && init_fn(np))
>>               pr_err("Failed to initialise IOMMU %s\n",
>>                   of_node_full_name(np));
>>       }
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 1c843e2..d58595c 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
>>    * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE
>> events
>>    * 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;
>> @@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct
>> device_node *np)
>>       ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>>       if (ret < 0) {
>>           dma_addr = offset = 0;
>> -        size = dev->coherent_dma_mask + 1;
>> +        size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
>>       } else {
>>           offset = PFN_DOWN(paddr - dma_addr);
>>           dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>> @@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct
>> device_node *np)
>>           coherent ? " " : " not ");
>>         iommu = of_iommu_configure(dev, np);
>> +    if (IS_ERR(iommu))
>> +        return PTR_ERR(iommu);
>> +
>>       dev_dbg(dev, "device is%sbehind an iommu\n",
>>           iommu ? " " : " not ");
>>         arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
>> +
>> +    return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(of_dma_configure);
>>   diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 04af770..6316cae 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev
>> *dev)
>>    * Function to update PCI devices's DMA configuration using the same
>>    * info from the OF node or ACPI node of host bridge's parent (if any).
>>    */
>> -void pci_dma_configure(struct device *dev)
>> +int pci_dma_configure(struct device *dev)
>>   {
>>       struct device *bridge =
>> pci_get_host_bridge_device(to_pci_dev(dev));
>> +    int ret = 0;
>>         if (IS_ENABLED(CONFIG_OF) &&
>>           bridge->parent && bridge->parent->of_node) {
>> -        of_dma_configure(dev, bridge->parent->of_node);
>> +        ret = of_dma_configure(dev, bridge->parent->of_node);
>>       } else if (has_acpi_companion(bridge)) {
>>           struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>>           enum dev_dma_attr attr = acpi_get_dma_attr(adev);
>> @@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
>>       }
>>         pci_put_host_bridge_device(bridge);
>> +    return ret;
>>   }
>>     void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
>> index d20a31a..6dca65c 100644
>> --- a/include/linux/of_device.h
>> +++ b/include/linux/of_device.h
>> @@ -55,7 +55,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);
>>   void of_dma_deconfigure(struct device *dev);
>>   #else /* CONFIG_OF */
>>   @@ -99,8 +99,11 @@ static inline struct device_node
>> *of_cpu_device_node_get(int cpu)
>>   {
>>       return NULL;
>>   }
>> -static inline void of_dma_configure(struct device *dev, struct
>> device_node *np)
>> -{}
>> +
>> +static inline int of_dma_configure(struct device *dev, struct
>> device_node *np)
>> +{
>> +    return 0;
>> +}
>>   static inline void of_dma_deconfigure(struct device *dev)
>>   {}
>>   #endif /* CONFIG_OF */
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index d04f651..989ca44 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const
>> struct pci_dev *dev,
>>   #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn :
>> false))
>>   #define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
>>   -void pci_dma_configure(struct device *dev);
>> +int pci_dma_configure(struct device *dev);
>>     /* Generic PCI functions exported to card drivers */
>>   @@ -1604,7 +1604,7 @@ static inline struct pci_dev
>> *pci_get_bus_and_slot(unsigned int bus,
>>   #define dev_is_pf(d) (false)
>>   #define dev_num_vf(d) (0)
>>   -static inline void pci_dma_configure(struct device *dev) { }
>> +static inline int pci_dma_configure(struct device *dev) { return 0; }
>>     #endif /* CONFIG_PCI */
>>   
> 
> Best regards

RE: [PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Sricharan]

Hi Robin,

>On 30/11/16 07:54, Marek Szyprowski wrote:
>> Hi Sricharan and Robin,
>>
>>
>> On 2016-11-30 01:22, Sricharan R wrote:
>>> From: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>>
>>> Failures to look up an IOMMU when parsing the DT iommus property need to
>>> be handled separately from the .of_xlate() failures to support deferred
>>> probing.
>>>
>>> The lack of a registered IOMMU can be caused by the lack of a driver for
>>> the IOMMU, the IOMMU device probe not having been performed yet, having
>>> been deferred, or having failed.
>>>
>>> The first case occurs when the device tree describes the bus master and
>>> IOMMU topology correctly but no device driver exists for the IOMMU yet
>>> or the device driver has not been compiled in. Return NULL, the caller
>>> will configure the device without an IOMMU.
>>>
>>> The second and third cases are handled by deferring the probe of the bus
>>> master device which will eventually get reprobed after the IOMMU.
>>>
>>> The last case is currently handled by deferring the probe of the bus
>>> master device as well. A mechanism to either configure the bus master
>>> device without an IOMMU or to fail the bus master device probe depending
>>> on whether the IOMMU is optional or mandatory would be a good
>>> enhancement.
>>>
>>> Signed-off-by: Laurent Pichart
>>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> [rm: massive PCI hacks]
>>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>>   drivers/base/dma-mapping.c | 4 ++--
>>>   drivers/iommu/dma-iommu.c  | 1 +
>>>   drivers/iommu/of_iommu.c   | 5 +++--
>>>   drivers/of/device.c        | 9 +++++++--
>>>   drivers/pci/probe.c        | 6 ++++--
>>>   include/linux/of_device.h  | 9 ++++++---
>>>   include/linux/pci.h        | 4 ++--
>>>   7 files changed, 25 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>>> index b2a5629..576fdfb 100644
>>> --- a/drivers/base/dma-mapping.c
>>> +++ b/drivers/base/dma-mapping.c
>>> @@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t
>>> size, unsigned long vm_flags)
>>>   int dma_configure(struct device *dev)
>>>   {
>>>       if (dev_is_pci(dev))
>>> -        pci_dma_configure(dev);
>>> +        return pci_dma_configure(dev);
>>>       else if (dev->of_node)
>>> -        of_dma_configure(dev, dev->of_node);
>>> +        return of_dma_configure(dev, dev->of_node);
>>>       return 0;
>>>   }
>>>   diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index c5ab866..d2a7a46 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain
>>> *domain, dma_addr_t base,
>>>       base_pfn = max_t(unsigned long, 1, base >> order);
>>>       end_pfn = (base + size - 1) >> order;
>>>   +    dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n",
>>> base, size, domain->geometry.aperture_start,
>>> domain->geometry.aperture_end,
>>
>> This causes a NULL pointer dereference if caller passes NULL device
>> pointer.
>> There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
>> Trivial to fix as it looks like a leftover from developement or
>> debugging stage.
>
>Yes, this is some development crap which was never intended to go
>upstream. Hence "massive PCI hacks" ;)
>
>Other than the first two patches, the rest of the stuff from me here was
>just an experiment which I'm not entirely convinced by the outcome of -
>I don't particularly like the resulting fragmentation of having
>pci_dma_configure() awkwardly floating around on its own in pci.c.

Ha, sorry looks like that i have misunderstood that then. I thought that
the V3 changes that i had for pci was hacky and the reworked patches
in your latest branch were correct. So do you suggest that the
pci_dma_configure gets removed and the pci related changes gets handled
in of_iommu_configure /acpi_dma_configure itself ?

Regards,
Sricharan

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

Sricharan, Robin,

I gave this series a go on ACPI and apart from an SMMU v3 fix-up
it seems to work, more thorough testing required though.

A key question below.

On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> 
> IOMMU configuration represents unchanging properties of the hardware,
> and as such should only need happen once in a device's lifetime, but
> the necessary interaction with the IOMMU device and driver complicates
> exactly when that point should be.
> 
> Since the only reasonable tool available for handling the inter-device
> dependency is probe deferral, we need to prepare of_iommu_configure()
> to run later than it is currently called (i.e. at driver probe rather
> than device creation), to handle being retried, and to tell whether a
> not-yet present IOMMU should be waited for or skipped (by virtue of
> having declared a built-in driver or not).
> 
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index ee49081..349bd1d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>  	int err;
>  
>  	ops = iommu_get_instance(fwnode);
> -	if (!ops || !ops->of_xlate)
> +	if ((ops && !ops->of_xlate) ||
> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))

IIUC of_match_node() here is there to check there is a driver compiled
in for this device_node (aka compatible string in OF world), correct ?
If that's the case (and I think that's what Sricharan was referring to
in his ACPI query) I need to cook-up something on the ACPI side to
emulate the OF linker table behaviour (or anyway to detect a driver is
actually in the kernel), it is not that difficult but it is key to know,
I will give it some thought to make it as clean as possible.

Thanks,
Lorenzo

>  		return NULL;
>  
>  	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
>  	if (err)
>  		return ERR_PTR(err);
> +	/*
> +	 * The otherwise-empty fwspec handily serves to indicate the specific
> +	 * IOMMU device we're waiting for, which will be useful if we ever get
> +	 * a proper probe-ordering dependency mechanism in future.
> +	 */
> +	if (!ops)
> +		return ERR_PTR(-EPROBE_DEFER);
>  
>  	err = ops->of_xlate(dev, iommu_spec);
>  	if (err)
> @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>  					   struct device_node *master_np)
>  {
>  	const struct iommu_ops *ops;
> +	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>  
>  	if (!master_np)
>  		return NULL;
>  
> +	if (fwspec) {
> +		if (fwspec->ops)
> +			return fwspec->ops;
> +
> +		/* In the deferred case, start again from scratch */
> +		iommu_fwspec_free(dev);
> +	}
> +
>  	if (dev_is_pci(dev))
>  		ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
>  	else
>  		ops = of_platform_iommu_init(dev, master_np);
> +	/*
> +	 * If we have reason to believe the IOMMU driver missed the initial
> +	 * add_device callback for dev, replay it to get things in order.
> +	 */
> +	if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
> +	    dev->bus && !dev->iommu_group) {
> +		int err = ops->add_device(dev);
> +
> +		if (err)
> +			ops = ERR_PTR(err);
> +	}
>  
>  	return IS_ERR(ops) ? NULL : ops;
>  }
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Robin Murphy]

On 30/11/16 16:17, Lorenzo Pieralisi wrote:
> Sricharan, Robin,
> 
> I gave this series a go on ACPI and apart from an SMMU v3 fix-up
> it seems to work, more thorough testing required though.
> 
> A key question below.
> 
> On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> IOMMU configuration represents unchanging properties of the hardware,
>> and as such should only need happen once in a device's lifetime, but
>> the necessary interaction with the IOMMU device and driver complicates
>> exactly when that point should be.
>>
>> Since the only reasonable tool available for handling the inter-device
>> dependency is probe deferral, we need to prepare of_iommu_configure()
>> to run later than it is currently called (i.e. at driver probe rather
>> than device creation), to handle being retried, and to tell whether a
>> not-yet present IOMMU should be waited for or skipped (by virtue of
>> having declared a built-in driver or not).
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index ee49081..349bd1d 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>>  	int err;
>>  
>>  	ops = iommu_get_instance(fwnode);
>> -	if (!ops || !ops->of_xlate)
>> +	if ((ops && !ops->of_xlate) ||
>> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
> 
> IIUC of_match_node() here is there to check there is a driver compiled
> in for this device_node (aka compatible string in OF world), correct ?

Yes - specifically, it's checking the magic table for a matching
IOMMU_OF_DECLARE entry.

> If that's the case (and I think that's what Sricharan was referring to
> in his ACPI query) I need to cook-up something on the ACPI side to
> emulate the OF linker table behaviour (or anyway to detect a driver is
> actually in the kernel), it is not that difficult but it is key to know,
> I will give it some thought to make it as clean as possible.

I didn't think this would be a concern for ACPI, since IORT works much
the same way the current of_iommu_init_fn/of_platform_device_create()
bodges in drivers so for DT. If you can only discover SMMUs from IORT,
then iort_init_platform_devices() will have already created every SMMU
that's going to exist before discovering other devices from wherever
they come from, thus you could never get into the situation of probing a
device without its SMMU being ready (if it's ever going to be). Is that
not right?

Robin.

> 
> Thanks,
> Lorenzo
> 
>>  		return NULL;
>>  
>>  	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
>>  	if (err)
>>  		return ERR_PTR(err);
>> +	/*
>> +	 * The otherwise-empty fwspec handily serves to indicate the specific
>> +	 * IOMMU device we're waiting for, which will be useful if we ever get
>> +	 * a proper probe-ordering dependency mechanism in future.
>> +	 */
>> +	if (!ops)
>> +		return ERR_PTR(-EPROBE_DEFER);
>>  
>>  	err = ops->of_xlate(dev, iommu_spec);
>>  	if (err)
>> @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>>  					   struct device_node *master_np)
>>  {
>>  	const struct iommu_ops *ops;
>> +	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>>  
>>  	if (!master_np)
>>  		return NULL;
>>  
>> +	if (fwspec) {
>> +		if (fwspec->ops)
>> +			return fwspec->ops;
>> +
>> +		/* In the deferred case, start again from scratch */
>> +		iommu_fwspec_free(dev);
>> +	}
>> +
>>  	if (dev_is_pci(dev))
>>  		ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
>>  	else
>>  		ops = of_platform_iommu_init(dev, master_np);
>> +	/*
>> +	 * If we have reason to believe the IOMMU driver missed the initial
>> +	 * add_device callback for dev, replay it to get things in order.
>> +	 */
>> +	if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
>> +	    dev->bus && !dev->iommu_group) {
>> +		int err = ops->add_device(dev);
>> +
>> +		if (err)
>> +			ops = ERR_PTR(err);
>> +	}
>>  
>>  	return IS_ERR(ops) ? NULL : ops;
>>  }
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote:
> On 30/11/16 16:17, Lorenzo Pieralisi wrote:
> > Sricharan, Robin,
> > 
> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up
> > it seems to work, more thorough testing required though.
> > 
> > A key question below.
> > 
> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
> >> From: Robin Murphy <robin.murphy@arm.com>
> >>
> >> IOMMU configuration represents unchanging properties of the hardware,
> >> and as such should only need happen once in a device's lifetime, but
> >> the necessary interaction with the IOMMU device and driver complicates
> >> exactly when that point should be.
> >>
> >> Since the only reasonable tool available for handling the inter-device
> >> dependency is probe deferral, we need to prepare of_iommu_configure()
> >> to run later than it is currently called (i.e. at driver probe rather
> >> than device creation), to handle being retried, and to tell whether a
> >> not-yet present IOMMU should be waited for or skipped (by virtue of
> >> having declared a built-in driver or not).
> >>
> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >> ---
> >>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
> >>  1 file changed, 29 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >> index ee49081..349bd1d 100644
> >> --- a/drivers/iommu/of_iommu.c
> >> +++ b/drivers/iommu/of_iommu.c
> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
> >>  	int err;
> >>  
> >>  	ops = iommu_get_instance(fwnode);
> >> -	if (!ops || !ops->of_xlate)
> >> +	if ((ops && !ops->of_xlate) ||
> >> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
> > 
> > IIUC of_match_node() here is there to check there is a driver compiled
> > in for this device_node (aka compatible string in OF world), correct ?
> 
> Yes - specifically, it's checking the magic table for a matching
> IOMMU_OF_DECLARE entry.
> 
> > If that's the case (and I think that's what Sricharan was referring to
> > in his ACPI query) I need to cook-up something on the ACPI side to
> > emulate the OF linker table behaviour (or anyway to detect a driver is
> > actually in the kernel), it is not that difficult but it is key to know,
> > I will give it some thought to make it as clean as possible.
> 
> I didn't think this would be a concern for ACPI, since IORT works much
> the same way the current of_iommu_init_fn/of_platform_device_create()
> bodges in drivers so for DT. If you can only discover SMMUs from IORT,
> then iort_init_platform_devices() will have already created every SMMU
> that's going to exist before discovering other devices from wherever
> they come from, thus you could never get into the situation of probing a
> device without its SMMU being ready (if it's ever going to be). Is that
> not right?

It is right, my point and question is: we are probing a device and we
have to know whether it is worth deferring its IOMMU DMA setup. On DT,
through of_match_node(&__iommu_of_table, iommu_device_node) we check at
once that:

1 - A device for the IOMMU exists

AND

2 - A driver for the IOMMU is compiled in the kernel

Is this correct ? As you said (1) is not a concern on ACPI IORT (because
we create the IOMMU device before _any_ other device so either the IOMMU
device is there or it will never be by the time master devices are
probed), but for (2) I need to slightly change how the IORT linker entry
work to make sure we can detect a driver is actually compiled in the
kernel, it is easy, I was just asking if my understanding was correct
and I think that was what Sricharan was referring to in his query.

I will put together a patch, again, it is a simple tweak.

Thanks !
Lorenzo

> Robin.
> 
> > 
> > Thanks,
> > Lorenzo
> > 
> >>  		return NULL;
> >>  
> >>  	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
> >>  	if (err)
> >>  		return ERR_PTR(err);
> >> +	/*
> >> +	 * The otherwise-empty fwspec handily serves to indicate the specific
> >> +	 * IOMMU device we're waiting for, which will be useful if we ever get
> >> +	 * a proper probe-ordering dependency mechanism in future.
> >> +	 */
> >> +	if (!ops)
> >> +		return ERR_PTR(-EPROBE_DEFER);
> >>  
> >>  	err = ops->of_xlate(dev, iommu_spec);
> >>  	if (err)
> >> @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> >>  					   struct device_node *master_np)
> >>  {
> >>  	const struct iommu_ops *ops;
> >> +	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> >>  
> >>  	if (!master_np)
> >>  		return NULL;
> >>  
> >> +	if (fwspec) {
> >> +		if (fwspec->ops)
> >> +			return fwspec->ops;
> >> +
> >> +		/* In the deferred case, start again from scratch */
> >> +		iommu_fwspec_free(dev);
> >> +	}
> >> +
> >>  	if (dev_is_pci(dev))
> >>  		ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
> >>  	else
> >>  		ops = of_platform_iommu_init(dev, master_np);
> >> +	/*
> >> +	 * If we have reason to believe the IOMMU driver missed the initial
> >> +	 * add_device callback for dev, replay it to get things in order.
> >> +	 */
> >> +	if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
> >> +	    dev->bus && !dev->iommu_group) {
> >> +		int err = ops->add_device(dev);
> >> +
> >> +		if (err)
> >> +			ops = ERR_PTR(err);
> >> +	}
> >>  
> >>  	return IS_ERR(ops) ? NULL : ops;
> >>  }
> >> -- 
> >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> >>
> 

RE: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Sricharan]

Hi Robin,Lorenzo,

>On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote:
>> On 30/11/16 16:17, Lorenzo Pieralisi wrote:
>> > Sricharan, Robin,
>> >
>> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up
>> > it seems to work, more thorough testing required though.
>> >
>> > A key question below.
>> >
>> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
>> >> From: Robin Murphy <robin.murphy@arm.com>
>> >>
>> >> IOMMU configuration represents unchanging properties of the hardware,
>> >> and as such should only need happen once in a device's lifetime, but
>> >> the necessary interaction with the IOMMU device and driver complicates
>> >> exactly when that point should be.
>> >>
>> >> Since the only reasonable tool available for handling the inter-device
>> >> dependency is probe deferral, we need to prepare of_iommu_configure()
>> >> to run later than it is currently called (i.e. at driver probe rather
>> >> than device creation), to handle being retried, and to tell whether a
>> >> not-yet present IOMMU should be waited for or skipped (by virtue of
>> >> having declared a built-in driver or not).
>> >>
>> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> >> ---
>> >>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
>> >>  1 file changed, 29 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> >> index ee49081..349bd1d 100644
>> >> --- a/drivers/iommu/of_iommu.c
>> >> +++ b/drivers/iommu/of_iommu.c
>> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>> >>  	int err;
>> >>
>> >>  	ops = iommu_get_instance(fwnode);
>> >> -	if (!ops || !ops->of_xlate)
>> >> +	if ((ops && !ops->of_xlate) ||
>> >> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
>> >
>> > IIUC of_match_node() here is there to check there is a driver compiled
>> > in for this device_node (aka compatible string in OF world), correct ?
>>
>> Yes - specifically, it's checking the magic table for a matching
>> IOMMU_OF_DECLARE entry.
>>
>> > If that's the case (and I think that's what Sricharan was referring to
>> > in his ACPI query) I need to cook-up something on the ACPI side to
>> > emulate the OF linker table behaviour (or anyway to detect a driver is
>> > actually in the kernel), it is not that difficult but it is key to know,
>> > I will give it some thought to make it as clean as possible.
>>
>> I didn't think this would be a concern for ACPI, since IORT works much
>> the same way the current of_iommu_init_fn/of_platform_device_create()
>> bodges in drivers so for DT. If you can only discover SMMUs from IORT,
>> then iort_init_platform_devices() will have already created every SMMU
>> that's going to exist before discovering other devices from wherever
>> they come from, thus you could never get into the situation of probing a
>> device without its SMMU being ready (if it's ever going to be). Is that
>> not right?
>
>It is right, my point and question is: we are probing a device and we
>have to know whether it is worth deferring its IOMMU DMA setup. On DT,
>through of_match_node(&__iommu_of_table, iommu_device_node) we check at
>once that:
>
>1 - A device for the IOMMU exists
>
>AND
>
>2 - A driver for the IOMMU is compiled in the kernel
>
>Is this correct ? As you said (1) is not a concern on ACPI IORT (because
>we create the IOMMU device before _any_ other device so either the IOMMU
>device is there or it will never be by the time master devices are
>probed), but for (2) I need to slightly change how the IORT linker entry
>work to make sure we can detect a driver is actually compiled in the
>kernel, it is easy, I was just asking if my understanding was correct
>and I think that was what Sricharan was referring to in his query.
>

Yes right, this was what i was looking for in the ACPI case and putting this
in the iort_iommu_xlate was needed to return EPROBE_DEFER when the
driver is not yet been probed.

Regards,
 Sricharan

RE: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Sricharan]

Hi Robin/Lorenzo,

>Hi Robin,Lorenzo,
>
>>On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote:
>>> On 30/11/16 16:17, Lorenzo Pieralisi wrote:
>>> > Sricharan, Robin,
>>> >
>>> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up
>>> > it seems to work, more thorough testing required though.
>>> >
>>> > A key question below.
>>> >
>>> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
>>> >> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>> >>
>>> >> IOMMU configuration represents unchanging properties of the hardware,
>>> >> and as such should only need happen once in a device's lifetime, but
>>> >> the necessary interaction with the IOMMU device and driver complicates
>>> >> exactly when that point should be.
>>> >>
>>> >> Since the only reasonable tool available for handling the inter-device
>>> >> dependency is probe deferral, we need to prepare of_iommu_configure()
>>> >> to run later than it is currently called (i.e. at driver probe rather
>>> >> than device creation), to handle being retried, and to tell whether a
>>> >> not-yet present IOMMU should be waited for or skipped (by virtue of
>>> >> having declared a built-in driver or not).
>>> >>
>>> >> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>> >> ---
>>> >>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
>>> >>  1 file changed, 29 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>> >> index ee49081..349bd1d 100644
>>> >> --- a/drivers/iommu/of_iommu.c
>>> >> +++ b/drivers/iommu/of_iommu.c
>>> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>>> >>  	int err;
>>> >>
>>> >>  	ops = iommu_get_instance(fwnode);
>>> >> -	if (!ops || !ops->of_xlate)
>>> >> +	if ((ops && !ops->of_xlate) ||
>>> >> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
>>> >
>>> > IIUC of_match_node() here is there to check there is a driver compiled
>>> > in for this device_node (aka compatible string in OF world), correct ?
>>>
>>> Yes - specifically, it's checking the magic table for a matching
>>> IOMMU_OF_DECLARE entry.
>>>
>>> > If that's the case (and I think that's what Sricharan was referring to
>>> > in his ACPI query) I need to cook-up something on the ACPI side to
>>> > emulate the OF linker table behaviour (or anyway to detect a driver is
>>> > actually in the kernel), it is not that difficult but it is key to know,
>>> > I will give it some thought to make it as clean as possible.
>>>
>>> I didn't think this would be a concern for ACPI, since IORT works much
>>> the same way the current of_iommu_init_fn/of_platform_device_create()
>>> bodges in drivers so for DT. If you can only discover SMMUs from IORT,
>>> then iort_init_platform_devices() will have already created every SMMU
>>> that's going to exist before discovering other devices from wherever
>>> they come from, thus you could never get into the situation of probing a
>>> device without its SMMU being ready (if it's ever going to be). Is that
>>> not right?
>>
>>It is right, my point and question is: we are probing a device and we
>>have to know whether it is worth deferring its IOMMU DMA setup. On DT,
>>through of_match_node(&__iommu_of_table, iommu_device_node) we check at
>>once that:
>>
>>1 - A device for the IOMMU exists
>>
>>AND
>>
>>2 - A driver for the IOMMU is compiled in the kernel
>>
>>Is this correct ? As you said (1) is not a concern on ACPI IORT (because
>>we create the IOMMU device before _any_ other device so either the IOMMU
>>device is there or it will never be by the time master devices are
>>probed), but for (2) I need to slightly change how the IORT linker entry
>>work to make sure we can detect a driver is actually compiled in the
>>kernel, it is easy, I was just asking if my understanding was correct
>>and I think that was what Sricharan was referring to in his query.
>>
>
>Yes right, this was what i was looking for in the ACPI case and putting this
>in the iort_iommu_xlate was needed to return EPROBE_DEFER when the
>driver is not yet been probed.

With the thinking of taking this series through, would it be fine if i cleanup
the pci configure hanging outside and push it in to of/acpi_iommu configure
respectively ? This time with all neeeded for ACPI added as well.
 Also on the last post of V4, Lorenzo commented that it worked for him, although
still the of_match_node equivalent in ACPI has to be added. If i can get that,
then i will add that as well to make this complete.

Regards,
 Sricharan

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

On Thu, Jan 05, 2017 at 02:04:37PM +0530, Sricharan wrote:
> Hi Robin/Lorenzo,
> 
> >Hi Robin,Lorenzo,
> >
> >>On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote:
> >>> On 30/11/16 16:17, Lorenzo Pieralisi wrote:
> >>> > Sricharan, Robin,
> >>> >
> >>> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up
> >>> > it seems to work, more thorough testing required though.
> >>> >
> >>> > A key question below.
> >>> >
> >>> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
> >>> >> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> >>> >>
> >>> >> IOMMU configuration represents unchanging properties of the hardware,
> >>> >> and as such should only need happen once in a device's lifetime, but
> >>> >> the necessary interaction with the IOMMU device and driver complicates
> >>> >> exactly when that point should be.
> >>> >>
> >>> >> Since the only reasonable tool available for handling the inter-device
> >>> >> dependency is probe deferral, we need to prepare of_iommu_configure()
> >>> >> to run later than it is currently called (i.e. at driver probe rather
> >>> >> than device creation), to handle being retried, and to tell whether a
> >>> >> not-yet present IOMMU should be waited for or skipped (by virtue of
> >>> >> having declared a built-in driver or not).
> >>> >>
> >>> >> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> >>> >> ---
> >>> >>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
> >>> >>  1 file changed, 29 insertions(+), 1 deletion(-)
> >>> >>
> >>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >>> >> index ee49081..349bd1d 100644
> >>> >> --- a/drivers/iommu/of_iommu.c
> >>> >> +++ b/drivers/iommu/of_iommu.c
> >>> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
> >>> >>  	int err;
> >>> >>
> >>> >>  	ops = iommu_get_instance(fwnode);
> >>> >> -	if (!ops || !ops->of_xlate)
> >>> >> +	if ((ops && !ops->of_xlate) ||
> >>> >> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
> >>> >
> >>> > IIUC of_match_node() here is there to check there is a driver compiled
> >>> > in for this device_node (aka compatible string in OF world), correct ?
> >>>
> >>> Yes - specifically, it's checking the magic table for a matching
> >>> IOMMU_OF_DECLARE entry.
> >>>
> >>> > If that's the case (and I think that's what Sricharan was referring to
> >>> > in his ACPI query) I need to cook-up something on the ACPI side to
> >>> > emulate the OF linker table behaviour (or anyway to detect a driver is
> >>> > actually in the kernel), it is not that difficult but it is key to know,
> >>> > I will give it some thought to make it as clean as possible.
> >>>
> >>> I didn't think this would be a concern for ACPI, since IORT works much
> >>> the same way the current of_iommu_init_fn/of_platform_device_create()
> >>> bodges in drivers so for DT. If you can only discover SMMUs from IORT,
> >>> then iort_init_platform_devices() will have already created every SMMU
> >>> that's going to exist before discovering other devices from wherever
> >>> they come from, thus you could never get into the situation of probing a
> >>> device without its SMMU being ready (if it's ever going to be). Is that
> >>> not right?
> >>
> >>It is right, my point and question is: we are probing a device and we
> >>have to know whether it is worth deferring its IOMMU DMA setup. On DT,
> >>through of_match_node(&__iommu_of_table, iommu_device_node) we check at
> >>once that:
> >>
> >>1 - A device for the IOMMU exists
> >>
> >>AND
> >>
> >>2 - A driver for the IOMMU is compiled in the kernel
> >>
> >>Is this correct ? As you said (1) is not a concern on ACPI IORT (because
> >>we create the IOMMU device before _any_ other device so either the IOMMU
> >>device is there or it will never be by the time master devices are
> >>probed), but for (2) I need to slightly change how the IORT linker entry
> >>work to make sure we can detect a driver is actually compiled in the
> >>kernel, it is easy, I was just asking if my understanding was correct
> >>and I think that was what Sricharan was referring to in his query.
> >>
> >
> >Yes right, this was what i was looking for in the ACPI case and putting this
> >in the iort_iommu_xlate was needed to return EPROBE_DEFER when the
> >driver is not yet been probed.
> 
> With the thinking of taking this series through, would it be fine if i
> cleanup the pci configure hanging outside and push it in to
> of/acpi_iommu configure respectively ? This time with all neeeded for
> ACPI added as well.  Also on the last post of V4, Lorenzo commented
> that it worked for him, although still the of_match_node equivalent in
> ACPI has to be added. If i can get that, then i will add that as well
> to make this complete.

Question: I had a look into this and instead of fiddling about with the
linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
patchset would help remove entirely), I think that the only check we
need in IORT is, depending on what type of SMMU a given device is
connected to, to check if the respective SMMU driver is compiled in the
kernel and it will be probed, _eventually_.

As Robin said, by the time a device is probed the respective SMMU
devices are already created and registered with IORT kernel code or
they will never be, so to understand if we should defer probing
SMMU device creation is _not_ really a problem in ACPI.

To check if a SMMU driver is enabled, do we really need a linker
table ?

Would not a check based on eg:

/**
 * @type: IOMMU IORT node type of the IOMMU a device is connected to
 */
static bool iort_iommu_driver_enabled(u8 type)
{
	switch (type) {
	case ACPI_IORT_SMMU_V3:
		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
	case ACPI_IORT_SMMU:
		return IS_ENABLED(CONFIG_ARM_SMMU);
	default:
		pr_warn("Unknown IORT SMMU type\n");
		return false;
	}
}

be sufficient (it is a bit gross, agreed, but it is to understand if
that's all we need) ? Is there anything I am missing ?

Let me know, I will put together a patch for you I really do not
want to block your series for this trivial niggle.

Thanks,
Lorenzo

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Robin Murphy]

On 05/01/17 12:27, Lorenzo Pieralisi wrote:
> On Thu, Jan 05, 2017 at 02:04:37PM +0530, Sricharan wrote:
>> Hi Robin/Lorenzo,
>>
>>> Hi Robin,Lorenzo,
>>>
>>>> On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote:
>>>>> On 30/11/16 16:17, Lorenzo Pieralisi wrote:
>>>>>> Sricharan, Robin,
>>>>>>
>>>>>> I gave this series a go on ACPI and apart from an SMMU v3 fix-up
>>>>>> it seems to work, more thorough testing required though.
>>>>>>
>>>>>> A key question below.
>>>>>>
>>>>>> On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote:
>>>>>>> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>>>>>>
>>>>>>> IOMMU configuration represents unchanging properties of the hardware,
>>>>>>> and as such should only need happen once in a device's lifetime, but
>>>>>>> the necessary interaction with the IOMMU device and driver complicates
>>>>>>> exactly when that point should be.
>>>>>>>
>>>>>>> Since the only reasonable tool available for handling the inter-device
>>>>>>> dependency is probe deferral, we need to prepare of_iommu_configure()
>>>>>>> to run later than it is currently called (i.e. at driver probe rather
>>>>>>> than device creation), to handle being retried, and to tell whether a
>>>>>>> not-yet present IOMMU should be waited for or skipped (by virtue of
>>>>>>> having declared a built-in driver or not).
>>>>>>>
>>>>>>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>>>>>> ---
>>>>>>>  drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
>>>>>>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>>>>>> index ee49081..349bd1d 100644
>>>>>>> --- a/drivers/iommu/of_iommu.c
>>>>>>> +++ b/drivers/iommu/of_iommu.c
>>>>>>> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>>>>>>>  	int err;
>>>>>>>
>>>>>>>  	ops = iommu_get_instance(fwnode);
>>>>>>> -	if (!ops || !ops->of_xlate)
>>>>>>> +	if ((ops && !ops->of_xlate) ||
>>>>>>> +	    (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
>>>>>>
>>>>>> IIUC of_match_node() here is there to check there is a driver compiled
>>>>>> in for this device_node (aka compatible string in OF world), correct ?
>>>>>
>>>>> Yes - specifically, it's checking the magic table for a matching
>>>>> IOMMU_OF_DECLARE entry.
>>>>>
>>>>>> If that's the case (and I think that's what Sricharan was referring to
>>>>>> in his ACPI query) I need to cook-up something on the ACPI side to
>>>>>> emulate the OF linker table behaviour (or anyway to detect a driver is
>>>>>> actually in the kernel), it is not that difficult but it is key to know,
>>>>>> I will give it some thought to make it as clean as possible.
>>>>>
>>>>> I didn't think this would be a concern for ACPI, since IORT works much
>>>>> the same way the current of_iommu_init_fn/of_platform_device_create()
>>>>> bodges in drivers so for DT. If you can only discover SMMUs from IORT,
>>>>> then iort_init_platform_devices() will have already created every SMMU
>>>>> that's going to exist before discovering other devices from wherever
>>>>> they come from, thus you could never get into the situation of probing a
>>>>> device without its SMMU being ready (if it's ever going to be). Is that
>>>>> not right?
>>>>
>>>> It is right, my point and question is: we are probing a device and we
>>>> have to know whether it is worth deferring its IOMMU DMA setup. On DT,
>>>> through of_match_node(&__iommu_of_table, iommu_device_node) we check at
>>>> once that:
>>>>
>>>> 1 - A device for the IOMMU exists
>>>>
>>>> AND
>>>>
>>>> 2 - A driver for the IOMMU is compiled in the kernel
>>>>
>>>> Is this correct ? As you said (1) is not a concern on ACPI IORT (because
>>>> we create the IOMMU device before _any_ other device so either the IOMMU
>>>> device is there or it will never be by the time master devices are
>>>> probed), but for (2) I need to slightly change how the IORT linker entry
>>>> work to make sure we can detect a driver is actually compiled in the
>>>> kernel, it is easy, I was just asking if my understanding was correct
>>>> and I think that was what Sricharan was referring to in his query.
>>>>
>>>
>>> Yes right, this was what i was looking for in the ACPI case and putting this
>>> in the iort_iommu_xlate was needed to return EPROBE_DEFER when the
>>> driver is not yet been probed.
>>
>> With the thinking of taking this series through, would it be fine if i
>> cleanup the pci configure hanging outside and push it in to
>> of/acpi_iommu configure respectively ? This time with all neeeded for
>> ACPI added as well.  Also on the last post of V4, Lorenzo commented
>> that it worked for him, although still the of_match_node equivalent in
>> ACPI has to be added. If i can get that, then i will add that as well
>> to make this complete.
> 
> Question: I had a look into this and instead of fiddling about with the
> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
> patchset would help remove entirely), I think that the only check we
> need in IORT is, depending on what type of SMMU a given device is
> connected to, to check if the respective SMMU driver is compiled in the
> kernel and it will be probed, _eventually_.
> 
> As Robin said, by the time a device is probed the respective SMMU
> devices are already created and registered with IORT kernel code or
> they will never be, so to understand if we should defer probing
> SMMU device creation is _not_ really a problem in ACPI.
> 
> To check if a SMMU driver is enabled, do we really need a linker
> table ?
> 
> Would not a check based on eg:
> 
> /**
>  * @type: IOMMU IORT node type of the IOMMU a device is connected to
>  */
> static bool iort_iommu_driver_enabled(u8 type)
> {
> 	switch (type) {
> 	case ACPI_IORT_SMMU_V3:
> 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);

IS_BUILTIN(...)

> 	case ACPI_IORT_SMMU:
> 		return IS_ENABLED(CONFIG_ARM_SMMU);
> 	default:
> 		pr_warn("Unknown IORT SMMU type\n");

Might displaying the actual value be helfpul for debugging a broken IORT
table?

> 		return false;
> 	}
> }
> 
> be sufficient (it is a bit gross, agreed, but it is to understand if
> that's all we need) ? Is there anything I am missing ?
> 
> Let me know, I will put together a patch for you I really do not
> want to block your series for this trivial niggle.

Other than that, though, I like it :) IORT has a small, strictly
bounded, set of supported devices, so I really don't see the need to go
overboard putting it on parity with DT when something this neat and
simple will suffice.

Robin.

> 
> Thanks,
> Lorenzo
> 

RE: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Sricharan]

Hi,

[...]

>>>
>>> With the thinking of taking this series through, would it be fine if i
>>> cleanup the pci configure hanging outside and push it in to
>>> of/acpi_iommu configure respectively ? This time with all neeeded for
>>> ACPI added as well.  Also on the last post of V4, Lorenzo commented
>>> that it worked for him, although still the of_match_node equivalent in
>>> ACPI has to be added. If i can get that, then i will add that as well
>>> to make this complete.
>>
>> Question: I had a look into this and instead of fiddling about with the
>> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
>> patchset would help remove entirely), I think that the only check we
>> need in IORT is, depending on what type of SMMU a given device is
>> connected to, to check if the respective SMMU driver is compiled in the
>> kernel and it will be probed, _eventually_.
>>
>> As Robin said, by the time a device is probed the respective SMMU
>> devices are already created and registered with IORT kernel code or
>> they will never be, so to understand if we should defer probing
>> SMMU device creation is _not_ really a problem in ACPI.
>>
>> To check if a SMMU driver is enabled, do we really need a linker
>> table ?
>>
>> Would not a check based on eg:
>>
>> /**
>>  * @type: IOMMU IORT node type of the IOMMU a device is connected to
>>  */
>> static bool iort_iommu_driver_enabled(u8 type)
>> {
>> 	switch (type) {
>> 	case ACPI_IORT_SMMU_V3:
>> 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
>
>IS_BUILTIN(...)
>
>> 	case ACPI_IORT_SMMU:
>> 		return IS_ENABLED(CONFIG_ARM_SMMU);
>> 	default:
>> 		pr_warn("Unknown IORT SMMU type\n");
>
>Might displaying the actual value be helfpul for debugging a broken IORT
>table?
>
>> 		return false;
>> 	}
>> }
>>
>> be sufficient (it is a bit gross, agreed, but it is to understand if
>> that's all we need) ? Is there anything I am missing ?
>>
>> Let me know, I will put together a patch for you I really do not
>> want to block your series for this trivial niggle.
>
>Other than that, though, I like it :) IORT has a small, strictly
>bounded, set of supported devices, so I really don't see the need to go
>overboard putting it on parity with DT when something this neat and
>simple will suffice.
>

Ok sure, looks correct for me as well in whole of the context here.

Regards,
 Sricharan

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

On Thu, Jan 05, 2017 at 01:52:29PM +0000, Robin Murphy wrote:

[...]

> > Question: I had a look into this and instead of fiddling about with the
> > linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
> > patchset would help remove entirely), I think that the only check we
> > need in IORT is, depending on what type of SMMU a given device is
> > connected to, to check if the respective SMMU driver is compiled in the
> > kernel and it will be probed, _eventually_.
> > 
> > As Robin said, by the time a device is probed the respective SMMU
> > devices are already created and registered with IORT kernel code or
> > they will never be, so to understand if we should defer probing
> > SMMU device creation is _not_ really a problem in ACPI.
> > 
> > To check if a SMMU driver is enabled, do we really need a linker
> > table ?
> > 
> > Would not a check based on eg:
> > 
> > /**
> >  * @type: IOMMU IORT node type of the IOMMU a device is connected to
> >  */
> > static bool iort_iommu_driver_enabled(u8 type)
> > {
> > 	switch (type) {
> > 	case ACPI_IORT_SMMU_V3:
> > 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
> 
> IS_BUILTIN(...)

Yep right, it is currently equivalent but that does not mean it
will always be.

> > 	case ACPI_IORT_SMMU:
> > 		return IS_ENABLED(CONFIG_ARM_SMMU);
> > 	default:
> > 		pr_warn("Unknown IORT SMMU type\n");
> 
> Might displaying the actual value be helfpul for debugging a broken IORT
> table?

Yes I will do.

> > 		return false;
> > 	}
> > }
> > 
> > be sufficient (it is a bit gross, agreed, but it is to understand if
> > that's all we need) ? Is there anything I am missing ?
> > 
> > Let me know, I will put together a patch for you I really do not
> > want to block your series for this trivial niggle.
> 
> Other than that, though, I like it :) IORT has a small, strictly
> bounded, set of supported devices, so I really don't see the need to go
> overboard putting it on parity with DT when something this neat and
> simple will suffice.

Ok, patch coming, which will also allow Sricharan to get rid of the
IORT linker script infrastructure altogether (and more than that,
I can write the patches on top of Sricharan series that I managed
to rebase against v4.10-rc2).

Thanks,
Lorenzo

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote:
> Hi,
> 
> [...]
> 
> >>>
> >>> With the thinking of taking this series through, would it be fine if i
> >>> cleanup the pci configure hanging outside and push it in to
> >>> of/acpi_iommu configure respectively ? This time with all neeeded for
> >>> ACPI added as well.  Also on the last post of V4, Lorenzo commented
> >>> that it worked for him, although still the of_match_node equivalent in
> >>> ACPI has to be added. If i can get that, then i will add that as well
> >>> to make this complete.
> >>
> >> Question: I had a look into this and instead of fiddling about with the
> >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
> >> patchset would help remove entirely), I think that the only check we
> >> need in IORT is, depending on what type of SMMU a given device is
> >> connected to, to check if the respective SMMU driver is compiled in the
> >> kernel and it will be probed, _eventually_.
> >>
> >> As Robin said, by the time a device is probed the respective SMMU
> >> devices are already created and registered with IORT kernel code or
> >> they will never be, so to understand if we should defer probing
> >> SMMU device creation is _not_ really a problem in ACPI.
> >>
> >> To check if a SMMU driver is enabled, do we really need a linker
> >> table ?
> >>
> >> Would not a check based on eg:
> >>
> >> /**
> >>  * @type: IOMMU IORT node type of the IOMMU a device is connected to
> >>  */
> >> static bool iort_iommu_driver_enabled(u8 type)
> >> {
> >> 	switch (type) {
> >> 	case ACPI_IORT_SMMU_V3:
> >> 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
> >
> >IS_BUILTIN(...)
> >
> >> 	case ACPI_IORT_SMMU:
> >> 		return IS_ENABLED(CONFIG_ARM_SMMU);
> >> 	default:
> >> 		pr_warn("Unknown IORT SMMU type\n");
> >
> >Might displaying the actual value be helfpul for debugging a broken IORT
> >table?
> >
> >> 		return false;
> >> 	}
> >> }
> >>
> >> be sufficient (it is a bit gross, agreed, but it is to understand if
> >> that's all we need) ? Is there anything I am missing ?
> >>
> >> Let me know, I will put together a patch for you I really do not
> >> want to block your series for this trivial niggle.
> >
> >Other than that, though, I like it :) IORT has a small, strictly
> >bounded, set of supported devices, so I really don't see the need to go
> >overboard putting it on parity with DT when something this neat and
> >simple will suffice.
> >
> 
> Ok sure, looks correct for me as well in whole of the context here.

Ok, I put together a branch where you can find your original series
plus some ACPI patches for you to test and use:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral

Feel free to post the additional patches I added along with your series
(that from what I gather you have reworked already) and please both have a
look if the deferral mechanism I put in place in ACPI makes sense to you.

Please CC linux-acpi upon posting, if you need help shout.

Lorenzo

Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Lorenzo Pieralisi]

Hi Sricharan,

On Fri, Jan 06, 2017 at 04:24:00PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote:
> > Hi,
> > 
> > [...]
> > 
> > >>>
> > >>> With the thinking of taking this series through, would it be fine if i
> > >>> cleanup the pci configure hanging outside and push it in to
> > >>> of/acpi_iommu configure respectively ? This time with all neeeded for
> > >>> ACPI added as well.  Also on the last post of V4, Lorenzo commented
> > >>> that it worked for him, although still the of_match_node equivalent in
> > >>> ACPI has to be added. If i can get that, then i will add that as well
> > >>> to make this complete.
> > >>
> > >> Question: I had a look into this and instead of fiddling about with the
> > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
> > >> patchset would help remove entirely), I think that the only check we
> > >> need in IORT is, depending on what type of SMMU a given device is
> > >> connected to, to check if the respective SMMU driver is compiled in the
> > >> kernel and it will be probed, _eventually_.
> > >>
> > >> As Robin said, by the time a device is probed the respective SMMU
> > >> devices are already created and registered with IORT kernel code or
> > >> they will never be, so to understand if we should defer probing
> > >> SMMU device creation is _not_ really a problem in ACPI.
> > >>
> > >> To check if a SMMU driver is enabled, do we really need a linker
> > >> table ?
> > >>
> > >> Would not a check based on eg:
> > >>
> > >> /**
> > >>  * @type: IOMMU IORT node type of the IOMMU a device is connected to
> > >>  */
> > >> static bool iort_iommu_driver_enabled(u8 type)
> > >> {
> > >> 	switch (type) {
> > >> 	case ACPI_IORT_SMMU_V3:
> > >> 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
> > >
> > >IS_BUILTIN(...)
> > >
> > >> 	case ACPI_IORT_SMMU:
> > >> 		return IS_ENABLED(CONFIG_ARM_SMMU);
> > >> 	default:
> > >> 		pr_warn("Unknown IORT SMMU type\n");
> > >
> > >Might displaying the actual value be helfpul for debugging a broken IORT
> > >table?
> > >
> > >> 		return false;
> > >> 	}
> > >> }
> > >>
> > >> be sufficient (it is a bit gross, agreed, but it is to understand if
> > >> that's all we need) ? Is there anything I am missing ?
> > >>
> > >> Let me know, I will put together a patch for you I really do not
> > >> want to block your series for this trivial niggle.
> > >
> > >Other than that, though, I like it :) IORT has a small, strictly
> > >bounded, set of supported devices, so I really don't see the need to go
> > >overboard putting it on parity with DT when something this neat and
> > >simple will suffice.
> > >
> > 
> > Ok sure, looks correct for me as well in whole of the context here.
> 
> Ok, I put together a branch where you can find your original series
> plus some ACPI patches for you to test and use:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral
> 
> Feel free to post the additional patches I added along with your series
> (that from what I gather you have reworked already) and please both have a
> look if the deferral mechanism I put in place in ACPI makes sense to you.

Did you have time to make progress on this ? I think it is time we
posted the complete series to aim for 4.11 please, if you need help just
let us know.

Thanks !
Lorenzo

RE: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration

[Sricharan]

Hi Lorenzo,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org] On Behalf Of Lorenzo Pieralisi
>Sent: Thursday, January 19, 2017 8:11 PM
>To: Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; 'Robin Murphy'
><robin.murphy-5wv7dgnIgG8@public.gmane.org>; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>Subject: Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration
>
>Hi Sricharan,
>
>On Fri, Jan 06, 2017 at 04:24:00PM +0000, Lorenzo Pieralisi wrote:
>> On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote:
>> > Hi,
>> >
>> > [...]
>> >
>> > >>>
>> > >>> With the thinking of taking this series through, would it be fine if i
>> > >>> cleanup the pci configure hanging outside and push it in to
>> > >>> of/acpi_iommu configure respectively ? This time with all neeeded for
>> > >>> ACPI added as well.  Also on the last post of V4, Lorenzo commented
>> > >>> that it worked for him, although still the of_match_node equivalent in
>> > >>> ACPI has to be added. If i can get that, then i will add that as well
>> > >>> to make this complete.
>> > >>
>> > >> Question: I had a look into this and instead of fiddling about with the
>> > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
>> > >> patchset would help remove entirely), I think that the only check we
>> > >> need in IORT is, depending on what type of SMMU a given device is
>> > >> connected to, to check if the respective SMMU driver is compiled in the
>> > >> kernel and it will be probed, _eventually_.
>> > >>
>> > >> As Robin said, by the time a device is probed the respective SMMU
>> > >> devices are already created and registered with IORT kernel code or
>> > >> they will never be, so to understand if we should defer probing
>> > >> SMMU device creation is _not_ really a problem in ACPI.
>> > >>
>> > >> To check if a SMMU driver is enabled, do we really need a linker
>> > >> table ?
>> > >>
>> > >> Would not a check based on eg:
>> > >>
>> > >> /**
>> > >>  * @type: IOMMU IORT node type of the IOMMU a device is connected to
>> > >>  */
>> > >> static bool iort_iommu_driver_enabled(u8 type)
>> > >> {
>> > >> 	switch (type) {
>> > >> 	case ACPI_IORT_SMMU_V3:
>> > >> 		return IS_ENABLED(CONFIG_ARM_SMMU_V3);
>> > >
>> > >IS_BUILTIN(...)
>> > >
>> > >> 	case ACPI_IORT_SMMU:
>> > >> 		return IS_ENABLED(CONFIG_ARM_SMMU);
>> > >> 	default:
>> > >> 		pr_warn("Unknown IORT SMMU type\n");
>> > >
>> > >Might displaying the actual value be helfpul for debugging a broken IORT
>> > >table?
>> > >
>> > >> 		return false;
>> > >> 	}
>> > >> }
>> > >>
>> > >> be sufficient (it is a bit gross, agreed, but it is to understand if
>> > >> that's all we need) ? Is there anything I am missing ?
>> > >>
>> > >> Let me know, I will put together a patch for you I really do not
>> > >> want to block your series for this trivial niggle.
>> > >
>> > >Other than that, though, I like it :) IORT has a small, strictly
>> > >bounded, set of supported devices, so I really don't see the need to go
>> > >overboard putting it on parity with DT when something this neat and
>> > >simple will suffice.
>> > >
>> >
>> > Ok sure, looks correct for me as well in whole of the context here.
>>
>> Ok, I put together a branch where you can find your original series
>> plus some ACPI patches for you to test and use:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral
>>
>> Feel free to post the additional patches I added along with your series
>> (that from what I gather you have reworked already) and please both have a
>> look if the deferral mechanism I put in place in ACPI makes sense to you.
>
>Did you have time to make progress on this ? I think it is time we
>posted the complete series to aim for 4.11 please, if you need help just
>let us know.
>

I just posted V5 now.  I have mainly reworked the pci dma configure code which
was lying outside and having it all in one place in dma_configure. The ACPI changes
from you looked fine for me. I have tested on arm64/32 with DT, and require testing
help on ACPI. Thanks.

Regards,
 Sricharan