From 4b1c88984c8ac894c2c411570757bed7fa5f3226 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 18 May 2017 15:13:57 +0200 Subject: [PATCH 1/7] iommu/dma: Fix function declaration Newly added code in the ipmmu-vmsa driver showed a small mistake in a header file that can't be included by itself without CONFIG_IOMMU_DMA enabled: In file included from drivers/iommu/ipmmu-vmsa.c:13:0: include/linux/dma-iommu.h:105:94: error: 'struct device' declared inside parameter list will not be visible outside of this definition or declaration [-Werror] This adds a forward declaration for 'struct device', similar to how we treat the other struct types in this case. Fixes: 3ae47292024f ("iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops") Fixes: 273df9635385 ("iommu/dma: Make PCI window reservation generic") Signed-off-by: Arnd Bergmann Acked-by: Robin Murphy Signed-off-by: Joerg Roedel --- include/linux/dma-iommu.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h index 4eac2670bfa1..92f20832fd28 100644 --- a/include/linux/dma-iommu.h +++ b/include/linux/dma-iommu.h @@ -78,6 +78,7 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list); struct iommu_domain; struct msi_msg; +struct device; static inline int iommu_dma_init(void) { From 573d41757c47cb71cd572236685d4c9e6cacc06c Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Sat, 27 May 2017 19:17:40 +0530 Subject: [PATCH 2/7] iommu/of: Fix check for returning EPROBE_DEFER Now with IOMMU probe deferral, we return -EPROBE_DEFER for masters that are connected to an IOMMU which is not probed yet, but going to get probed, so that we can attach the correct dma_ops. So while trying to defer the probe of the master, check if the of_iommu node that it is connected to is marked in DT as 'status=disabled', then the IOMMU is never is going to get probed. So simply return NULL and let the master work without an IOMMU. Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred probing or error") Reported-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Tested-by: Will Deacon Tested-by: Magnus Damn Acked-by: Will Deacon Signed-off-by: Sricharan R Signed-off-by: Joerg Roedel --- drivers/iommu/of_iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 9f44ee8ea1bc..e6e9bec42e5f 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -118,6 +118,7 @@ static const struct iommu_ops ops = iommu_ops_from_fwnode(fwnode); if ((ops && !ops->of_xlate) || + !of_device_is_available(iommu_spec->np) || (!ops && !of_iommu_driver_present(iommu_spec->np))) return NULL; From a37b19a384914c60b7e1264a6c21e7bf96b637e8 Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Sat, 27 May 2017 19:17:41 +0530 Subject: [PATCH 3/7] iommu/of: Ignore all errors except EPROBE_DEFER While deferring the probe of IOMMU masters, xlate and add_device callbacks called from of_iommu_configure can pass back error values like -ENODEV, which means the IOMMU cannot be connected with that master for real reasons. Before the IOMMU probe deferral, all such errors were ignored. Now all those errors are propagated back, killing the master's probe for such errors. Instead ignore all the errors except EPROBE_DEFER, which is the only one of concern and let the master work without IOMMU, thus restoring the old behavior. Also make explicit that of_dma_configure handles only -EPROBE_DEFER from of_iommu_configure. Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred probing or error") Reported-by: Geert Uytterhoeven Tested-by: Magnus Damn Signed-off-by: Sricharan R Signed-off-by: Joerg Roedel --- drivers/iommu/of_iommu.c | 6 ++++++ drivers/of/device.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index e6e9bec42e5f..19779b88a479 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -237,6 +237,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, ops = ERR_PTR(err); } + /* Ignore all other errors apart from EPROBE_DEFER */ + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); + ops = NULL; + } + return ops; } diff --git a/drivers/of/device.c b/drivers/of/device.c index 9416d052cb89..28c38c756f92 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -144,8 +144,8 @@ int of_dma_configure(struct device *dev, struct device_node *np) coherent ? " " : " not "); iommu = of_iommu_configure(dev, np); - if (IS_ERR(iommu)) - return PTR_ERR(iommu); + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) + return -EPROBE_DEFER; dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); From 058f8c3fb44d028ba44c2e3635d8bee9787018d6 Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Sat, 27 May 2017 19:17:42 +0530 Subject: [PATCH 4/7] ACPI/IORT: Ignore all errors except EPROBE_DEFER While deferring the probe of IOMMU masters, xlate and add_device callbacks called from iort_iommu_configure can pass back error values like -ENODEV, which means the IOMMU cannot be connected with that master for real reasons. Before the IOMMU probe deferral, all such errors were ignored. Now all those errors are propagated back, killing the master's probe for such errors. Instead ignore all the errors except EPROBE_DEFER, which is the only one of concern and let the master work without IOMMU, thus restoring the old behavior. Also make explicit that acpi_dma_configure handles only -EPROBE_DEFER from iort_iommu_configure. Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") Signed-off-by: Sricharan R Signed-off-by: Joerg Roedel --- drivers/acpi/arm64/iort.c | 6 ++++++ drivers/acpi/scan.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index c5fecf97ee2f..16e101f1f69d 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) if (err) ops = ERR_PTR(err); + /* Ignore all other errors apart from EPROBE_DEFER */ + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); + ops = NULL; + } + return ops; } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index e39ec7b7cb67..3a10d7573477 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1371,8 +1371,8 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) iort_set_dma_mask(dev); iommu = iort_iommu_configure(dev); - if (IS_ERR(iommu)) - return PTR_ERR(iommu); + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) + return -EPROBE_DEFER; size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1); /* From a93a121a96531b99d6feb8db96417b49e1bb28e8 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 27 May 2017 19:17:43 +0530 Subject: [PATCH 5/7] ARM: dma-mapping: Don't tear down third-party mappings arch_setup_dma_ops() is used in device probe code paths to create an IOMMU mapping and attach it to the device. The function assumes that the device is attached to a device-specific IOMMU instance (or at least a device-specific TLB in a shared IOMMU instance) and thus creates a separate mapping for every device. On several systems (Renesas R-Car Gen2 being one of them), that assumption is not true, and IOMMU mappings must be shared between multiple devices. In those cases the IOMMU driver knows better than the generic ARM dma-mapping layer and attaches mapping to devices manually with arm_iommu_attach_device(), which sets the DMA ops for the device. The arch_setup_dma_ops() function takes this into account and bails out immediately if the device already has DMA ops assigned. However, the corresponding arch_teardown_dma_ops() function, called from driver unbind code paths (including probe deferral), will tear the mapping down regardless of who created it. When the device is reprobed arch_setup_dma_ops() will be called again but won't perform any operation as the DMA ops will still be set. We need to reset the DMA ops in arch_teardown_dma_ops() to fix this. However, we can't do so unconditionally, as then a new mapping would be created by arch_setup_dma_ops() when the device is reprobed, regardless of whether the device needs to share a mapping or not. We must thus keep track of whether arch_setup_dma_ops() created the mapping, and only in that case tear it down in arch_teardown_dma_ops(). Keep track of that information in the dev_archdata structure. As the structure is embedded in all instances of struct device let's not grow it, but turn the existing dma_coherent bool field into a bitfield that can be used for other purposes. Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") Reviewed-by: Robin Murphy Signed-off-by: Laurent Pinchart Signed-off-by: Joerg Roedel --- arch/arm/include/asm/device.h | 3 ++- arch/arm/mm/dma-mapping.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h index 36ec9c8f6e16..3234fe9bba6e 100644 --- a/arch/arm/include/asm/device.h +++ b/arch/arm/include/asm/device.h @@ -19,7 +19,8 @@ struct dev_archdata { #ifdef CONFIG_XEN const struct dma_map_ops *dev_dma_ops; #endif - bool dma_coherent; + unsigned int dma_coherent:1; + unsigned int dma_ops_setup:1; }; struct omap_device; diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index c742dfd2967b..b48998f90bfb 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2430,9 +2430,13 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, dev->dma_ops = xen_dma_ops; } #endif + dev->archdata.dma_ops_setup = true; } void arch_teardown_dma_ops(struct device *dev) { + if (!dev->archdata.dma_ops_setup) + return; + arm_teardown_iommu_dma_ops(dev); } From 4dac3210c54a04be9c187dd31b4debb46e3d792b Mon Sep 17 00:00:00 2001 From: Lorenzo Pieralisi Date: Sat, 27 May 2017 19:17:44 +0530 Subject: [PATCH 6/7] ACPI/IORT: Move the check to get iommu_ops from translated fwspec With IOMMU probe deferral, iort_iommu_configure can be called multiple times for the same device. Hence we have a check to see if the device's fwspec is already translated and return the iommu_ops from that directly. But the check is wrongly placed in iort_iommu_xlate, which breaks devices with multiple sids. Move the check to iort_iommu_configure. Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") Tested-by: Nate Watterson Signed-off-by: Lorenzo Pieralisi Signed-off-by: Joerg Roedel --- drivers/acpi/arm64/iort.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 16e101f1f69d..797b28dc7b34 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -666,14 +666,6 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev, int ret = -ENODEV; struct fwnode_handle *iort_fwnode; - /* - * If we already translated the fwspec there - * is nothing left to do, return the iommu_ops. - */ - ops = iort_fwspec_iommu_ops(dev->iommu_fwspec); - if (ops) - return ops; - if (node) { iort_fwnode = iort_get_fwnode(node); if (!iort_fwnode) @@ -735,6 +727,14 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) u32 streamid = 0; int err; + /* + * If we already translated the fwspec there + * is nothing left to do, return the iommu_ops. + */ + ops = iort_fwspec_iommu_ops(dev->iommu_fwspec); + if (ops) + return ops; + if (dev_is_pci(dev)) { struct pci_bus *bus = to_pci_dev(dev)->bus; u32 rid; From d3e01c51598b2639a4830549058500e5f2ace86f Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Sat, 27 May 2017 19:17:45 +0530 Subject: [PATCH 7/7] arm: dma-mapping: Reset the device's dma_ops arch_teardown_dma_ops() being the inverse of arch_setup_dma_ops() ,dma_ops should be cleared in the teardown path. Currently, only the device's iommu mapping structures are cleared in arch_teardown_dma_ops, but not the dma_ops. So on the next reprobe, dma_ops left in place is stale from the first IOMMU setup, but iommu mappings has been disposed of. This is a problem when the probe of the device is deferred and recalled with the IOMMU probe deferral. So for fixing this, slightly refactor by moving the code from __arm_iommu_detach_device to arm_iommu_detach_device and cleanup the former. This takes care of resetting the dma_ops in the teardown path. Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") Reviewed-by: Laurent Pinchart Signed-off-by: Sricharan R Signed-off-by: Joerg Roedel --- arch/arm/mm/dma-mapping.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index b48998f90bfb..bd83c531828a 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2311,7 +2311,14 @@ int arm_iommu_attach_device(struct device *dev, } EXPORT_SYMBOL_GPL(arm_iommu_attach_device); -static void __arm_iommu_detach_device(struct device *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) { struct dma_iommu_mapping *mapping; @@ -2324,22 +2331,10 @@ static void __arm_iommu_detach_device(struct device *dev) iommu_detach_device(mapping->domain, dev); kref_put(&mapping->kref, release_iommu_mapping); to_dma_iommu_mapping(dev) = NULL; + set_dma_ops(dev, NULL); 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 const struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent) @@ -2379,7 +2374,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev) if (!mapping) return; - __arm_iommu_detach_device(dev); + arm_iommu_detach_device(dev); arm_iommu_release_mapping(mapping); }