[PATCHv2 0/2] OMAP IOMMU Fixes for DT-clients

[PATCHv2 0/2] OMAP IOMMU Fixes for DT-clients

[Suman Anna]

Hi,

This is an updated series addressing Laurent's review comments on
Patch1. Patch2 is unchanged from the previous series.

Patches are based on 3.17-rc3.

v1:
http://marc.info/?l=linux-omap&m=140978874026176&w=2

Suman Anna (2):
  iommu/omap: Check for valid archdata in attach_dev
  iommu/omap: Fix iommu archdata name for DT-based devices

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

-- 
2.0.4

[PATCHv2 2/2] iommu/omap: Fix iommu archdata name for DT-based devices

[Suman Anna]

A device is tied to an iommu through its archdata field. The archdata
is allocated on the fly for DT-based devices automatically through the
.add_device iommu ops. The current logic incorrectly assigned the name
of the IOMMU user device, instead of the name of the IOMMU device as
required by the attach logic. Fix this issue so that DT-based devices
can attach successfully to an IOMMU domain.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/iommu/omap-iommu.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index ea6e59d..8cf8bf1 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -26,6 +26,7 @@
 #include <linux/of.h>
 #include <linux/of_iommu.h>
 #include <linux/of_irq.h>
+#include <linux/of_platform.h>
 
 #include <asm/cacheflush.h>
 
@@ -1243,6 +1244,7 @@ static int omap_iommu_add_device(struct device *dev)
 {
 	struct omap_iommu_arch_data *arch_data;
 	struct device_node *np;
+	struct platform_device *pdev;
 
 	/*
 	 * Allocate the archdata iommu structure for DT-based devices.
@@ -1257,13 +1259,19 @@ static int omap_iommu_add_device(struct device *dev)
 	if (!np)
 		return 0;
 
+	pdev = of_find_device_by_node(np);
+	if (WARN_ON(!pdev)) {
+		of_node_put(np);
+		return -EINVAL;
+	}
+
 	arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL);
 	if (!arch_data) {
 		of_node_put(np);
 		return -ENOMEM;
 	}
 
-	arch_data->name = kstrdup(dev_name(dev), GFP_KERNEL);
+	arch_data->name = kstrdup(dev_name(&pdev->dev), GFP_KERNEL);
 	dev->archdata.iommu = arch_data;
 
 	of_node_put(np);
-- 
2.0.4

Re: [PATCHv2 2/2] iommu/omap: Fix iommu archdata name for DT-based devices

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 04 September 2014 17:27:30 Suman Anna wrote:
> A device is tied to an iommu through its archdata field. The archdata
> is allocated on the fly for DT-based devices automatically through the
> .add_device iommu ops. The current logic incorrectly assigned the name
> of the IOMMU user device, instead of the name of the IOMMU device as
> required by the attach logic. Fix this issue so that DT-based devices
> can attach successfully to an IOMMU domain.
> 
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

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

> ---
>  drivers/iommu/omap-iommu.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index ea6e59d..8cf8bf1 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -26,6 +26,7 @@
>  #include <linux/of.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> 
>  #include <asm/cacheflush.h>
> 
> @@ -1243,6 +1244,7 @@ static int omap_iommu_add_device(struct device *dev)
>  {
>  	struct omap_iommu_arch_data *arch_data;
>  	struct device_node *np;
> +	struct platform_device *pdev;
> 
>  	/*
>  	 * Allocate the archdata iommu structure for DT-based devices.
> @@ -1257,13 +1259,19 @@ static int omap_iommu_add_device(struct device *dev)
> if (!np)
>  		return 0;
> 
> +	pdev = of_find_device_by_node(np);
> +	if (WARN_ON(!pdev)) {
> +		of_node_put(np);
> +		return -EINVAL;
> +	}
> +
>  	arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL);
>  	if (!arch_data) {
>  		of_node_put(np);
>  		return -ENOMEM;
>  	}
> 
> -	arch_data->name = kstrdup(dev_name(dev), GFP_KERNEL);
> +	arch_data->name = kstrdup(dev_name(&pdev->dev), GFP_KERNEL);
>  	dev->archdata.iommu = arch_data;
> 
>  	of_node_put(np);

-- 
Regards,

Laurent Pinchart

[PATCHv2 1/2] iommu/omap: Check for valid archdata in attach_dev

[Suman Anna]

Any device requiring to be attached to an iommu_domain must have
valid archdata containing the necessary iommu information, which
is SoC-specific. Add a check in the omap_iommu_attach_dev to make
sure that the device has valid archdata before accessing
different SoC-specific fields of the archdata. This prevents a
NULL pointer dereference on any misconfigured devices.

Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 drivers/iommu/omap-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 02ef0ac..ea6e59d 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1090,6 +1090,11 @@ omap_iommu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
 	int ret = 0;
 
+	if (!arch_data || !arch_data->name) {
+		dev_err(dev, "device doesn't have an associated iommu\n");
+		return -EINVAL;
+	}
+
 	spin_lock(&omap_domain->lock);
 
 	/* only a single device is supported per domain for now */
-- 
2.0.4

Re: [PATCHv2 1/2] iommu/omap: Check for valid archdata in attach_dev

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 04 September 2014 17:27:29 Suman Anna wrote:
> Any device requiring to be attached to an iommu_domain must have
> valid archdata containing the necessary iommu information, which
> is SoC-specific. Add a check in the omap_iommu_attach_dev to make
> sure that the device has valid archdata before accessing
> different SoC-specific fields of the archdata. This prevents a
> NULL pointer dereference on any misconfigured devices.
> 
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

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

> ---
>  drivers/iommu/omap-iommu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 02ef0ac..ea6e59d 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1090,6 +1090,11 @@ omap_iommu_attach_dev(struct iommu_domain *domain,
> struct device *dev) struct omap_iommu_arch_data *arch_data =
> dev->archdata.iommu;
>  	int ret = 0;
> 
> +	if (!arch_data || !arch_data->name) {
> +		dev_err(dev, "device doesn't have an associated iommu\n");
> +		return -EINVAL;
> +	}
> +
>  	spin_lock(&omap_domain->lock);
> 
>  	/* only a single device is supported per domain for now */

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 0/2] OMAP IOMMU Fixes for DT-clients

[Joerg Roedel]

On Thu, Sep 04, 2014 at 05:27:28PM -0500, Suman Anna wrote:
> Suman Anna (2):
>   iommu/omap: Check for valid archdata in attach_dev
>   iommu/omap: Fix iommu archdata name for DT-based devices
> 
>  drivers/iommu/omap-iommu.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Applied, thanks.