arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
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().
--- 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.
arch/arm/mm/dma-mapping.c +38 -15
@@ -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); }
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
Quoted 5 lines · On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
arch/arm/mm/dma-mapping.c +3
-0+/** +* arm_iommu_detach_device +* @dev: valid struct device pointer
arch/arm/mm/dma-mapping.c void arm_iommu_detach_device(struct device *dev) · line 2015
Quoted 10 lines
arch/arm/mm/dma-mapping.c +9
-0+* +* 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
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
Hi Arnd,
Quoted 14 lines · On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
Quoted 13 lines · On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
arch/arm/mm/dma-mapping.c +12
-0+/** +* 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 ); +}
Quoted 4 lines
Quoted 1 line
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_opsset?
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 ?
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
Quoted 21 lines · On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
Quoted 14 lines · On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
Quoted 13 lines · On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
arch/arm/mm/dma-mapping.c +12
-0+/** +* 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 ); +}Quoted 4 lines
Quoted 1 line
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_opsset?
Quoted 5 lines
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
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
Quoted 28 lines · Arnd Bergmann
Quoted 21 lines · On Friday 23 January 2015 17:55:25 Laurent Pinchart wrote:
Quoted 14 lines · On Friday 23 January 2015 16:27:24 Arnd Bergmann wrote:
Quoted 13 lines · On Friday 23 January 2015 16:21:49 Laurent Pinchart wrote:
arch/arm/mm/dma-mapping.c +12
-0+/** +* 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 ); +}Quoted 4 lines
Quoted 1 line
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_opsset?Quoted 5 lines
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 ?
Quoted 3 lines
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.
Quoted 2 lines
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
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
In reply to Laurent PinchartAm Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
Quoted 9 lines
Commit
4bb25789ed28228a("arm: dma-mapping: plumb our iommu mapping ops intoarch_setup_dma_ops") moved the setting of the DMA operations fromarm_iommu_attach_device()toarch_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 IOMMUof_xlateAPI. 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.
Quoted 6 lines
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 byarch_setup_dma_ops().Signed-off-by Laurent Pinchart <[email protected]>
on a rk3288 board with the currently failing drm driver this patch brought it back to display something :-) , so
Thanks
Heiko
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
In reply to Laurent PinchartQuoted 14 lines · Laurent Pinchart
Commit
4bb25789ed28228a("arm: dma-mapping: plumb our iommu mapping ops intoarch_setup_dma_ops") moved the setting of the DMA operations fromarm_iommu_attach_device()toarch_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 IOMMUof_xlateAPI. 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 byarch_setup_dma_ops().
Quoted 4 lines
Signed-off-by Laurent Pinchart <[email protected]>--- 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
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
In reply to Laurent PinchartHi Arnd, Olof,
Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
Quoted 9 lines
Commit
4bb25789ed28228a("arm: dma-mapping: plumb our iommu mapping ops intoarch_setup_dma_ops") moved the setting of the DMA operations fromarm_iommu_attach_device()toarch_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 IOMMUof_xlateAPI. 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.
Quoted 6 lines
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 byarch_setup_dma_ops().Signed-off-by Laurent Pinchart <[email protected]>
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
Can you find the original patch somehow or should it be resend to include arm at kernel.org ?
Thanks
Heiko
PATCH 1/1 arm: dma-mapping: Set DMA IOMMU ops in arm_iommu_attach_device()
Quoted 22 lines · Heiko St?bner
Hi Arnd, Olof,
Am Freitag, 23. Januar 2015, 16:21:49 schrieb Laurent Pinchart:
Quoted 9 lines
Commit
4bb25789ed28228a("arm: dma-mapping: plumb our iommu mapping ops intoarch_setup_dma_ops") moved the setting of the DMA operations fromarm_iommu_attach_device()toarch_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 IOMMUof_xlateAPI. 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.Quoted 6 lines
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 byarch_setup_dma_ops().Signed-off-by Laurent Pinchart <[email protected]>in the original submission arm at kernel.org was not included, but it looks like the patch should go through arm-soc.
Quoted 6 lines
We have two tags
Acked-by Will Deacon <[email protected]>Tested-by Heiko Stuebner <[email protected]>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