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

[PATCH 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.

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   | 3 +++
 drivers/iommu/omap-iommu.c     | 6 ++++++
 drivers/iommu/rockchip-iommu.c | 6 ++++++
 4 files changed, 16 insertions(+)

-- 
2.1.3

[PATCH 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>
---
 drivers/iommu/exynos-iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 7ce52737c7a1..d4b41fa32368 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1188,6 +1188,9 @@ static int __init exynos_iommu_init(void)
 {
 	int ret;
 
+	if (!of_find_matching_node(NULL, sysmmu_of_match))
+		return 0;
+
 	lv2table_kmem_cache = kmem_cache_create("exynos-iommu-lv2table",
 				LV2TABLE_SIZE, LV2TABLE_SIZE, 0, NULL);
 	if (!lv2table_kmem_cache) {
-- 
2.1.3

[PATCH 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>
---
 drivers/iommu/omap-iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bbb7dcef02d3..e4d4f133a3b3 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
 	const unsigned long flags = SLAB_HWCACHE_ALIGN;
 	size_t align = 1 << 10; /* L2 pagetable alignement */
 
+	if (!of_find_matching_node(NULL, omap_iommu_of_match))
+		return 0;
+
 	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
 			      iopte_cachep_ctor);
 	if (!p)
@@ -1394,6 +1397,9 @@ subsys_initcall(omap_iommu_init);
 
 static void __exit omap_iommu_exit(void)
 {
+	if (!of_find_matching_node(NULL, omap_iommu_of_match))
+		return;
+
 	kmem_cache_destroy(iopte_cachep);
 
 	platform_driver_unregister(&omap_iommu_driver);
-- 
2.1.3

[PATCH 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>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/rockchip-iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 6a8b1ec4a48a..869e9c9d5df7 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1017,6 +1017,9 @@ static int __init rk_iommu_init(void)
 {
 	int ret;
 
+	if (!of_find_matching_node(NULL, rk_iommu_dt_ids))
+		return 0;
+
 	ret = bus_set_iommu(&platform_bus_type, &rk_iommu_ops);
 	if (ret)
 		return ret;
@@ -1025,6 +1028,9 @@ static int __init rk_iommu_init(void)
 }
 static void __exit rk_iommu_exit(void)
 {
+	if (!of_find_matching_node(NULL, rk_iommu_dt_ids))
+		return;
+
 	platform_driver_unregister(&rk_iommu_driver);
 }
 
-- 
2.1.3

[PATCH 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>
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 325188eef1c1..0f70bc1fce65 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -31,6 +31,7 @@ config FSL_PAMU
 config MSM_IOMMU
 	bool "MSM IOMMU Support"
 	depends on ARCH_MSM8X60 || ARCH_MSM8960
+	depends on BROKEN
 	select IOMMU_API
 	help
 	  Support for the IOMMUs found on certain Qualcomm SOCs.
-- 
2.1.3

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

[Marek Szyprowski]

Hello,

On 2015-02-04 08:58, Thierry Reding wrote:
> 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>

Frankly, you may mark the existing exynos iommu driver as BROKEN, what will
solve a few other issues as well. In the current version this driver is not
functional and not used on any platform. I thought that my fixes will get
into v3.20, but it looks that it won't happen.

> ---
>   drivers/iommu/exynos-iommu.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 7ce52737c7a1..d4b41fa32368 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1188,6 +1188,9 @@ static int __init exynos_iommu_init(void)
>   {
>   	int ret;
>   
> +	if (!of_find_matching_node(NULL, sysmmu_of_match))
> +		return 0;
> +
>   	lv2table_kmem_cache = kmem_cache_create("exynos-iommu-lv2table",
>   				LV2TABLE_SIZE, LV2TABLE_SIZE, 0, NULL);
>   	if (!lv2table_kmem_cache) {

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

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

[Laurent Pinchart]

Hi Thierry,

Thank you for the patch.

On Wednesday 04 February 2015 08:58:08 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>
> ---
>  drivers/iommu/omap-iommu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index bbb7dcef02d3..e4d4f133a3b3 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
>  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
>  	size_t align = 1 << 10; /* L2 pagetable alignement */
> 
> +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> +		return 0;
> +

We should convert the omap-iommu driver to proper DT instantiation, but this 
should do for now.

>  	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
>  			      iopte_cachep_ctor);
>  	if (!p)
> @@ -1394,6 +1397,9 @@ subsys_initcall(omap_iommu_init);
> 
>  static void __exit omap_iommu_exit(void)
>  {
> +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> +		return;
> +
>  	kmem_cache_destroy(iopte_cachep);
> 
>  	platform_driver_unregister(&omap_iommu_driver);

The exit function will never be called as the omap-iommu driver is always 
built-in. You could just remove the exit function.

-- 
Regards,

Laurent Pinchart

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

[Suman Anna]

Hi Thierry,

On 02/04/2015 08:31 AM, Laurent Pinchart wrote:
> Hi Thierry,
> 
> Thank you for the patch.
> 
> On Wednesday 04 February 2015 08:58:08 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>
>> ---
>>  drivers/iommu/omap-iommu.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>> index bbb7dcef02d3..e4d4f133a3b3 100644
>> --- a/drivers/iommu/omap-iommu.c
>> +++ b/drivers/iommu/omap-iommu.c
>> @@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
>>  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
>>  	size_t align = 1 << 10; /* L2 pagetable alignement */
>>
>> +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
>> +		return 0;
>> +

Need to call the of_node_put on the cases it succeeds right, you should
change this to the semantics you used in the arm-smmu driver.

> 
> We should convert the omap-iommu driver to proper DT instantiation, but this 
> should do for now.

Agreed, and I think this check is better than the iommu_present check
used in some of the other IOMMU drivers, as that would work for a SoC
family only if that is the first one getting registered.

regards
Suman

> 
>>  	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
>>  			      iopte_cachep_ctor);
>>  	if (!p)
>> @@ -1394,6 +1397,9 @@ subsys_initcall(omap_iommu_init);
>>
>>  static void __exit omap_iommu_exit(void)
>>  {
>> +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
>> +		return;
>> +
>>  	kmem_cache_destroy(iopte_cachep);
>>
>>  	platform_driver_unregister(&omap_iommu_driver);
> 
> The exit function will never be called as the omap-iommu driver is always 
> built-in. You could just remove the exit function.
> 

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

[Olav Haugan]

Adding Mitch H. and Rob Clark.

On 2/3/2015 11:58 PM, Thierry Reding wrote:
> 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>
> 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 325188eef1c1..0f70bc1fce65 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -31,6 +31,7 @@ config FSL_PAMU
>   config MSM_IOMMU
>   	bool "MSM IOMMU Support"
>   	depends on ARCH_MSM8X60 || ARCH_MSM8960
> +	depends on BROKEN
>   	select IOMMU_API
>   	help
>   	  Support for the IOMMUs found on certain Qualcomm SOCs.
>


.Olav

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Rob Clark]

On Wed, Feb 4, 2015 at 1:54 PM, Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Adding Mitch H. and Rob Clark.
>

The current upstream msm-iommu isn't actually used by anything,
afaict.. it lacks support for DT based platforms.  So this is fine by
me.

Acked-by: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


>
> On 2/3/2015 11:58 PM, Thierry Reding wrote:
>>
>> 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>
>> 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 325188eef1c1..0f70bc1fce65 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -31,6 +31,7 @@ config FSL_PAMU
>>   config MSM_IOMMU
>>         bool "MSM IOMMU Support"
>>         depends on ARCH_MSM8X60 || ARCH_MSM8960
>> +       depends on BROKEN
>>         select IOMMU_API
>>         help
>>           Support for the IOMMUs found on certain Qualcomm SOCs.
>>
>
>
> .Olav
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

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

[Heiko Stübner]

Hi Thierry,

Am Mittwoch, 4. Februar 2015, 08:58:09 schrieb 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>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

on a rk3288-firefly with drm through the hdmi connector
Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

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

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2833 bytes --]

On Wed, Feb 04, 2015 at 04:31:55PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> Thank you for the patch.
> 
> On Wednesday 04 February 2015 08:58:08 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>
> > ---
> >  drivers/iommu/omap-iommu.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> > index bbb7dcef02d3..e4d4f133a3b3 100644
> > --- a/drivers/iommu/omap-iommu.c
> > +++ b/drivers/iommu/omap-iommu.c
> > @@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
> >  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
> >  	size_t align = 1 << 10; /* L2 pagetable alignement */
> > 
> > +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> > +		return 0;
> > +
> 
> We should convert the omap-iommu driver to proper DT instantiation, but this 
> should do for now.
> 
> >  	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
> >  			      iopte_cachep_ctor);
> >  	if (!p)
> > @@ -1394,6 +1397,9 @@ subsys_initcall(omap_iommu_init);
> > 
> >  static void __exit omap_iommu_exit(void)
> >  {
> > +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> > +		return;
> > +
> >  	kmem_cache_destroy(iopte_cachep);
> > 
> >  	platform_driver_unregister(&omap_iommu_driver);
> 
> The exit function will never be called as the omap-iommu driver is always 
> built-in. You could just remove the exit function.

I've omitted this hunk since this code will not run anyway. I agree that
we should remove the code altogether if it's dead anyway, but that kind
of cleanup isn't really suitable for patches this late in the release
cycle.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2227 bytes --]

On Wed, Feb 04, 2015 at 11:37:52AM -0600, Suman Anna wrote:
> Hi Thierry,
> 
> On 02/04/2015 08:31 AM, Laurent Pinchart wrote:
> > Hi Thierry,
> > 
> > Thank you for the patch.
> > 
> > On Wednesday 04 February 2015 08:58:08 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>
> >> ---
> >>  drivers/iommu/omap-iommu.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> >> index bbb7dcef02d3..e4d4f133a3b3 100644
> >> --- a/drivers/iommu/omap-iommu.c
> >> +++ b/drivers/iommu/omap-iommu.c
> >> @@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
> >>  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
> >>  	size_t align = 1 << 10; /* L2 pagetable alignement */
> >>
> >> +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> >> +		return 0;
> >> +
> 
> Need to call the of_node_put on the cases it succeeds right, you should
> change this to the semantics you used in the arm-smmu driver.

Good point. I've sent out an updated set of patches.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]