All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 14:21 ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2015-01-23 14:21 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: jroedel-l3A5Bk7waGM, Heiko Stuebner, arnd-r2nGTMty4D4,
	Will Deacon, Thierry Reding, Alexandre Courbot,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
into arch_setup_dma_ops") moved the setting of the DMA operations from
arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
operations to be used are selected based on whether the device is
connected to an IOMMU. However, the IOMMU detection scheme requires the
IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
has been ported yet, this effectively breaks all IOMMU ARM users that
depend on the IOMMU being handled transparently by the DMA mapping API.

Fix this by restoring the setting of DMA IOMMU ops in
arm_iommu_attach_device() and splitting the rest of the function into a
new internal __arm_iommu_attach_device() function, called by
arch_setup_dma_ops().

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 arch/arm/mm/dma-mapping.c | 53 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 15 deletions(-)

This fixes a v3.19 regression that breaks IOMMU on the Exynos, OMAP3, Renesas
and Rockchip platforms.

The patch has been tested on a Renesas M2 platform with the ipmmu-vmsa driver.

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7864797609b3..a673c7f7e208 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1940,13 +1940,32 @@ void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping)
 }
 EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
 
+static int __arm_iommu_attach_device(struct device *dev,
+				     struct dma_iommu_mapping *mapping)
+{
+	int err;
+
+	err = iommu_attach_device(mapping->domain, dev);
+	if (err)
+		return err;
+
+	kref_get(&mapping->kref);
+	dev->archdata.mapping = mapping;
+
+	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
+	return 0;
+}
+
 /**
  * arm_iommu_attach_device
  * @dev: valid struct device pointer
  * @mapping: io address space mapping structure (returned from
  *	arm_iommu_create_mapping)
  *
- * Attaches specified io address space mapping to the provided device,
+ * Attaches specified io address space mapping to the provided device.
+ * This replaces the dma operations (dma_map_ops pointer) with the
+ * IOMMU aware version.
+ *
  * More than one client might be attached to the same io address space
  * mapping.
  */
@@ -1955,25 +1974,16 @@ int arm_iommu_attach_device(struct device *dev,
 {
 	int err;
 
-	err = iommu_attach_device(mapping->domain, dev);
+	err = __arm_iommu_attach_device(dev, mapping);
 	if (err)
 		return err;
 
-	kref_get(&mapping->kref);
-	dev->archdata.mapping = mapping;
-
-	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
+	set_dma_ops(dev, &iommu_ops);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
 
-/**
- * arm_iommu_detach_device
- * @dev: valid struct device pointer
- *
- * Detaches the provided device from a previously attached map.
- */
-void arm_iommu_detach_device(struct device *dev)
+static void __arm_iommu_detach_device(struct device *dev)
 {
 	struct dma_iommu_mapping *mapping;
 
@@ -1989,6 +1999,19 @@ void arm_iommu_detach_device(struct device *dev)
 
 	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
 }
+
+/**
+ * arm_iommu_detach_device
+ * @dev: valid struct device pointer
+ *
+ * Detaches the provided device from a previously attached map.
+ * This voids the dma operations (dma_map_ops pointer)
+ */
+void arm_iommu_detach_device(struct device *dev)
+{
+	__arm_iommu_detach_device(dev);
+	set_dma_ops(dev, NULL);
+}
 EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
 
 static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
@@ -2011,7 +2034,7 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		return false;
 	}
 
-	if (arm_iommu_attach_device(dev, mapping)) {
+	if (__arm_iommu_attach_device(dev, mapping)) {
 		pr_warn("Failed to attached device %s to IOMMU_mapping\n",
 				dev_name(dev));
 		arm_iommu_release_mapping(mapping);
@@ -2025,7 +2048,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev)
 {
 	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
 
-	arm_iommu_detach_device(dev);
+	__arm_iommu_detach_device(dev);
 	arm_iommu_release_mapping(mapping);
 }
 
-- 
Regards,

Laurent Pinchart

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 14:21 ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2015-01-23 14:21 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
into arch_setup_dma_ops") moved the setting of the DMA operations from
arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
operations to be used are selected based on whether the device is
connected to an IOMMU. However, the IOMMU detection scheme requires the
IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
has been ported yet, this effectively breaks all IOMMU ARM users that
depend on the IOMMU being handled transparently by the DMA mapping API.

Fix this by restoring the setting of DMA IOMMU ops in
arm_iommu_attach_device() and splitting the rest of the function into a
new internal __arm_iommu_attach_device() function, called by
arch_setup_dma_ops().

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mm/dma-mapping.c | 53 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 15 deletions(-)

This fixes a v3.19 regression that breaks IOMMU on the Exynos, OMAP3, Renesas
and Rockchip platforms.

The patch has been tested on a Renesas M2 platform with the ipmmu-vmsa driver.

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7864797609b3..a673c7f7e208 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1940,13 +1940,32 @@ void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping)
 }
 EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
 
