On 21/01/16 14:01, Niklas Söderlund wrote:
Add a version of dmap_{map,unmap}_page that can pass on attributes to
the underlaying map_page. This already exists for dma_{map,unmap}_single
and dmap_{map,unmap}_sg versions.
This looks reasonable in isolation, but for the task at hand I'm pretty
sure it's the wrong thing to do. The problem is that the DMA mapping and
IOMMU APIs are all geared around dealing with RAM, so what you're going
to end up with if you use this on an ARM system is the slave device's
MMIO registers mapped in the IOMMU as Normal memory. The net result of
that is that the interconnects between the IOMMU's downstream port and
the slave device are going to have free reign to reorder or merge the
DMA engine's transactions, and I'm sure you can imagine how utterly
disastrous that would be for e.g. reading/writing a FIFO. It's even
worse if the DMA engine is cache-coherent (either naturally, or by
virtue of the IOMMU), in which case the prospect of reads and writes
coming out of the IOMMU marked as Normal-Cacheable and allocating into
system caches is even more terrifyingly unpredictable.
I spent a little time recently looking into how we might handle platform
MSIs with IOMMU-backed DMA mapping, and the notion of slave DMA being a
similar problem did cross my mind, so I'm glad it's already being looked
at :) My initial thought was that a robust general solution probably
involves extending the DMA API with something like dma_map_resource(),
which would be a no-op with no IOMMU (or with IOMMUs like VT-d where
magic hardware solves the problem), interacting with something like the
below extension of the IOMMU API (plucked from my local MSI
proof-of-concept hacks).
Robin.
--->8---
From: Robin Murphy <robin.murphy@xxxxxxx>
Date: Wed, 23 Sep 2015 15:49:05 +0100
Subject: [PATCH] iommu: Add MMIO mapping type
On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---
drivers/iommu/io-pgtable-arm.c | 4 +++-
include/linux/iommu.h | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 8bbcbfe..5b5c299 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -363,7 +363,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
pte |= ARM_LPAE_PTE_HAP_READ;
if (prot & IOMMU_WRITE)
pte |= ARM_LPAE_PTE_HAP_WRITE;
- if (prot & IOMMU_CACHE)
+ if (prot & IOMMU_MMIO)
+ pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+ else if (prot & IOMMU_CACHE)
pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
else
pte |= ARM_LPAE_PTE_MEMATTR_NC;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f28dff3..addd25d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -30,6 +30,7 @@
#define IOMMU_WRITE (1 << 1)
#define IOMMU_CACHE (1 << 2) /* DMA cache coherency */
#define IOMMU_NOEXEC (1 << 3)
+#define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
struct iommu_ops;
struct iommu_group;
--->8---
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
---
include/asm-generic/dma-mapping-common.h | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
index b1bc954..bb08302 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -74,29 +74,33 @@ static inline void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg
ops->unmap_sg(dev, sg, nents, dir, attrs);
}
-static inline dma_addr_t dma_map_page(struct device *dev, struct page *page,
- size_t offset, size_t size,
- enum dma_data_direction dir)
+static inline dma_addr_t dma_map_page_attrs(struct device *dev,
+ struct page *page,
+ size_t offset, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
{
struct dma_map_ops *ops = get_dma_ops(dev);
dma_addr_t addr;
kmemcheck_mark_initialized(page_address(page) + offset, size);
BUG_ON(!valid_dma_direction(dir));
- addr = ops->map_page(dev, page, offset, size, dir, NULL);
+ addr = ops->map_page(dev, page, offset, size, dir, attrs);
debug_dma_map_page(dev, page, offset, size, dir, addr, false);
return addr;
}
-static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
+static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
+ size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
{
struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
if (ops->unmap_page)
- ops->unmap_page(dev, addr, size, dir, NULL);
+ ops->unmap_page(dev, addr, size, dir, attrs);
debug_dma_unmap_page(dev, addr, size, dir, false);
}
@@ -181,6 +185,8 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
#define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, NULL)
#define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, NULL)
#define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, NULL)
+#define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, r, NULL)
+#define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, NULL)
extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, dma_addr_t dma_addr, size_t size);
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
