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

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

[Marek Szyprowski]

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.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
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(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 97c41b8ab5d9..e7f873253a6b 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -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);
 
-- 
1.9.2

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

[Joerg Roedel]

Hi Marek,

On Thu, Jun 25, 2015 at 03:10:44PM +0200, Marek Szyprowski wrote:
> +	/* 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?

>From 3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
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.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 49e7542..f286090 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -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)
-- 
1.8.4.5

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

[Marek Szyprowski]

Hello,

On 2015-06-29 10:35, Joerg Roedel wrote:
> Hi Marek,
>
> On Thu, Jun 25, 2015 at 03:10:44PM +0200, Marek Szyprowski wrote:
>> +	/* 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?
>
> >From 3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412 Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 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.
>
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Works fine!

Tested-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> ---
>   drivers/iommu/iommu.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 49e7542..f286090 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -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)

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

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

[Joerg Roedel]

On Mon, Jun 29, 2015 at 10:53:43AM +0200, Marek Szyprowski wrote:
> On 2015-06-29 10:35, Joerg Roedel wrote:
> >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.
> >
> >Signed-off-by: Joerg Roedel <jroedel@suse.de>
> 
> Works fine!
> 
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks for testing! The patch is queued now.