+static int __arm_iommu_attach_device(struct device *dev,
+				     struct dma_iommu_mapping *mapping)
+{
+	int err;
+
+	err = iommu_attach_device(mapping->domain, dev);
+	if (err)
+		return err;
+
+	kref_get(&mapping->kref);
+	dev->archdata.mapping = mapping;
+
+	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
+	return 0;
+}
+
 /**
  * arm_iommu_attach_device
  * @dev: valid struct device pointer
  * @mapping: io address space mapping structure (returned from
  *	arm_iommu_create_mapping)
  *
- * Attaches specified io address space mapping to the provided device,
+ * Attaches specified io address space mapping to the provided device.
+ * This replaces the dma operations (dma_map_ops pointer) with the
+ * IOMMU aware version.
+ *
  * More than one client might be attached to the same io address space
  * mapping.
  */
@@ -1955,25 +1974,16 @@ int arm_iommu_attach_device(struct device *dev,
 {
 	int err;
 
-	err = iommu_attach_device(mapping->domain, dev);
+	err = __arm_iommu_attach_device(dev, mapping);
 	if (err)
 		return err;
 
-	kref_get(&mapping->kref);
-	dev->archdata.mapping = mapping;
-
-	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
+	set_dma_ops(dev, &iommu_ops);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
 
-/**
- * arm_iommu_detach_device
- * @dev: valid struct device pointer
- *
- * Detaches the provided device from a previously attached map.
- */
-void arm_iommu_detach_device(struct device *dev)
+static void __arm_iommu_detach_device(struct device *dev)
 {
 	struct dma_iommu_mapping *mapping;
 
@@ -1989,6 +1999,19 @@ void arm_iommu_detach_device(struct device *dev)
 
 	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
 }
+
+/**
+ * arm_iommu_detach_device
+ * @dev: valid struct device pointer
+ *
+ * Detaches the provided device from a previously attached map.
+ * This voids the dma operations (dma_map_ops pointer)
+ */
+void arm_iommu_detach_device(struct device *dev)
+{
+	__arm_iommu_detach_device(dev);
+	set_dma_ops(dev, NULL);
+}
 EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
 
 static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
@@ -2011,7 +2034,7 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		return false;
 	}
 
-	if (arm_iommu_attach_device(dev, mapping)) {
+	if (__arm_iommu_attach_device(dev, mapping)) {
 		pr_warn("Failed to attached device %s to IOMMU_mapping\n",
 				dev_name(dev));
 		arm_iommu_release_mapping(mapping);
@@ -2025,7 +2048,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev)
 {
 	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
 
-	arm_iommu_detach_device(dev);
+	__arm_iommu_detach_device(dev);
 	arm_iommu_release_mapping(mapping);
 }
 
-- 
Regards,

Laurent Pinchart

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 14:21 ` Laurent Pinchart
@ 2015-01-23 15:27     ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2015-01-23 15:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: jroedel-l3A5Bk7waGM, Heiko Stuebner, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Thierry Reding, Alexandre Courbot,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> +/**
> + * arm_iommu_detach_device
> + * @dev: valid struct device pointer
> + *
> + * Detaches the provided device from a previously attached map.
> + * This voids the dma operations (dma_map_ops pointer)
> + */
> +void arm_iommu_detach_device(struct device *dev)
> +{
> +       __arm_iommu_detach_device(dev);
> +       set_dma_ops(dev, NULL);
> +}
>  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
>  
> 

Would this introduce a regression in the case where the device is
cache coherent and needs arm_coherent_dma_ops set?

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 15:27     ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2015-01-23 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> +/**
> + * arm_iommu_detach_device
> + * @dev: valid struct device pointer
> + *
> + * Detaches the provided device from a previously attached map.
> + * This voids the dma operations (dma_map_ops pointer)
> + */
> +void arm_iommu_detach_device(struct device *dev)
> +{
> +       __arm_iommu_detach_device(dev);
> +       set_dma_ops(dev, NULL);
> +}
>  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
>  
> 

Would this introduce a regression in the case where the device is
cache coherent and needs arm_coherent_dma_ops set?

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 15:27     ` Arnd Bergmann
@ 2015-01-23 15:55       ` Laurent Pinchart
  -1 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2015-01-23 15:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: jroedel-l3A5Bk7waGM, Heiko Stuebner, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Thierry Reding,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, Alexandre Courbot,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Arnd,

On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > +/**
> > + * arm_iommu_detach_device
> > + * @dev: valid struct device pointer
> > + *
> > + * Detaches the provided device from a previously attached map.
> > + * This voids the dma operations (dma_map_ops pointer)
> > + */
> > +void arm_iommu_detach_device(struct device *dev)
> > +{
> > +       __arm_iommu_detach_device(dev);
> > +       set_dma_ops(dev, NULL);
> > +}
> > 
> >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> 
> Would this introduce a regression in the case where the device is
> cache coherent and needs arm_coherent_dma_ops set?

I think we need to handle that case (as well as the coherent IOMMU case in 
arm_iommu_attach_device), but this patch only tries to restore the previous 
behaviour to fix the bug detailed in the commit message. Would you prefer 
fixing both issues in one go ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 15:55       ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2015-01-23 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd,

On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > +/**
> > + * arm_iommu_detach_device
> > + * @dev: valid struct device pointer
> > + *
> > + * Detaches the provided device from a previously attached map.
> > + * This voids the dma operations (dma_map_ops pointer)
> > + */
> > +void arm_iommu_detach_device(struct device *dev)
> > +{
> > +       __arm_iommu_detach_device(dev);
> > +       set_dma_ops(dev, NULL);
> > +}
> > 
> >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> 
> Would this introduce a regression in the case where the device is
> cache coherent and needs arm_coherent_dma_ops set?

I think we need to handle that case (as well as the coherent IOMMU case in 
arm_iommu_attach_device), but this patch only tries to restore the previous 
behaviour to fix the bug detailed in the commit message. Would you prefer 
fixing both issues in one go ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 15:55       ` Laurent Pinchart
@ 2015-01-23 17:00         ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2015-01-23 17:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: jroedel-l3A5Bk7waGM, Heiko Stuebner, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Thierry Reding,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, Alexandre Courbot,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
> 
> On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> > On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > > +/**
> > > + * arm_iommu_detach_device
> > > + * @dev: valid struct device pointer
> > > + *
> > > + * Detaches the provided device from a previously attached map.
> > > + * This voids the dma operations (dma_map_ops pointer)
> > > + */
> > > +void arm_iommu_detach_device(struct device *dev)
> > > +{
> > > +       __arm_iommu_detach_device(dev);
> > > +       set_dma_ops(dev, NULL);
> > > +}
> > > 
> > >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> > 
> > Would this introduce a regression in the case where the device is
> > cache coherent and needs arm_coherent_dma_ops set?
> 
> I think we need to handle that case (as well as the coherent IOMMU case in 
> arm_iommu_attach_device), but this patch only tries to restore the previous 
> behaviour to fix the bug detailed in the commit message. Would you prefer 
> fixing both issues in one go ?
> 

No, I was specifically trying to find out whether the new behavior would
be worse than what we had in 3.18. If it's a preexisting bug that nobody
has run into, there is no hurry.

It's quite likely that to date, all IOMMU users on ARM32 are not
cache coherent.

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 17:00         ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2015-01-23 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
> 
> On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> > On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > > +/**
> > > + * arm_iommu_detach_device
> > > + * @dev: valid struct device pointer
> > > + *
> > > + * Detaches the provided device from a previously attached map.
> > > + * This voids the dma operations (dma_map_ops pointer)
> > > + */
> > > +void arm_iommu_detach_device(struct device *dev)
> > > +{
> > > +       __arm_iommu_detach_device(dev);
> > > +       set_dma_ops(dev, NULL);
> > > +}
> > > 
> > >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> > 
> > Would this introduce a regression in the case where the device is
> > cache coherent and needs arm_coherent_dma_ops set?
> 
> I think we need to handle that case (as well as the coherent IOMMU case in 
> arm_iommu_attach_device), but this patch only tries to restore the previous 
> behaviour to fix the bug detailed in the commit message. Would you prefer 
> fixing both issues in one go ?
> 

No, I was specifically trying to find out whether the new behavior would
be worse than what we had in 3.18. If it's a preexisting bug that nobody
has run into, there is no hurry.

It's quite likely that to date, all IOMMU users on ARM32 are not
cache coherent.

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 17:00         ` Arnd Bergmann
@ 2015-01-23 17:04           ` Will Deacon
  -1 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2015-01-23 17:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: jroedel-l3A5Bk7waGM, Heiko Stuebner,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Thierry Reding, Laurent Pinchart,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, Alexandre Courbot,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Jan 23, 2015 at 05:00:45PM +0000, Arnd Bergmann wrote:
> On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
> > 
> > On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> > > On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > > > +/**
> > > > + * arm_iommu_detach_device
> > > > + * @dev: valid struct device pointer
> > > > + *
> > > > + * Detaches the provided device from a previously attached map.
> > > > + * This voids the dma operations (dma_map_ops pointer)
> > > > + */
> > > > +void arm_iommu_detach_device(struct device *dev)
> > > > +{
> > > > +       __arm_iommu_detach_device(dev);
> > > > +       set_dma_ops(dev, NULL);
> > > > +}
> > > > 
> > > >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> > > 
> > > Would this introduce a regression in the case where the device is
> > > cache coherent and needs arm_coherent_dma_ops set?
> > 
> > I think we need to handle that case (as well as the coherent IOMMU case in 
> > arm_iommu_attach_device), but this patch only tries to restore the previous 
> > behaviour to fix the bug detailed in the commit message. Would you prefer 
> > fixing both issues in one go ?
> > 
> 
> No, I was specifically trying to find out whether the new behavior would
> be worse than what we had in 3.18. If it's a preexisting bug that nobody
> has run into, there is no hurry.

Agreed, I think Laurent's proposal here is the smallest thing that fixes
the issue without breaking the new of_xlate-based configuration.

  Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

> It's quite likely that to date, all IOMMU users on ARM32 are not
> cache coherent.

Highbank has an ARM SMMU, but I don't see the iommu_coherent_ops getting
used, so your assertion is probably right.

Will

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-23 17:04           ` Will Deacon
  0 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2015-01-23 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 23, 2015 at 05:00:45PM +0000, Arnd Bergmann wrote:
> On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
> > 
> > On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
> > > On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
> > > > +/**
> > > > + * arm_iommu_detach_device
> > > > + * @dev: valid struct device pointer
> > > > + *
> > > > + * Detaches the provided device from a previously attached map.
> > > > + * This voids the dma operations (dma_map_ops pointer)
> > > > + */
> > > > +void arm_iommu_detach_device(struct device *dev)
> > > > +{
> > > > +       __arm_iommu_detach_device(dev);
> > > > +       set_dma_ops(dev, NULL);
> > > > +}
> > > > 
> > > >  EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
> > > 
> > > Would this introduce a regression in the case where the device is
> > > cache coherent and needs arm_coherent_dma_ops set?
> > 
> > I think we need to handle that case (as well as the coherent IOMMU case in 
> > arm_iommu_attach_device), but this patch only tries to restore the previous 
> > behaviour to fix the bug detailed in the commit message. Would you prefer 
> > fixing both issues in one go ?
> > 
> 
> No, I was specifically trying to find out whether the new behavior would
> be worse than what we had in 3.18. If it's a preexisting bug that nobody
> has run into, there is no hurry.

Agreed, I think Laurent's proposal here is the smallest thing that fixes
the issue without breaking the new of_xlate-based configuration.

  Acked-by: Will Deacon <will.deacon@arm.com>

> It's quite likely that to date, all IOMMU users on ARM32 are not
> cache coherent.

Highbank has an ARM SMMU, but I don't see the iommu_coherent_ops getting
used, so your assertion is probably right.

Will

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 14:21 ` Laurent Pinchart
@ 2015-01-26  0:34     ` Heiko Stübner
  -1 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2015-01-26  0:34 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: jroedel-l3A5Bk7waGM, arnd-r2nGTMty4D4, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Thierry Reding, Alexandre Courbot,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

on a rk3288 board with the currently failing drm driver this patch brought it 
back to display something :-) , so

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


Thanks
Heiko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-26  0:34     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2015-01-26  0:34 UTC (permalink / raw)
  To: linux-arm-kernel

Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

on a rk3288 board with the currently failing drm driver this patch brought it 
back to display something :-) , so

Tested-by: Heiko Stuebner <heiko@sntech.de>


Thanks
Heiko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 14:21 ` Laurent Pinchart
@ 2015-01-26 10:42     ` Joerg Roedel
  -1 siblings, 0 replies; 18+ messages in thread
From: Joerg Roedel @ 2015-01-26 10:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Heiko Stuebner, arnd-r2nGTMty4D4, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Thierry Reding, Alexandre Courbot,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Jan 23, 2015 at 04:21:49PM +0200, Laurent Pinchart wrote:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
>  arch/arm/mm/dma-mapping.c | 53 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 15 deletions(-)

I guess this patch will be merged by one of the ARM trees?


	Joerg

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-26 10:42     ` Joerg Roedel
  0 siblings, 0 replies; 18+ messages in thread
From: Joerg Roedel @ 2015-01-26 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 23, 2015 at 04:21:49PM +0200, Laurent Pinchart wrote:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  arch/arm/mm/dma-mapping.c | 53 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 15 deletions(-)

I guess this patch will be merged by one of the ARM trees?


	Joerg

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-23 14:21 ` Laurent Pinchart
@ 2015-01-28  0:25     ` Heiko Stübner
  -1 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2015-01-28  0:25 UTC (permalink / raw)
  To: arm-DgEjT+Ai2ygdnm+yROfE0A
  Cc: jroedel-l3A5Bk7waGM, arnd-r2nGTMty4D4, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Thierry Reding,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, Alexandre Courbot,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Arnd, Olof,

Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

in the original submission arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org was not included, but it looks like 
the patch should go through arm-soc.

We have two tags
	Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
	Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

Can you find the original patch somehow or should it be resend to include 
arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org ?


Thanks
Heiko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-28  0:25     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2015-01-28  0:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd, Olof,

Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> into arch_setup_dma_ops") moved the setting of the DMA operations from
> arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> operations to be used are selected based on whether the device is
> connected to an IOMMU. However, the IOMMU detection scheme requires the
> IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> has been ported yet, this effectively breaks all IOMMU ARM users that
> depend on the IOMMU being handled transparently by the DMA mapping API.
> 
> Fix this by restoring the setting of DMA IOMMU ops in
> arm_iommu_attach_device() and splitting the rest of the function into a
> new internal __arm_iommu_attach_device() function, called by
> arch_setup_dma_ops().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

in the original submission arm at kernel.org was not included, but it looks like 
the patch should go through arm-soc.

We have two tags
	Acked-by: Will Deacon <will.deacon@arm.com>
	Tested-by: Heiko Stuebner <heiko@sntech.de>

Can you find the original patch somehow or should it be resend to include 
arm at kernel.org ?


Thanks
Heiko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
  2015-01-28  0:25     ` Heiko Stübner
@ 2015-01-29 18:57       ` Olof Johansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Olof Johansson @ 2015-01-29 18:57 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: jroedel-l3A5Bk7waGM, arnd-r2nGTMty4D4, Will Deacon,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	arm-DgEjT+Ai2ygdnm+yROfE0A, Thierry Reding,
	Varun.Sethi-KZfg59tc24xl57MIdRCFDg, Alexandre Courbot,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, Laurent Pinchart,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Jan 28, 2015 at 01:25:35AM +0100, Heiko Stübner wrote:
> Hi Arnd, Olof,
> 
> Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> > Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> > into arch_setup_dma_ops") moved the setting of the DMA operations from
> > arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> > operations to be used are selected based on whether the device is
> > connected to an IOMMU. However, the IOMMU detection scheme requires the
> > IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> > has been ported yet, this effectively breaks all IOMMU ARM users that
> > depend on the IOMMU being handled transparently by the DMA mapping API.
> > 
> > Fix this by restoring the setting of DMA IOMMU ops in
> > arm_iommu_attach_device() and splitting the rest of the function into a
> > new internal __arm_iommu_attach_device() function, called by
> > arch_setup_dma_ops().
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> in the original submission arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org was not included, but it looks like 
> the patch should go through arm-soc.
> 
> We have two tags
> 	Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> 	Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> 
> Can you find the original patch somehow or should it be resend to include 
> arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org ?

The patch was posted on a list that I am subscribed to, so it popped up in
the same thread this time. I've applied it to fixes for 3.19.

Thanks,

-Olof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
@ 2015-01-29 18:57       ` Olof Johansson
  0 siblings, 0 replies; 18+ messages in thread
From: Olof Johansson @ 2015-01-29 18:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 28, 2015 at 01:25:35AM +0100, Heiko St?bner wrote:
> Hi Arnd, Olof,
> 
> Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
> > Commit 4bb25789ed28228a ("arm: dma-mapping: plumb our iommu mapping ops
> > into arch_setup_dma_ops") moved the setting of the DMA operations from
> > arm_iommu_attach_device() to arch_setup_dma_ops() where the DMA
> > operations to be used are selected based on whether the device is
> > connected to an IOMMU. However, the IOMMU detection scheme requires the
> > IOMMU driver to be ported to the new IOMMU of_xlate API. As no driver
> > has been ported yet, this effectively breaks all IOMMU ARM users that
> > depend on the IOMMU being handled transparently by the DMA mapping API.
> > 
> > Fix this by restoring the setting of DMA IOMMU ops in
> > arm_iommu_attach_device() and splitting the rest of the function into a
> > new internal __arm_iommu_attach_device() function, called by
> > arch_setup_dma_ops().
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> in the original submission arm at kernel.org was not included, but it looks like 
> the patch should go through arm-soc.
> 
> We have two tags
> 	Acked-by: Will Deacon <will.deacon@arm.com>
> 	Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> Can you find the original patch somehow or should it be resend to include 
> arm at kernel.org ?

The patch was posted on a list that I am subscribed to, so it popped up in
the same thread this time. I've applied it to fixes for 3.19.

Thanks,

-Olof

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-01-29 18:57 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 14:21 [PATCH URGENT] arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device() Laurent Pinchart
2015-01-23 14:21 ` Laurent Pinchart
     [not found] ` <1422022909-31044-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2015-01-23 15:27   ` Arnd Bergmann
2015-01-23 15:27     ` Arnd Bergmann
2015-01-23 15:55     ` Laurent Pinchart
2015-01-23 15:55       ` Laurent Pinchart
2015-01-23 17:00       ` Arnd Bergmann
2015-01-23 17:00         ` Arnd Bergmann
2015-01-23 17:04         ` Will Deacon
2015-01-23 17:04           ` Will Deacon
2015-01-26  0:34   ` Heiko Stübner
2015-01-26  0:34     ` Heiko Stübner
2015-01-26 10:42   ` Joerg Roedel
2015-01-26 10:42     ` Joerg Roedel
2015-01-28  0:25   ` Heiko Stübner
2015-01-28  0:25     ` Heiko Stübner
2015-01-29 18:57     ` Olof Johansson
2015-01-29 18:57       ` Olof Johansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.