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);
