Skip to contentSkip to conversation

iommu/exynos: Skip unsupported devices instead of returning -ENODEV

PATCH New Marek Szyprowski 3 participants 4 messages Jun 25, 2015 – Jun 29, 2015 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

Marek Szyprowski

author maintainer
#0 lore raw

PATCH 1/1 iommu/exynos: Skip unsupported devices instead of returning -ENODEV

IOMMU core calls add_device() for every device on the given bus and since commit 19762d7095e6392b6ec56c363a6f29b2119488c2 ("iommu: Propagate error in add_iommu_group") all error from it are propagated to the iommu core. In case of platform bus there are many devices without sysmmu controller, so skip them gently instead of returning error, which would be propagated up to bus_set_iommu() and cause initialization failure.

---
Please queue this patch to v4.2-rcX line. It fixes regression introduced by a commit merged in parallel to the exynos iommu patches.

---
 drivers/iommu/exynos-iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/iommu/exynos-iommu.c +2 -1
index 97c41b8ab5d9..e7f873253a6b 100644
@@ -1119,8 +1119,9 @@ static int exynos_iommu_add_device(struct device *dev)
 	struct iommu_group *group;
 	int ret;
 
+	/* skip devices which doesn't have sysmmu controller */
 	if (!has_sysmmu(dev))
-		return -ENODEV;
+		return 0;
 
 	group = iommu_group_get(dev);
 
Signature · 1 line
1.9.2
maintainer
#1 lore raw

PATCH 1/1 iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV

Hi Marek,

Review

Quoted 4 lines · Marek Szyprowski
(quoted diff) +2 -1
+	/* skip devices which doesn't have sysmmu controller */
 	if (!has_sysmmu(dev))
-		return -ENODEV;
+		return 0;

Thanks for reporting this! But I think that the -ENODEV return value could be of use for the iommu core in the future. Can you please try the attached patch, which just ignores -ENODEV as a return value from add_device?

Quoted 1 line

From 3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412 Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
Date: Mon, 29 Jun 2015 10:16:08 +0200
Subject: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back

The -ENODEV error just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.

No returning -ENODEV in the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.

---
 drivers/iommu/iommu.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/iommu/iommu.c +12 -1
index 49e7542..f286090 100644
@@ -847,13 +847,24 @@ static int add_iommu_group(struct device *dev, void *data)
 {
 	struct iommu_callback_data *cb = data;
 	const struct iommu_ops *ops = cb->ops;
+	int ret;
 
 	if (!ops->add_device)
 		return 0;
 
 	WARN_ON(dev->iommu_group);
 
-	return ops->add_device(dev);
+	ret = ops->add_device(dev);
+
+	/*
+	 * We ignore -ENODEV errors for now, as they just mean that the
+	 * device is not translated by an IOMMU. We still care about
+	 * other errors and fail to initialize when they happen.
+	 */
+	if (ret == -ENODEV)
+		ret = 0;
+
+	return ret;
 }
 
 static int remove_iommu_group(struct device *dev, void *data)
Signature · 1 line
1.8.4.5

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

Re: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV

Hello,

Quoted 22 lines · Joerg Roedel

Hi Marek,

Review

Quoted 4 lines · Marek Szyprowski
(quoted diff) +2 -1
+	/* skip devices which doesn't have sysmmu controller */
  	if (!has_sysmmu(dev))
-		return -ENODEV;
+		return 0;

Thanks for reporting this! But I think that the -ENODEV return value could be of use for the iommu core in the future. Can you please try the attached patch, which just ignores -ENODEV as a return value from add_device?

Quoted 1 line

From 3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412 Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
Date: Mon, 29 Jun 2015 10:16:08 +0200
Subject: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back

The -ENODEV error just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.

Quoted 6 lines

No returning -ENODEV in the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.

Works fine!

Quoted 24 lines
---
  drivers/iommu/iommu.c | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)
drivers/iommu/iommu.c +5 -1
index 49e7542..f286090 100644
@@ -847,13 +847,24 @@ static int add_iommu_group(struct device *dev, void *data)
  {
  	struct iommu_callback_data *cb = data;
  	const struct iommu_ops *ops = cb->ops;
+	int ret;
 
  	if (!ops->add_device)
  		return 0;
 
  	WARN_ON(dev->iommu_group);
 
-	return ops->add_device(dev);
+	ret = ops->add_device(dev);
+
+	/*
+	 * We ignore -ENODEV errors for now, as they just mean that the

drivers/iommu/iommu.c static int add_iommu_group(struct device *dev, void *data) · line 870

Quoted 10 lines
drivers/iommu/iommu.c +7 -0
+	 * device is not translated by an IOMMU. We still care about
+	 * other errors and fail to initialize when they happen.
+	 */
+	if (ret == -ENODEV)
+		ret = 0;
+
+	return ret;
  }
 
  static int remove_iommu_group(struct device *dev, void *data)

Best regards

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland
maintainer
#4 lore raw

Re: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV

Quoted 11 lines · Marek Szyprowski
Quoted 10 lines · Joerg Roedel

The -ENODEV error just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.

No returning -ENODEV in the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.

Quoted 5 lines
Quoted 1 line

Works fine!

Thanks for testing! The patch is queued now.