RE: [PATCH V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error

Hi Lorenzo,

[+hanjun, tomasz, sinan]

It is quite a key patchset, I would be glad if they can test on their
respective platforms with IORT.

On Mon, Jan 23, 2017 at 09:48:10PM +0530, Sricharan R wrote:

quoted

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 <redacted>
---
 drivers/acpi/arm64/iort.c  | 25 ++++++++++++++++++++++++-
 drivers/acpi/scan.c        |  7 +++++--
 drivers/base/dma-mapping.c |  2 +-
 include/acpi/acpi_bus.h    |  2 +-
 include/linux/acpi.h       |  7 +++++--
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index bf0ed09..d01bae8 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c

@@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
 			return NULL;

 		ops = iommu_get_instance(iort_fwnode);
+		/*
+		 * If the ops look-up fails, this means that either
+		 * the SMMU drivers have not been probed yet or that
+		 * the SMMU drivers are not built in the kernel;
+		 * Depending on whether the SMMU drivers are built-in
+		 * in the kernel or not, defer the IOMMU configuration
+		 * or just abort it.
+		 */
 		if (!ops)
-			return NULL;
+			return iort_iommu_driver_enabled(node->type) ?
+			       ERR_PTR(-EPROBE_DEFER) : NULL;

 		ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
 	}

@@ -625,12 +634,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);
+	}

I think there is nothing ACPI specific in this add_device() replay
path, so there is room for further DT/ACPI consolidation here.

ok, the only way is keep this in a function and call it for both DT and ACPI
cases.

Without any further ado:

Acked-by: Lorenzo Pieralisi <redacted>

Thanks for that.

Regards,
 Sricharan