[PATCH 0/2] SH Mobile IOMMU fixes

[PATCH 0/2] SH Mobile IOMMU fixes

[Laurent Pinchart]

Hello,

These two small patches fix bugs in the SH Mobile IOMMU driver that result in
crashes at runtime. Given that the bugs have been present in the driver since
v3.9 there's no urgency to push the fixes to v3.13.

Laurent Pinchart (2):
  iommu/shmobile: Allocate archdata with kzalloc()
  iommu/shmobile: Turn the flush_lock mutex into a spinlock

 drivers/iommu/shmobile-iommu.c |  3 +--
 drivers/iommu/shmobile-ipmmu.c | 10 +++++-----
 drivers/iommu/shmobile-ipmmu.h |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 2/2] iommu/shmobile: Turn the flush_lock mutex into a spinlock

[Laurent Pinchart]

The lock is taken in atomic context, replace it with a spinlock.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-ipmmu.c | 10 +++++-----
 drivers/iommu/shmobile-ipmmu.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index 8321f89..e3bc2e1 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -35,12 +35,12 @@ void ipmmu_tlb_flush(struct shmobile_ipmmu *ipmmu)
 	if (!ipmmu)
 		return;
 
-	mutex_lock(&ipmmu->flush_lock);
+	spin_lock(&ipmmu->flush_lock);
 	if (ipmmu->tlb_enabled)
 		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
 	else
 		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH);
-	mutex_unlock(&ipmmu->flush_lock);
+	spin_unlock(&ipmmu->flush_lock);
 }
 
 void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
@@ -49,7 +49,7 @@ void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
 	if (!ipmmu)
 		return;
 
-	mutex_lock(&ipmmu->flush_lock);
+	spin_lock(&ipmmu->flush_lock);
 	switch (size) {
 	default:
 		ipmmu->tlb_enabled = 0;
@@ -85,7 +85,7 @@ void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
 	}
 	ipmmu_reg_write(ipmmu, IMTTBR, phys);
 	ipmmu_reg_write(ipmmu, IMASID, asid);
-	mutex_unlock(&ipmmu->flush_lock);
+	spin_unlock(&ipmmu->flush_lock);
 }
 
 static int ipmmu_probe(struct platform_device *pdev)
@@ -104,7 +104,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "cannot allocate device data\n");
 		return -ENOMEM;
 	}
-	mutex_init(&ipmmu->flush_lock);
+	spin_lock_init(&ipmmu->flush_lock);
 	ipmmu->dev = &pdev->dev;
 	ipmmu->ipmmu_base = devm_ioremap_nocache(&pdev->dev, res->start,
 						resource_size(res));
diff --git a/drivers/iommu/shmobile-ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
index 4d53684..9524743 100644
--- a/drivers/iommu/shmobile-ipmmu.h
+++ b/drivers/iommu/shmobile-ipmmu.h
@@ -14,7 +14,7 @@ struct shmobile_ipmmu {
 	struct device *dev;
 	void __iomem *ipmmu_base;
 	int tlb_enabled;
-	struct mutex flush_lock;
+	spinlock_t flush_lock;
 	const char * const *dev_names;
 	unsigned int num_dev_names;
 };
-- 
1.8.3.2

Re: [PATCH 2/2] iommu/shmobile: Turn the flush_lock mutex into a spinlock

[Simon Horman]

On Tue, Dec 17, 2013 at 06:18:50PM +0100, Laurent Pinchart wrote:
> The lock is taken in atomic context, replace it with a spinlock.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/iommu/shmobile-ipmmu.c | 10 +++++-----
>  drivers/iommu/shmobile-ipmmu.h |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
> index 8321f89..e3bc2e1 100644
> --- a/drivers/iommu/shmobile-ipmmu.c
> +++ b/drivers/iommu/shmobile-ipmmu.c
> @@ -35,12 +35,12 @@ void ipmmu_tlb_flush(struct shmobile_ipmmu *ipmmu)
>  	if (!ipmmu)
>  		return;
>  
> -	mutex_lock(&ipmmu->flush_lock);
> +	spin_lock(&ipmmu->flush_lock);
>  	if (ipmmu->tlb_enabled)
>  		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
>  	else
>  		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH);
> -	mutex_unlock(&ipmmu->flush_lock);
> +	spin_unlock(&ipmmu->flush_lock);
>  }
>  
>  void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
> @@ -49,7 +49,7 @@ void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
>  	if (!ipmmu)
>  		return;
>  
> -	mutex_lock(&ipmmu->flush_lock);
> +	spin_lock(&ipmmu->flush_lock);
>  	switch (size) {
>  	default:
>  		ipmmu->tlb_enabled = 0;
> @@ -85,7 +85,7 @@ void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
>  	}
>  	ipmmu_reg_write(ipmmu, IMTTBR, phys);
>  	ipmmu_reg_write(ipmmu, IMASID, asid);
> -	mutex_unlock(&ipmmu->flush_lock);
> +	spin_unlock(&ipmmu->flush_lock);
>  }
>  
>  static int ipmmu_probe(struct platform_device *pdev)
> @@ -104,7 +104,7 @@ static int ipmmu_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "cannot allocate device data\n");
>  		return -ENOMEM;
>  	}
> -	mutex_init(&ipmmu->flush_lock);
> +	spin_lock_init(&ipmmu->flush_lock);
>  	ipmmu->dev = &pdev->dev;
>  	ipmmu->ipmmu_base = devm_ioremap_nocache(&pdev->dev, res->start,
>  						resource_size(res));
> diff --git a/drivers/iommu/shmobile-ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
> index 4d53684..9524743 100644
> --- a/drivers/iommu/shmobile-ipmmu.h
> +++ b/drivers/iommu/shmobile-ipmmu.h
> @@ -14,7 +14,7 @@ struct shmobile_ipmmu {
>  	struct device *dev;
>  	void __iomem *ipmmu_base;
>  	int tlb_enabled;
> -	struct mutex flush_lock;
> +	spinlock_t flush_lock;
>  	const char * const *dev_names;
>  	unsigned int num_dev_names;
>  };
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 1/2] iommu/shmobile: Allocate archdata with kzalloc()

