[PATCH v2 0/4] iommu: A couple of urgent fixes

[PATCH v2 0/4] iommu: A couple of urgent fixes

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Hi Joerg,

Here are a couple of urgent fixes for a regression on old Tegra devices
related to IOMMU support. The issue is that many drivers think it's a
good idea to register IOMMU support unconditionally, which is not the
smart thing to do at all on multi-platform kernels. This probably went
unnoticed for a while because the offending drivers aren't enabled in
any of the multi-platform default configurations. Fedora ARM has their
own config where the offending drivers did get enabled, hence caused a
regression on Tegra20. I would expect the same regression to exist on a
number of other SoCs, possibly all that support IOMMU.

I've tried to keep the patches minimal in the hopes of still getting
this into v3.19-rc8 or the final release to avoid the regression.

Changes in v2:
- avoid potential leak by dropping references to device tree nodes
- remove fixups from module exit functions since they are unused

Thierry

Thierry Reding (4):
  iommu/exynos: Play nice in multi-platform builds
  iommu/omap: Play nice in multi-platform builds
  iommu/rockchip: Play nice in multi-platform builds
  iommu/msm: Mark driver BROKEN

 drivers/iommu/Kconfig          | 1 +
 drivers/iommu/exynos-iommu.c   | 7 +++++++
 drivers/iommu/omap-iommu.c     | 7 +++++++
 drivers/iommu/rockchip-iommu.c | 7 +++++++
 4 files changed, 22 insertions(+)

-- 
2.1.3

[PATCH v2 1/4] iommu/exynos: Play nice in multi-platform builds

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The Exynos System MMU driver unconditionally executes code and registers
a struct iommu_ops with the platform bus irrespective of whether it runs
on an Exynos SoC or not. This causes problems in multi-platform kernels
where drivers for other SoCs will no longer be able to register their
own struct iommu_ops or even try to use a struct iommu_ops for an IOMMU
that obviously isn't there.

The smallest fix I could think of is to check for the existence of any
Exynos System MMU devices in the device tree and skip initialization
otherwise.

