Hi Marek,
On Wed, Nov 19, 2014 at 11:21:26AM +0000, Marek Szyprowski wrote:
> On 2014-11-14 19:56, Will Deacon wrote:
> > Hello everybody,
> >
> > Here is the fourth iteration of the RFC I've previously posted here:
> >
> > RFCv1:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/283023.html
> > RFCv2:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/283752.html
> > RFCv3:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/287031.html
> >
> > Changes since RFCv3 include:
> >
> > - Drastic simplification of the data structures, so that we no longer
> > pass around lists of domains. Instead, dma-mapping is expected to
> > allocate the domain (Joerg talked about adding a get_default_domain
> > operation to iommu_ops).
> >
> > - iommu_ops is used to hold the per-instance IOMMU data
> >
> > - Configuration of DMA segments added to of_dma_configure
> >
> > All feedback welcome.
>
> I've rebased my Exynos SYSMMU patches on top of this patchset and it
> works fine,
> You can find them in the "[PATCH v3 00/19] Exynos SYSMMU (IOMMU)
> integration with DT
> and DMA-mapping subsystem" thread.
I just saw that and it looks great, thanks! FWIW, I'll take the first 3
patches you have into my series in some shape or another.
> You can add to all your patches:
> Acked-by: Marek Szyprowski <m.szyprow...@samsung.com>
Cheers.
> I'm also interested in adding get_default_domain() callback, but I
> assume that this
> can be done once the basic patchset get merged. Do you plan to work on
> it, do you want
> me to implement it?
If Joerg isn't working on it already (I don't think he is), then please
do have a go if you have time. You'll probably want to avoid adding devices
with addressing restrictions (i.e. non-zero dma_pfn_offset, weird dma masks)
to the default domain, otherwise you'll run into issues initialising the
iova allocator.
I had a go at getting ARM dma-mapping to use a hypothetical
get_default_domain function, so I've included the diff I ended up with below,
in case it's at all useful.
Will
--->8
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index f3c0d953f6a2..5071553bf6b8 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,14 +121,9 @@ static inline unsigned long dma_max_pfn(struct device *dev)
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
-static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
- u64 size, struct iommu_ops *iommu,
- bool coherent)
-{
- if (coherent)
- set_dma_ops(dev, &arm_coherent_dma_ops);
-}
#define arch_setup_dma_ops arch_setup_dma_ops
+extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+ struct iommu_ops *iommu, bool coherent);
static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
{
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c245d903927f..da2c2667bbb1 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1849,7 +1849,8 @@ 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 iommu_domain *domain, dma_addr_t base,
+ size_t size)
{
unsigned int bits = size >> PAGE_SHIFT;
unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
@@ -1883,17 +1884,12 @@ arm_iommu_create_mapping(struct bus_type *bus,
dma_addr_t base, size_t size)
mapping->extensions = extensions;
mapping->base = base;
mapping->bits = BITS_PER_BYTE * bitmap_size;
+ mapping->domain = domain;
spin_lock_init(&mapping->lock);
- mapping->domain = iommu_domain_alloc(bus);
- if (!mapping->domain)
- goto err4;
-
kref_init(&mapping->kref);
return mapping;
-err4:
- kfree(mapping->bitmaps[0]);
err3:
kfree(mapping->bitmaps);
err2:
@@ -1901,6 +1897,23 @@ err2:
err:
return ERR_PTR(err);
}
+
+struct dma_iommu_mapping *
+arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
+{
+ struct dma_iommu_mapping *mapping;
+ struct iommu_domain *domain;
+
+ domain = iommu_domain_alloc(bus);
+ if (!domain)
+ return ERR_PTR(-ENOMEM);
+
+ mapping = __arm_iommu_create_mapping(domain, base, size);
+ if (IS_ERR(mapping))
+ iommu_domain_free(domain);
+
+ return mapping;
+}
EXPORT_SYMBOL_GPL(arm_iommu_create_mapping);
static void release_iommu_mapping(struct kref *kref)
@@ -1948,9 +1961,8 @@ EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
* arm_iommu_create_mapping)
*
* 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.
+ * More than one client might be attached to the same io address space
+ * mapping.
*/
int arm_iommu_attach_device(struct device *dev,
struct dma_iommu_mapping *mapping)
@@ -1963,7 +1975,6 @@ int arm_iommu_attach_device(struct device *dev,
kref_get(&mapping->kref);
dev->archdata.mapping = mapping;
- set_dma_ops(dev, &iommu_ops);
pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
return 0;
@@ -1975,7 +1986,6 @@ EXPORT_SYMBOL_GPL(arm_iommu_attach_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)
{
@@ -1990,10 +2000,141 @@ void arm_iommu_detach_device(struct device *dev)
iommu_detach_device(mapping->domain, dev);
kref_put(&mapping->kref, release_iommu_mapping);
dev->archdata.mapping = NULL;
- set_dma_ops(dev, NULL);
pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
}
EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
-#endif
+static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
+{
+ return coherent ? &iommu_coherent_ops : &iommu_ops;
+}
+
+struct dma_iommu_mapping_entry {
+ struct list_head list;
+ struct dma_iommu_mapping *mapping;
+ struct iommu_domain *domain;
+ u64 dma_base;
+ u64 size;
+ struct kref kref;
+};
+
+static DEFINE_SPINLOCK(dma_iommu_mapping_lock);
+static LIST_HEAD(dma_iommu_mapping_table);
+
+static void __remove_iommu_mapping_entry(struct kref *kref)
+{
+ struct dma_iommu_mapping_entry *entry;
+
+ entry = container_of(kref, struct dma_iommu_mapping_entry, kref);
+ list_del(&entry->list);
+}
+
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
+ struct iommu_ops *iommu)
+{
+ struct iommu_domain *domain;
+ struct dma_iommu_mapping_entry *entry = NULL;
+
+ if (!iommu->get_default_domain)
+ return false;
+
+ domain = iommu->get_default_domain(dev);
+ if (!domain)
+ return false;
+
+ spin_lock(&dma_iommu_mapping_lock);
+
+ list_for_each_entry(entry, &dma_iommu_mapping_table, list) {
+ if (entry->domain == domain)
+ break;
+ }
+
+ /* Load entry->mapping after entry -- not strictly necessary for ARM */
+ smp_read_barrier_depends();
+
+ if (!entry) {
+ struct dma_iommu_mapping *mapping;
+
+ entry = kzalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!entry)
+ goto err_unlock;
+
+ entry->domain = domain;
+ entry->dma_base = dma_base;
+ entry->size = size;
+ kref_init(&entry->kref);
+ list_add(&entry->list, &dma_iommu_mapping_table);
+ spin_unlock(&dma_iommu_mapping_lock);
+
+ mapping = __arm_iommu_create_mapping(domain, dma_base, size);
+ if (!IS_ERR(mapping))
+ return false;
+
+ smp_wmb();
+ entry->mapping = mapping;
+ } else if (entry->mapping) {
+ if (entry->dma_base > dma_base || entry->size > size)
+ goto err_unlock;
+
+ kref_get(&entry->kref);
+ spin_unlock(&dma_iommu_mapping_lock);
+ } else {
+ /* Racing on the same IOMMU */
+ goto err_unlock;
+ }
+
+ if (arm_iommu_attach_device(dev, entry->mapping)) {
+ int entry_dead;
+
+ pr_warn("Failed to attached device %s to IOMMU mapping\n",
+ dev_name(dev));
+ spin_lock(&dma_iommu_mapping_lock);
+ entry_dead = kref_put(&entry->kref,
+ __remove_iommu_mapping_entry);
+ spin_unlock(&dma_iommu_mapping_lock);
+
+ if (entry_dead) {
+ entry->mapping->domain = NULL;
+ arm_iommu_release_mapping(entry->mapping);
+ }
+
+ return false;
+ }
+
+ return true;
+
+err_unlock:
+ spin_unlock(&dma_iommu_mapping_lock);
+ return false;
+}
+
+#else
+
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
+ struct iommu_ops *iommu)
+{
+ return false;
+}
+
+#define arm_get_iommu_dma_map_ops arm_get_dma_map_ops
+
+#endif /* CONFIG_ARM_DMA_USE_IOMMU */
+
+static struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
+{
+ return coherent ? &arm_coherent_dma_ops : &arm_dma_ops;
+}
+
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+ struct iommu_ops *iommu, bool coherent)
+{
+ struct dma_map_ops *dma_ops;
+
+ if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu))
+ dma_ops = arm_get_iommu_dma_map_ops(coherent);
+ else
+ dma_ops = arm_get_dma_map_ops(coherent);
+
+ set_dma_ops(dev, dma_ops);
+}
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
