Hello,
This patch series attempts to implement support for deferring probe of both
IOMMU drivers and bus master drivers.
The relationship between bus masters and IOMMUs creates a strong ordering
during initialization of devices. As in the general case IOMMUs are hidden
behind the DMA mapping API, IOMMU support relies on the automatic setup of DMA
operations without any direct intervention of bus master drivers.
DMA operations are set up when platform devices are added to the system. This
requires IOMMUs to be available at that time. On systems where ordering of
device add and probe can't be guaranteed (such as, but not limited to,
DT-based systems) this caused incorrect DMA operation setup. This has been
addressed by a patches series [1] that introduced a DT-based early
registration mechanism for IOMMUs.
However, that mechanism fails to address all issues. Various dependencies
exist between IOMMU devices and other devices, in particular on clocks and on
power domains (as mentioned by Marek in [2]). While there are mechanisms to
handle some of them without probe deferral (for instance by using the
OF_DECLARE macros to register clock drivers), generalizing those mechanisms
would essentially recreate a probe ordering mechanism similar to link order
probe ordering and couldn't really scale.
Additionally, IOMMUs could also be present hot-pluggable devices and depend on
resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case.
For all those reasons probe deferral for IOMMUs has been considered as desired
if it can be implemented cleanly. For more in-depth information see [3].
This RFC series is a first attempt at implementing IOMMU probe deferral
support cleanly.
The core idea is to move setup of DMA operations from device add time to
device probe time, implemented in patch 6/9. It could be possible to move
setup of other DMA parameters (namely masks and offset) to probe time as well,
but that change would be more intrusive and has a higher risk of introducing
regressions. For that reason I've decided to keep DMA masks and offset setup
at device add time and thus split DMA configuration in masks and operations
(patch 5/9). This can be revisited if we decide that the DMA mapping API
shouldn't require masks and offset to be set before probe time.
Patch 8/9 then defers probe of bus master drivers when required IOMMUs are not
available yet. This requires knowing when a failed IOMMU lookup should be
considered as permanent or temporary. I've reused the OF_DECLARE_IOMMU for
this purpose, considering that the presence of a driver compatible with the
IOMMU DT node indicates that the failure is temporary and probing of the bus
master device should be deferred.
Note that only IOMMU drivers using the recent .of_xlate() mechanism for
DT-based IOMMU reference can cause probe deferral of bus master devices. The
.add_device() mechanism isn't supported in this case.
As an example I've converted the ipmmu-vmsa driver to the new API in patch 9/9.
At this point many enhancements are possible, but I'd like to receive feedback
on the proposed approach before basing more patches on this series. One
particular point I would like to address (or see being addressed) in the
future is the use of struct iommu_ops with of_iommu_get_ops() and
of_iommu_set_ops(). I believe we should introduct a struct iommu and register
IOMMU instances instead of IOMMU operations. That should bring us one step
closer to removing bus_set_iommu().
[1] http://www.spinics.net/lists/arm-kernel/msg382787.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html
[3] https://lkml.org/lkml/2015/2/16/345
Laurent Pinchart (9):
arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits
range
of: dma: Move range size workaround to of_dma_get_range()
of: dma: Make of_dma_deconfigure() public
of: dma: Split of_configure_dma() into mask and ops configuration
drivers: platform: Configure dma operations at probe time
iommu: of: Document the of_iommu_configure() function
iommu: of: Handle IOMMU lookup failure with deferred probing or error
iommu/ipmmu-vmsa: Use DT-based instantiation
arch/arm/include/asm/dma-iommu.h | 2 +-
arch/arm/mm/dma-mapping.c | 21 +++--
drivers/base/platform.c | 9 ++
drivers/iommu/ipmmu-vmsa.c | 189 +++++++++++++--------------------------
drivers/iommu/of_iommu.c | 29 +++++-
drivers/of/address.c | 20 ++++-
drivers/of/device.c | 77 ++++++++++------
drivers/of/of_pci.c | 3 +-
drivers/of/platform.c | 16 ++--
include/linux/of_device.h | 14 ++-
10 files changed, 195 insertions(+), 185 deletions(-)
--
Regards,
Laurent Pinchart
The arch_setup_dma_ops() function is in charge of setting dma_ops with a
call to set_dma_ops(). set_dma_ops() is also called from
- highbank and mvebu bus notifiers
- dmabounce (to be replaced with swiotlb)
- arm_iommu_attach_device
(arm_iommu_attach_device is itself called from IOMMU and bus master
device drivers)
To allow the arch_setup_dma_ops() call to be moved from device add time
to device probe time we must ensure that dma_ops already setup by any of
the above callers will not be overriden.
Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to
of_xlate and taking care of highbank and mvebu, the workaround should be
removed.
Signed-off-by: Laurent Pinchart <[email protected]>
---
arch/arm/mm/dma-mapping.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 09c5fe3d30c2..7aa5e339a596 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2117,6 +2117,15 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
struct dma_map_ops *dma_ops;
dev->archdata.dma_coherent = coherent;
+
+ /*
+ * Don't override the dma_ops if they have already been set. Ideally
+ * this should be the only location where dma_ops are set, remove this
+ * check when all other callers of set_dma_ops will have disappeared.
+ */
+ if (dev->archdata.dma_ops)
+ return;
+
if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu))
dma_ops = arm_get_iommu_dma_map_ops(coherent);
else
--
2.3.6
The arm_iommu_create_mapping() function takes the IOMMU mapping size as
a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
bus masters would support the full 32 bits range. Fix this by passing
the size as a 64 bits integer, and limiting it to 4GB for now.
Signed-off-by: Laurent Pinchart <[email protected]>
---
arch/arm/include/asm/dma-iommu.h | 2 +-
arch/arm/mm/dma-mapping.c | 12 +++---------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 8e3fcb924db6..2ef282f96651 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -25,7 +25,7 @@ struct dma_iommu_mapping {
};
struct dma_iommu_mapping *
-arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
+arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size);
void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7aa5e339a596..c69e216a0cf9 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
* arm_iommu_attach_device function.
*/
struct dma_iommu_mapping *
-arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
+arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size)
{
unsigned int bits = size >> PAGE_SHIFT;
unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
@@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
int extensions = 1;
int err = -ENOMEM;
- if (!bitmap_size)
+ /* Limit the size to 4GB for now. */
+ if (!size || size > (1ULL << 32))
return ERR_PTR(-EINVAL);
if (bitmap_size > PAGE_SIZE) {
@@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
if (!iommu)
return false;
- /*
- * currently arm_iommu_create_mapping() takes a max of size_t
- * for size param. So check this limit for now.
- */
- if (size > SIZE_MAX)
- return false;
-
mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
if (IS_ERR(mapping)) {
pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
--
2.3.6
Invalid dma-ranges values should be worked around when retrieving the
DMA range in of_dma_get_range(), not by all callers of the function.
This isn't much of a problem now that we have a single caller, but that
situation will change when moving DMA configuration to device probe
time.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/of/address.c | 20 ++++++++++++++++++--
drivers/of/device.c | 15 ---------------
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 78a7dcbec7d8..75eebd19ebfa 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
* CPU addr (phys_addr_t) : pna cells
* size : nsize cells
*
- * It returns -ENODEV if "dma-ranges" property was not found
- * for this device in DT.
+ * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
+ * this device in DT, or -EINVAL if the CPU address or size is invalid.
*/
int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
{
@@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
*dma_addr = dmaaddr;
*size = of_read_number(ranges + naddr + pna, nsize);
+ /*
+ * DT nodes sometimes incorrectly set the size as a mask. Work around
+ * those incorrect DT by computing the size as mask + 1.
+ */
+ if (*size & 1) {
+ pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
+ __func__, *size, np->full_name);
+ *size = *size + 1;
+ }
+
+ if (!*size) {
+ pr_err("%s: invalid size zero for dma-range in node(%s)\n",
+ __func__, np->full_name);
+ ret = -EINVAL;
+ goto out;
+ }
pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
*dma_addr, *paddr, *size);
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 20c1332a0018..530aa1ed3e1b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
-
- /*
- * Add a work around to treat the size as mask + 1 in case
- * it is defined in DT as a mask.
- */
- if (size & 1) {
- dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
- size);
- size = size + 1;
- }
-
- if (!size) {
- dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
- return;
- }
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
--
2.3.6
As part of moving DMA initializing to probe time the
of_dma_deconfigure() function will need to be called from different
source files. Make it public and move it to drivers/of/device.c where
the of_dma_configure() function is.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/of/device.c | 12 ++++++++++++
drivers/of/platform.c | 5 -----
include/linux/of_device.h | 3 +++
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 530aa1ed3e1b..f1b84f464fe1 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -135,6 +135,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
}
EXPORT_SYMBOL_GPL(of_dma_configure);
+/**
+ * of_dma_deconfigure - Clean up DMA configuration
+ * @dev: Device for which to clean up DMA configuration
+ *
+ * Clean up all configuration performed by of_dma_configure_ops() and free all
+ * resources that have been allocated.
+ */
+void of_dma_deconfigure(struct device *dev)
+{
+ arch_teardown_dma_ops(dev);
+}
+
int of_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index a01f57c9e34e..7a660c79ff84 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -149,11 +149,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
}
EXPORT_SYMBOL(of_device_alloc);
-static void of_dma_deconfigure(struct device *dev)
-{
- arch_teardown_dma_ops(dev);
-}
-
/**
* of_platform_device_create_pdata - Alloc, initialize and register an of_device
* @np: pointer to node to create device for
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 22801b10cef5..6710807b0653 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -54,6 +54,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}
void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */
static inline int of_driver_match_device(struct device *dev,
@@ -93,6 +94,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}
static inline void of_dma_configure(struct device *dev, struct device_node *np)
{}
+static inline void of_dma_deconfigure(struct device *dev)
+{}
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
--
2.3.6
The of_configure_dma() function configures both the DMA masks and ops.
Moving DMA ops configuration to probe time would thus also delay
configuration of the DMA masks, which might not be safe. To avoid issues
split the configuration in two to allow keeping masks configuration at
device add time and move ops configuration to device probe time.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/of/device.c | 48 ++++++++++++++++++++++++++++++++++-------------
drivers/of/of_pci.c | 3 ++-
drivers/of/platform.c | 6 ++++--
include/linux/of_device.h | 11 +++++++++--
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f1b84f464fe1..3cb3f78a6d13 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
}
/**
- * of_dma_configure - Setup DMA configuration
+ * of_dma_configure - Setup DMA masks and offset
* @dev: Device to apply DMA configuration
* @np: Pointer to OF node having DMA configuration
*
@@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+void of_dma_configure_masks(struct device *dev, struct device_node *np)
{
- u64 dma_addr, paddr, size;
- int ret;
- bool coherent;
+ u64 dma_addr, paddr, size, range_mask;
unsigned long offset;
- struct iommu_ops *iommu;
+ int ret;
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
@@ -105,9 +103,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
- dma_addr = offset = 0;
- size = dev->coherent_dma_mask + 1;
+ range_mask = dev->coherent_dma_mask + 1;
+ offset = 0;
} else {
+ range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
@@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct device_node *np)
* Limit coherent and dma mask based on size and default mask
* set by the driver.
*/
- dev->coherent_dma_mask = min(dev->coherent_dma_mask,
- DMA_BIT_MASK(ilog2(dma_addr + size)));
- *dev->dma_mask = min((*dev->dma_mask),
- DMA_BIT_MASK(ilog2(dma_addr + size)));
+ dev->coherent_dma_mask = min(dev->coherent_dma_mask, range_mask);
+ *dev->dma_mask = min((*dev->dma_mask), range_mask);
+}
+EXPORT_SYMBOL_GPL(of_dma_configure_masks);
+
+/**
+ * of_dma_configure_ops - Setup DMA operations
+ * @dev: Device to apply DMA configuration
+ * @np: Pointer to OF node having DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly.
+ */
+int of_dma_configure_ops(struct device *dev, struct device_node *np)
+{
+ u64 dma_addr, paddr, size;
+ struct iommu_ops *iommu;
+ bool coherent;
+ int ret;
+
+ ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
+ if (ret < 0) {
+ dma_addr = 0;
+ size = dev->coherent_dma_mask + 1;
+ }
coherent = of_dma_is_coherent(np);
dev_dbg(dev, "device is%sdma coherent\n",
@@ -132,8 +152,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
iommu ? " " : " not ");
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(of_dma_configure);
+EXPORT_SYMBOL_GPL(of_dma_configure_ops);
/**
* of_dma_deconfigure - Clean up DMA configuration
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5b6494..c65803da2067 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -132,7 +132,8 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
if (!bridge->parent)
return;
- of_dma_configure(dev, bridge->parent->of_node);
+ of_dma_configure_masks(dev, bridge->parent->of_node);
+ of_dma_configure_ops(dev, bridge->parent->of_node);
pci_put_host_bridge_device(bridge);
}
EXPORT_SYMBOL_GPL(of_pci_dma_configure);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7a660c79ff84..9a29f09b7723 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -177,7 +177,8 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
- of_dma_configure(&dev->dev, dev->dev.of_node);
+ of_dma_configure_masks(&dev->dev, dev->dev.of_node);
+ of_dma_configure_ops(&dev->dev, dev->dev.of_node);
if (of_device_add(dev) != 0) {
of_dma_deconfigure(&dev->dev);
@@ -240,7 +241,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev_set_name(&dev->dev, "%s", bus_id);
else
of_device_make_bus_id(&dev->dev);
- of_dma_configure(&dev->dev, dev->dev.of_node);
+ of_dma_configure_masks(&dev->dev, dev->dev.of_node);
+ of_dma_configure_ops(&dev->dev, dev->dev.of_node);
/* Allow the HW Peripheral ID to be overridden */
prop = of_get_property(node, "arm,primecell-periphid", NULL);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 6710807b0653..794f234c7c7d 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -53,7 +53,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}
-void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_configure_masks(struct device *dev, struct device_node *np);
+int of_dma_configure_ops(struct device *dev, struct device_node *np);
void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */
@@ -92,8 +93,14 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
{
return NULL;
}
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
+static inline void of_dma_configure_masks(struct device *dev,
+ struct device_node *np)
{}
+static inline int of_dma_configure_ops(struct device *dev,
+ struct device_node *np)
+{
+ return 0;
+}
static inline void of_dma_deconfigure(struct device *dev)
{}
#endif /* CONFIG_OF */
--
2.3.6
Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/base/platform.c | 9 +++++++++
drivers/of/platform.c | 7 +++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b97278..508a866859dc 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
if (ret < 0)
return ret;
+ ret = of_dma_configure_ops(_dev, _dev->of_node);
+ if (ret < 0)
+ goto done;
+
ret = dev_pm_domain_attach(_dev, true);
if (ret != -EPROBE_DEFER) {
ret = drv->probe(dev);
@@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
dev_pm_domain_detach(_dev, true);
}
+ if (ret)
+ of_dma_deconfigure(_dev);
+
+done:
if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
dev_warn(_dev, "probe deferral not supported\n");
ret = -ENXIO;
@@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
ret = drv->remove(dev);
dev_pm_domain_detach(_dev, true);
+ of_dma_deconfigure(_dev);
return ret;
}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9a29f09b7723..fc939bec799e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -178,10 +178,8 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
of_dma_configure_masks(&dev->dev, dev->dev.of_node);
- of_dma_configure_ops(&dev->dev, dev->dev.of_node);
if (of_device_add(dev) != 0) {
- of_dma_deconfigure(&dev->dev);
platform_device_put(dev);
goto err_clear_flag;
}
@@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device *dev, void *data)
if (dev->bus == &platform_bus_type)
platform_device_unregister(to_platform_device(dev));
#ifdef CONFIG_ARM_AMBA
- else if (dev->bus == &amba_bustype)
+ else if (dev->bus == &amba_bustype) {
amba_device_unregister(to_amba_device(dev));
+ of_dma_deconfigure(dev);
+ }
#endif
- of_dma_deconfigure(dev);
of_node_clear_flag(dev->of_node, OF_POPULATED);
of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
return 0;
--
2.3.6
The function isn't trivial, document its behaviour.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/iommu/of_iommu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 43429ab62228..b922ed4f9fb3 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,6 +133,19 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
return ops;
}
+/**
+ * of_iommu_configure - Configure and return the IOMMU for a device
+ * @dev: device for which to configure the IOMMU
+ * @master_np: device node of the bus master connected to the IOMMU
+ *
+ * The master_np parameter specifies the device node of the bus master seen by
+ * the IOMMU. This is usually the device node of the dev device, but can be the
+ * device node of a bridge when the device is dynamically discovered and
+ * instantiated and thus has no device node (such as PCI devices for instance).
+ *
+ * Return a pointer to the iommu_ops for the device, NULL if the device isn't
+ * connected to an IOMMU, or a negative value if an error occurs.
+ */
struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
--
2.3.6
Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.
The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.
The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.
The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/iommu/of_iommu.c | 16 +++++++++++++---
drivers/of/device.c | 2 ++
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index b922ed4f9fb3..e98f19a3b740 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -170,8 +170,18 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
np = iommu_spec.np;
ops = of_iommu_get_ops(np);
- if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+ if (!ops) {
+ const struct of_device_id *oid;
+
+ oid = of_match_node(&__iommu_of_table, np);
+ ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL;
+ goto err_put_node;
+ }
+
+ if (!ops->of_xlate || ops->of_xlate(dev, &iommu_spec)) {
+ ops = NULL;
goto err_put_node;
+ }
of_node_put(np);
idx++;
@@ -181,7 +191,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
err_put_node:
of_node_put(np);
- return NULL;
+ return ops;
}
void __init of_iommu_init(void)
@@ -192,7 +202,7 @@ void __init of_iommu_init(void)
for_each_matching_node_and_match(np, matches, &match) {
const of_iommu_init_fn init_fn = match->data;
- if (init_fn(np))
+ if (init_fn && init_fn(np))
pr_err("Failed to initialise IOMMU %s\n",
of_node_full_name(np));
}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 3cb3f78a6d13..f4964ab21026 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -148,6 +148,8 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np)
coherent ? " " : " not ");
iommu = of_iommu_configure(dev, np);
+ if (IS_ERR(iommu))
+ return PTR_ERR(iommu);
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");
--
2.3.6
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 189 ++++++++++++++-------------------------------
1 file changed, 60 insertions(+), 129 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 24a950091458..7fa2afb5d7d1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -18,6 +18,8 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/sizes.h>
#include <linux/slab.h>
@@ -30,7 +32,6 @@
struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
- struct list_head list;
unsigned int num_utlbs;
@@ -54,9 +55,6 @@ struct ipmmu_vmsa_archdata {
unsigned int num_utlbs;
};
-static DEFINE_SPINLOCK(ipmmu_devices_lock);
-static LIST_HEAD(ipmmu_devices);
-
static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
{
return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
@@ -578,110 +576,81 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
return domain->iop->iova_to_phys(domain->iop, iova);
}
-static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device *dev,
- unsigned int *utlbs, unsigned int num_utlbs)
+static void ipmmu_remove_device(struct device *dev)
{
- unsigned int i;
-
- for (i = 0; i < num_utlbs; ++i) {
- struct of_phandle_args args;
- int ret;
-
- ret = of_parse_phandle_with_args(dev->of_node, "iommus",
- "#iommu-cells", i, &args);
- if (ret < 0)
- return ret;
+ struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
- of_node_put(args.np);
+ if (!archdata)
+ return;
- if (args.np != mmu->dev->of_node || args.args_count != 1)
- return -EINVAL;
+ arm_iommu_detach_device(dev);
+ iommu_group_remove_device(dev);
- utlbs[i] = args.args[0];
- }
+ kfree(archdata->utlbs);
+ kfree(archdata);
- return 0;
+ dev->archdata.iommu = NULL;
}
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_of_xlate(struct device *dev, struct of_phandle_args *args)
{
struct ipmmu_vmsa_archdata *archdata;
struct ipmmu_vmsa_device *mmu;
- struct iommu_group *group = NULL;
+ struct platform_device *pdev;
+ unsigned int num_utlbs;
unsigned int *utlbs;
- unsigned int i;
- int num_utlbs;
- int ret = -ENODEV;
-
- if (dev->archdata.iommu) {
- dev_warn(dev, "IOMMU driver already assigned to device %s\n",
- dev_name(dev));
- return -EINVAL;
- }
+ unsigned int utlb;
+ int ret;
/* Find the master corresponding to the device. */
-
- num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
- "#iommu-cells");
- if (num_utlbs < 0)
+ pdev = of_find_device_by_node(args->np);
+ if (!pdev) {
+ dev_dbg(dev, "%s: ipmmu pdev not found\n", __func__);
return -ENODEV;
-
- utlbs = kcalloc(num_utlbs, sizeof(*utlbs), GFP_KERNEL);
- if (!utlbs)
- return -ENOMEM;
-
- spin_lock(&ipmmu_devices_lock);
-
- list_for_each_entry(mmu, &ipmmu_devices, list) {
- ret = ipmmu_find_utlbs(mmu, dev, utlbs, num_utlbs);
- if (!ret) {
- /*
- * TODO Take a reference to the MMU to protect
- * against device removal.
- */
- break;
- }
}
- spin_unlock(&ipmmu_devices_lock);
-
- if (ret < 0)
+ mmu = platform_get_drvdata(pdev);
+ if (!mmu) {
+ dev_dbg(dev, "%s: ipmmu not found\n", __func__);
return -ENODEV;
-
- for (i = 0; i < num_utlbs; ++i) {
- if (utlbs[i] >= mmu->num_utlbs) {
- ret = -EINVAL;
- goto error;
- }
}
- /* Create a device group and add the device to it. */
- group = iommu_group_alloc();
- if (IS_ERR(group)) {
- dev_err(dev, "Failed to allocate IOMMU group\n");
- ret = PTR_ERR(group);
- goto error;
+ /* Allocate arch data if not already done. */
+ if (!dev->archdata.iommu) {
+ dev->archdata.iommu = kzalloc(sizeof(*archdata), GFP_KERNEL);
+ if (!dev->archdata.iommu)
+ return -ENOMEM;
}
- ret = iommu_group_add_device(group, dev);
- iommu_group_put(group);
+ archdata = dev->archdata.iommu;
+ archdata->mmu = mmu;
- if (ret < 0) {
- dev_err(dev, "Failed to add device to IPMMU group\n");
- group = NULL;
- goto error;
+ /*
+ * We don't support handling a device through different IOMMU
+ * instances.
+ */
+ if (archdata->mmu && archdata->mmu != mmu) {
+ dev_warn(dev, "IOMMU driver already assigned to device %s\n",
+ dev_name(dev));
+ return -EINVAL;
}
- archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
- if (!archdata) {
- ret = -ENOMEM;
- goto error;
+ /* Validate and store the microTLB number. */
+ utlb = args->args[0];
+ if (utlb >= mmu->num_utlbs) {
+ dev_dbg(dev, "%s: invalid utlb %u\n", __func__, utlb);
+ return -EINVAL;
}
- archdata->mmu = mmu;
+ num_utlbs = archdata->num_utlbs + 1;
+ utlbs = krealloc(archdata->utlbs, num_utlbs * sizeof(*utlbs),
+ GFP_KERNEL);
+ if (utlbs == NULL)
+ return -ENOMEM;
+ utlbs[archdata->num_utlbs] = utlb;
+
archdata->utlbs = utlbs;
archdata->num_utlbs = num_utlbs;
- dev->archdata.iommu = archdata;
/*
* Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -699,50 +668,27 @@ static int ipmmu_add_device(struct device *dev)
SZ_1G, SZ_2G);
if (IS_ERR(mapping)) {
dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
- ret = PTR_ERR(mapping);
- goto error;
+ return PTR_ERR(mapping);
}
mmu->mapping = mapping;
}
- /* Attach the ARM VA mapping to the device. */
+ /*
+ * Detach the device from the default ARM VA mapping and attach it to
+ * our private mapping.
+ */
+ arm_iommu_detach_device(dev);
ret = arm_iommu_attach_device(dev, mmu->mapping);
if (ret < 0) {
dev_err(dev, "Failed to attach device to VA mapping\n");
- goto error;
+ return ret;
}
return 0;
-
-error:
- arm_iommu_release_mapping(mmu->mapping);
-
- kfree(dev->archdata.iommu);
- kfree(utlbs);
-
- dev->archdata.iommu = NULL;
-
- if (!IS_ERR_OR_NULL(group))
- iommu_group_remove_device(dev);
-
- return ret;
}
-static void ipmmu_remove_device(struct device *dev)
-{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
-
- arm_iommu_detach_device(dev);
- iommu_group_remove_device(dev);
-
- kfree(archdata->utlbs);
- kfree(archdata);
-
- dev->archdata.iommu = NULL;
-}
-
-static const struct iommu_ops ipmmu_ops = {
+static struct iommu_ops ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc,
.domain_free = ipmmu_domain_free,
.attach_dev = ipmmu_attach_device,
@@ -751,8 +697,8 @@ static const struct iommu_ops ipmmu_ops = {
.unmap = ipmmu_unmap,
.map_sg = default_iommu_map_sg,
.iova_to_phys = ipmmu_iova_to_phys,
- .add_device = ipmmu_add_device,
.remove_device = ipmmu_remove_device,
+ .of_xlate = ipmmu_of_xlate,
.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
};
@@ -831,10 +777,7 @@ static int ipmmu_probe(struct platform_device *pdev)
* ipmmu_init() after the probe function returns.
*/
- spin_lock(&ipmmu_devices_lock);
- list_add(&mmu->list, &ipmmu_devices);
- spin_unlock(&ipmmu_devices_lock);
-
+ of_iommu_set_ops(mmu->dev->of_node, &ipmmu_ops);
platform_set_drvdata(pdev, mmu);
return 0;
@@ -844,10 +787,6 @@ static int ipmmu_remove(struct platform_device *pdev)
{
struct ipmmu_vmsa_device *mmu = platform_get_drvdata(pdev);
- spin_lock(&ipmmu_devices_lock);
- list_del(&mmu->list);
- spin_unlock(&ipmmu_devices_lock);
-
arm_iommu_release_mapping(mmu->mapping);
ipmmu_device_reset(mmu);
@@ -883,14 +822,6 @@ static int __init ipmmu_init(void)
return 0;
}
-static void __exit ipmmu_exit(void)
-{
- return platform_driver_unregister(&ipmmu_driver);
-}
-
subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
-MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
-MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
-MODULE_LICENSE("GPL v2");
+IOMMU_OF_DECLARE(ipmmu_vmsa_of, "renesas,ipmmu-vmsa", NULL);
--
2.3.6
On Fri, May 15, 2015 at 02:00:07AM +0300, Laurent Pinchart wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet.
>
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> drivers/base/platform.c | 9 +++++++++
> drivers/of/platform.c | 7 +++----
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
Acked-by: Greg Kroah-Hartman <[email protected]>
Hi Laurent,
On 15/05/15 00:00, Laurent Pinchart wrote:
> The arm_iommu_create_mapping() function takes the IOMMU mapping size as
> a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
> bus masters would support the full 32 bits range. Fix this by passing
> the size as a 64 bits integer, and limiting it to 4GB for now.
FYI there's already an almost-identical patch in -rc3 from Marek fixing
this: 1424532b2163 ("ARM: 8347/1: dma-mapping: fix off-by-one check in
arm_setup_iommu_dma_ops").
Robin.
>
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> arch/arm/include/asm/dma-iommu.h | 2 +-
> arch/arm/mm/dma-mapping.c | 12 +++---------
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
> index 8e3fcb924db6..2ef282f96651 100644
> --- a/arch/arm/include/asm/dma-iommu.h
> +++ b/arch/arm/include/asm/dma-iommu.h
> @@ -25,7 +25,7 @@ struct dma_iommu_mapping {
> };
>
> struct dma_iommu_mapping *
> -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
> +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size);
>
> void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7aa5e339a596..c69e216a0cf9 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
> * arm_iommu_attach_device function.
> */
> struct dma_iommu_mapping *
> -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
> +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size)
> {
> unsigned int bits = size >> PAGE_SHIFT;
> unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
> @@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
> int extensions = 1;
> int err = -ENOMEM;
>
> - if (!bitmap_size)
> + /* Limit the size to 4GB for now. */
> + if (!size || size > (1ULL << 32))
> return ERR_PTR(-EINVAL);
>
> if (bitmap_size > PAGE_SIZE) {
> @@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
> if (!iommu)
> return false;
>
> - /*
> - * currently arm_iommu_create_mapping() takes a max of size_t
> - * for size param. So check this limit for now.
> - */
> - if (size > SIZE_MAX)
> - return false;
> -
> mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
> if (IS_ERR(mapping)) {
> pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
>
Hi Laurent,
On 15/05/15 00:00, Laurent Pinchart wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet.
This is great, as I think it also subtly solves the ordering problem the
current domain allocation has with platform devices. WRT to your comment
on the other thread, this actually makes things slightly saner for IOMMU
groups - the group assignment has to happen after device creation or
else some sysfs stuff blows up, so of_xlate is far too early and the
add_device callback is a reasonable place for it to be (until we can
move it out of every driver and into bus code). However, we're currently
attaching the device to the automatic domain long before that, so things
happen logically backwards and drivers like the ARM SMMU which actually
use the group to store relevant data get all confused.
With this change, the existing attach_device call in arch_setup_dma_ops
will actually work far more reliably, and I might be able to revive my
attempt to port the ARM SMMU driver over to of_xlate :D
Robin.
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> drivers/base/platform.c | 9 +++++++++
> drivers/of/platform.c | 7 +++----
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index ebf034b97278..508a866859dc 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
> if (ret < 0)
> return ret;
>
> + ret = of_dma_configure_ops(_dev, _dev->of_node);
> + if (ret < 0)
> + goto done;
> +
> ret = dev_pm_domain_attach(_dev, true);
> if (ret != -EPROBE_DEFER) {
> ret = drv->probe(dev);
> @@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
> dev_pm_domain_detach(_dev, true);
> }
>
> + if (ret)
> + of_dma_deconfigure(_dev);
> +
> +done:
> if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> dev_warn(_dev, "probe deferral not supported\n");
> ret = -ENXIO;
> @@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
>
> ret = drv->remove(dev);
> dev_pm_domain_detach(_dev, true);
> + of_dma_deconfigure(_dev);
>
> return ret;
> }
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9a29f09b7723..fc939bec799e 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -178,10 +178,8 @@ static struct platform_device *of_platform_device_create_pdata(
> dev->dev.bus = &platform_bus_type;
> dev->dev.platform_data = platform_data;
> of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> - of_dma_configure_ops(&dev->dev, dev->dev.of_node);
>
> if (of_device_add(dev) != 0) {
> - of_dma_deconfigure(&dev->dev);
> platform_device_put(dev);
> goto err_clear_flag;
> }
> @@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device *dev, void *data)
> if (dev->bus == &platform_bus_type)
> platform_device_unregister(to_platform_device(dev));
> #ifdef CONFIG_ARM_AMBA
> - else if (dev->bus == &amba_bustype)
> + else if (dev->bus == &amba_bustype) {
> amba_device_unregister(to_amba_device(dev));
> + of_dma_deconfigure(dev);
> + }
> #endif
>
> - of_dma_deconfigure(dev);
> of_node_clear_flag(dev->of_node, OF_POPULATED);
> of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
> return 0;
>
Hi Robin,
On Tuesday 19 May 2015 11:17:32 Robin Murphy wrote:
> On 15/05/15 00:00, Laurent Pinchart wrote:
> > The arm_iommu_create_mapping() function takes the IOMMU mapping size as
> > a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
> > bus masters would support the full 32 bits range. Fix this by passing
> > the size as a 64 bits integer, and limiting it to 4GB for now.
>
> FYI there's already an almost-identical patch in -rc3 from Marek fixing
> this: 1424532b2163 ("ARM: 8347/1: dma-mapping: fix off-by-one check in
> arm_setup_iommu_dma_ops").
Indeed, I should have rebased on top of -rc3. I suppose it's a good indication
that I was on the right track :-) I'll drop this patch from my series.
> > Signed-off-by: Laurent Pinchart
> > <[email protected]>
> > ---
> >
> > arch/arm/include/asm/dma-iommu.h | 2 +-
> > arch/arm/mm/dma-mapping.c | 12 +++---------
> > 2 files changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/dma-iommu.h
> > b/arch/arm/include/asm/dma-iommu.h index 8e3fcb924db6..2ef282f96651
> > 100644
> > --- a/arch/arm/include/asm/dma-iommu.h
> > +++ b/arch/arm/include/asm/dma-iommu.h
> > @@ -25,7 +25,7 @@ struct dma_iommu_mapping {
> > };
> >
> > struct dma_iommu_mapping *
> > -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> > size);
> > +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64
> > size);
> > void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
> >
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 7aa5e339a596..c69e216a0cf9 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
> >
> > * arm_iommu_attach_device function.
> > */
> >
> > struct dma_iommu_mapping *
> >
> > -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> > size) +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base,
> > u64 size)
> > {
> > unsigned int bits = size >> PAGE_SHIFT;
> > unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
> > @@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus,
> > dma_addr_t base, size_t size)
> > int extensions = 1;
> > int err = -ENOMEM;
> >
> > - if (!bitmap_size)
> > + /* Limit the size to 4GB for now. */
> > + if (!size || size > (1ULL << 32))
> > return ERR_PTR(-EINVAL);
> >
> > if (bitmap_size > PAGE_SIZE) {
> > @@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device
> > *dev, u64 dma_base, u64 size,
> > if (!iommu)
> > return false;
> >
> > - /*
> > - * currently arm_iommu_create_mapping() takes a max of size_t
> > - * for size param. So check this limit for now.
> > - */
> > - if (size > SIZE_MAX)
> > - return false;
> > -
> > mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
> > if (IS_ERR(mapping)) {
> > pr_warn("Failed to create %llu-byte IOMMU mapping for device
%s\n",
--
Regards,
Laurent Pinchart
Hi Robin,
On Tuesday 19 May 2015 11:39:17 Robin Murphy wrote:
> On 15/05/15 00:00, Laurent Pinchart wrote:
> > Configuring DMA ops at probe time will allow deferring device probe when
> > the IOMMU isn't available yet.
>
> This is great, as I think it also subtly solves the ordering problem the
> current domain allocation has with platform devices. WRT to your comment
> on the other thread, this actually makes things slightly saner for IOMMU
> groups - the group assignment has to happen after device creation or
> else some sysfs stuff blows up, so of_xlate is far too early and the
> add_device callback is a reasonable place for it to be (until we can
> move it out of every driver and into bus code). However, we're currently
> attaching the device to the automatic domain long before that, so things
> happen logically backwards and drivers like the ARM SMMU which actually
> use the group to store relevant data get all confused.
>
> With this change, the existing attach_device call in arch_setup_dma_ops
> will actually work far more reliably, and I might be able to revive my
> attempt to port the ARM SMMU driver over to of_xlate :D
Please do :-) I second IOMMU driver using the of_xlate + probe deferral
mechanism would help validate this patch series.
> > Signed-off-by: Laurent Pinchart
> > <[email protected]>
> > ---
> >
> > drivers/base/platform.c | 9 +++++++++
> > drivers/of/platform.c | 7 +++----
> > 2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index ebf034b97278..508a866859dc 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
> > if (ret < 0)
> > return ret;
> >
> > + ret = of_dma_configure_ops(_dev, _dev->of_node);
> > + if (ret < 0)
> > + goto done;
> > +
> > ret = dev_pm_domain_attach(_dev, true);
> > if (ret != -EPROBE_DEFER) {
> > ret = drv->probe(dev);
> > @@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
> > dev_pm_domain_detach(_dev, true);
> > }
> >
> > + if (ret)
> > + of_dma_deconfigure(_dev);
> > +
> >
> > +done:
> > if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> > dev_warn(_dev, "probe deferral not supported\n");
> > ret = -ENXIO;
> > @@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
> >
> > ret = drv->remove(dev);
> > dev_pm_domain_detach(_dev, true);
> > + of_dma_deconfigure(_dev);
> > return ret;
> > }
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 9a29f09b7723..fc939bec799e 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -178,10 +178,8 @@ static struct platform_device
> > *of_platform_device_create_pdata(
> > dev->dev.bus = &platform_bus_type;
> > dev->dev.platform_data = platform_data;
> > of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> > - of_dma_configure_ops(&dev->dev, dev->dev.of_node);
> >
> > if (of_device_add(dev) != 0) {
> > - of_dma_deconfigure(&dev->dev);
> > platform_device_put(dev);
> > goto err_clear_flag;
> > }
> > @@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device
> > *dev, void *data)
> > if (dev->bus == &platform_bus_type)
> > platform_device_unregister(to_platform_device(dev));
> >
> > #ifdef CONFIG_ARM_AMBA
> > - else if (dev->bus == &amba_bustype)
> > + else if (dev->bus == &amba_bustype) {
> > amba_device_unregister(to_amba_device(dev));
> > + of_dma_deconfigure(dev);
> > + }
> > #endif
> >
> > - of_dma_deconfigure(dev);
> > of_node_clear_flag(dev->of_node, OF_POPULATED);
> > of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
> > return 0;
--
Regards,
Laurent Pinchart
Hi Laurent,
On Fri, May 15, 2015 at 12:00:01AM +0100, Laurent Pinchart wrote:
> This patch series attempts to implement support for deferring probe of both
> IOMMU drivers and bus master drivers.
Have you had a chance to look at any of the feedback you've received on
this?
Will