This fixes a problem on Tegra20 where the DRM driver will try to use the
obviously non-existent Exynos System MMU.

Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
- drop reference to struct device_node

 drivers/iommu/exynos-iommu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 7ce52737c7a1..dc14fec4ede1 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1186,8 +1186,15 @@ static const struct iommu_ops exynos_iommu_ops = {
 
 static int __init exynos_iommu_init(void)
 {
+	struct device_node *np;
 	int ret;
 
+	np = of_find_matching_node(NULL, sysmmu_of_match);
+	if (!np)
+		return 0;
+
+	of_node_put(np);
+
 	lv2table_kmem_cache = kmem_cache_create("exynos-iommu-lv2table",
 				LV2TABLE_SIZE, LV2TABLE_SIZE, 0, NULL);
 	if (!lv2table_kmem_cache) {
-- 
2.1.3

[PATCH v2 2/4] iommu/omap: Play nice in multi-platform builds

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The OMAP IOMMU driver unconditionally executes code and registers a
struct iommu_ops with the platform bus irrespective of whether it runs
on an OMAP SoC or not. This causes problems in multi-platform kernels
where drivers for other SoCs will no longer be able to register their
own struct iommu_ops or even try to use a struct iommu_ops for an IOMMU
that obviously isn't there.

The smallest fix I could think of is to check for the existence of any
OMAP IOMMU devices in the device tree and skip initialization otherwise.

This fixes a problem on Tegra20 where the DRM driver will try to use the
obviously non-existent OMAP IOMMU.

Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
- do not fix up module exit function since it's dead code
- drop reference to struct device_node

 drivers/iommu/omap-iommu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index f59f857b702e..a4ba851825c2 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1376,6 +1376,13 @@ static int __init omap_iommu_init(void)
 	struct kmem_cache *p;
 	const unsigned long flags = SLAB_HWCACHE_ALIGN;
 	size_t align = 1 << 10; /* L2 pagetable alignement */
+	struct device_node *np;
+
+	np = of_find_matching_node(NULL, omap_iommu_of_match);
+	if (!np)
+		return 0;
+
+	of_node_put(np);
 
 	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
 			      iopte_cachep_ctor);
-- 
2.1.3

[PATCH v2 3/4] iommu/rockchip: Play nice in multi-platform builds

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The Rockchip IOMMU driver unconditionally executes code and registers a
struct iommu_ops with the platform bus irrespective of whether it runs
on a Rockchip SoC or not. This causes problems in multi-platform kernels
where drivers for other SoCs will no longer be able to register their
own struct iommu_ops or even try to use a struct iommu_ops for an IOMMU
that obviously isn't there.

The smallest fix I could think of is to check for the existence of any
Rockchip IOMMU devices in the device tree and skip initialization
otherwise.

This fixes a problem on Tegra20 where the DRM driver will try to use the
obviously non-existent Rockchip IOMMU.

Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
- do not fix up module exit function since it's dead code
- drop reference to struct device_node

 drivers/iommu/rockchip-iommu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 6a8b1ec4a48a..9f74fddcd304 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1015,8 +1015,15 @@ static struct platform_driver rk_iommu_driver = {
 
 static int __init rk_iommu_init(void)
 {
+	struct device_node *np;
 	int ret;
 
+	np = of_find_matching_node(NULL, rk_iommu_dt_ids);
+	if (!np)
+		return 0;
+
+	of_node_put(np);
+
 	ret = bus_set_iommu(&platform_bus_type, &rk_iommu_ops);
 	if (ret)
 		return ret;
-- 
2.1.3

[PATCH v2 4/4] iommu/msm: Mark driver BROKEN

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The MSM IOMMU driver unconditionally calls bus_set_iommu(), which is a
very stupid thing to do on multi-platform kernels. While marking the
driver BROKEN may seem a little extreme, there is no other way to make
the driver skip initialization. One of the problems is that it doesn't
have devicetree binding documentation and the driver doesn't contain a
struct of_device_id table either, so no way to check that it is indeed
valid to set up the IOMMU operations for this driver.

This fixes a problem on Tegra20 where the DRM driver will try to use the
obviously non-existent MSM IOMMU.

Marking the driver BROKEN shouldn't do any harm, since there aren't any
users currently. There is no struct of_device_id table, so the device
can't be instantiated from device tree, and I couldn't find any code
that would instantiate a matching platform_device either, so the driver
is effectively unused.

Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Daniel Walker <dwalker-zu3NM2574RrQT0dZR+AlfA@public.gmane.org>
Cc: Bryan Huntsman <bryanh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b280f0e21c8c..619bfd7bab1a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -63,6 +63,7 @@ config MSM_IOMMU
 	bool "MSM IOMMU Support"
 	depends on ARM
 	depends on ARCH_MSM8X60 || ARCH_MSM8960 || COMPILE_TEST
+	depends on BROKEN
 	select IOMMU_API
 	help
 	  Support for the IOMMUs found on certain Qualcomm SOCs.
-- 
2.1.3

Re: [PATCH v2 0/4] iommu: A couple of urgent fixes

[Suman Anna]

Hi Thierry,

On 02/06/2015 04:44 AM, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Hi Joerg,
> 
> Here are a couple of urgent fixes for a regression on old Tegra devices
> related to IOMMU support. The issue is that many drivers think it's a
> good idea to register IOMMU support unconditionally, which is not the
> smart thing to do at all on multi-platform kernels. This probably went
> unnoticed for a while because the offending drivers aren't enabled in
> any of the multi-platform default configurations. Fedora ARM has their
> own config where the offending drivers did get enabled, hence caused a
> regression on Tegra20. I would expect the same regression to exist on a
> number of other SoCs, possibly all that support IOMMU.
> 
> I've tried to keep the patches minimal in the hopes of still getting
> this into v3.19-rc8 or the final release to avoid the regression.
> 
> Changes in v2:
> - avoid potential leak by dropping references to device tree nodes
> - remove fixups from module exit functions since they are unused

Thanks for the revised patches, tested them on the OMAP platforms. I was
not able to apply the iommu/msm patch though against 3.19-rc7 cleanly.

regards
Suman

> Thierry
> 
> Thierry Reding (4):
>   iommu/exynos: Play nice in multi-platform builds
>   iommu/omap: Play nice in multi-platform builds
>   iommu/rockchip: Play nice in multi-platform builds
>   iommu/msm: Mark driver BROKEN
> 
>  drivers/iommu/Kconfig          | 1 +
>  drivers/iommu/exynos-iommu.c   | 7 +++++++
>  drivers/iommu/omap-iommu.c     | 7 +++++++
>  drivers/iommu/rockchip-iommu.c | 7 +++++++
>  4 files changed, 22 insertions(+)
> 

Re: [PATCH v2 2/4] iommu/omap: Play nice in multi-platform builds

[Laurent Pinchart]

Hi Thierry,

Thank you for the patch.

On Friday 06 February 2015 11:44:06 Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> The OMAP IOMMU driver unconditionally executes code and registers a
> struct iommu_ops with the platform bus irrespective of whether it runs
> on an OMAP SoC or not. This causes problems in multi-platform kernels
> where drivers for other SoCs will no longer be able to register their
> own struct iommu_ops or even try to use a struct iommu_ops for an IOMMU
> that obviously isn't there.
> 
> The smallest fix I could think of is to check for the existence of any
> OMAP IOMMU devices in the device tree and skip initialization otherwise.
> 
> This fixes a problem on Tegra20 where the DRM driver will try to use the
> obviously non-existent OMAP IOMMU.
> 
> Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
> Changes in v2:
> - do not fix up module exit function since it's dead code
> - drop reference to struct device_node
> 
>  drivers/iommu/omap-iommu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index f59f857b702e..a4ba851825c2 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1376,6 +1376,13 @@ static int __init omap_iommu_init(void)
>  	struct kmem_cache *p;
>  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
>  	size_t align = 1 << 10; /* L2 pagetable alignement */
> +	struct device_node *np;
> +
> +	np = of_find_matching_node(NULL, omap_iommu_of_match);
> +	if (!np)
> +		return 0;
> +
> +	of_node_put(np);
> 
>  	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
>  			      iopte_cachep_ctor);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] iommu: A couple of urgent fixes

[Joerg Roedel]

On Fri, Feb 06, 2015 at 11:44:04AM +0100, Thierry Reding wrote:
> Thierry Reding (4):
>   iommu/exynos: Play nice in multi-platform builds
>   iommu/omap: Play nice in multi-platform builds
>   iommu/rockchip: Play nice in multi-platform builds
>   iommu/msm: Mark driver BROKEN

Applied to my fixes branch.