[Laurent Pinchart]

The archdata attached_list field isn't initialized, leading to random
crashes when accessed. Use kzalloc() to allocate the whole structure and
make sure all fields get initialized properly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index d572863..7a3b928 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -380,14 +380,13 @@ int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
 		kmem_cache_destroy(l1cache);
 		return -ENOMEM;
 	}
-	archdata = kmalloc(sizeof(*archdata), GFP_KERNEL);
+	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
 	if (!archdata) {
 		kmem_cache_destroy(l1cache);
 		kmem_cache_destroy(l2cache);
 		return -ENOMEM;
 	}
 	spin_lock_init(&archdata->attach_lock);
-	archdata->attached = NULL;
 	archdata->ipmmu = ipmmu;
 	ipmmu_archdata = archdata;
 	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
-- 
1.8.3.2

Re: [PATCH 1/2] iommu/shmobile: Allocate archdata with kzalloc()

[Simon Horman]

On Tue, Dec 17, 2013 at 06:18:49PM +0100, Laurent Pinchart wrote:
> The archdata attached_list field isn't initialized, leading to random
> crashes when accessed. Use kzalloc() to allocate the whole structure and
> make sure all fields get initialized properly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/iommu/shmobile-iommu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
> index d572863..7a3b928 100644
> --- a/drivers/iommu/shmobile-iommu.c
> +++ b/drivers/iommu/shmobile-iommu.c
> @@ -380,14 +380,13 @@ int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
>  		kmem_cache_destroy(l1cache);
>  		return -ENOMEM;
>  	}
> -	archdata = kmalloc(sizeof(*archdata), GFP_KERNEL);
> +	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
>  	if (!archdata) {
>  		kmem_cache_destroy(l1cache);
>  		kmem_cache_destroy(l2cache);
>  		return -ENOMEM;
>  	}
>  	spin_lock_init(&archdata->attach_lock);
> -	archdata->attached = NULL;
>  	archdata->ipmmu = ipmmu;
>  	ipmmu_archdata = archdata;
>  	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
> -- 
> 1.8.3.2
> 

Re: [PATCH 1/2] iommu/shmobile: Allocate archdata with kzalloc()

[Laurent Pinchart]

On Wednesday 18 December 2013 10:15:21 Simon Horman wrote:
> On Tue, Dec 17, 2013 at 06:18:49PM +0100, Laurent Pinchart wrote:
> > The archdata attached_list field isn't initialized, leading to random
> > crashes when accessed. Use kzalloc() to allocate the whole structure and
> > make sure all fields get initialized properly.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thank you Simon.

Joerg, could you please pick those two patches up for v3.14 ?

> > ---
> > 
> >  drivers/iommu/shmobile-iommu.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/shmobile-iommu.c
> > b/drivers/iommu/shmobile-iommu.c index d572863..7a3b928 100644
> > --- a/drivers/iommu/shmobile-iommu.c
> > +++ b/drivers/iommu/shmobile-iommu.c
> > @@ -380,14 +380,13 @@ int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
> >  		kmem_cache_destroy(l1cache);
> >  		return -ENOMEM;
> >  	}
> > -	archdata = kmalloc(sizeof(*archdata), GFP_KERNEL);
> > +	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
> >  	if (!archdata) {
> >  		kmem_cache_destroy(l1cache);
> >  		kmem_cache_destroy(l2cache);
> >  		return -ENOMEM;
> >  	}
> >  	spin_lock_init(&archdata->attach_lock);
> > -	archdata->attached = NULL;
> >  	archdata->ipmmu = ipmmu;
> >  	ipmmu_archdata = archdata;
> >  	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] SH Mobile IOMMU fixes

[Joerg Roedel]

On Tue, Dec 17, 2013 at 06:18:48PM +0100, Laurent Pinchart wrote:
> Hello,
> 
> These two small patches fix bugs in the SH Mobile IOMMU driver that result in
> crashes at runtime. Given that the bugs have been present in the driver since
> v3.9 there's no urgency to push the fixes to v3.13.
> 
> Laurent Pinchart (2):
>   iommu/shmobile: Allocate archdata with kzalloc()
>   iommu/shmobile: Turn the flush_lock mutex into a spinlock
> 
>  drivers/iommu/shmobile-iommu.c |  3 +--
>  drivers/iommu/shmobile-ipmmu.c | 10 +++++-----
>  drivers/iommu/shmobile-ipmmu.h |  2 +-
>  3 files changed, 7 insertions(+), 8 deletions(-)

Applied both, thanks.