[PATCHv8 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[PATCHv8 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Niklas Söderlund]

Hi,

This series tries to solve the problem with DMA with device registers
(MMIO registers) that are behind an IOMMU for the rcar-dmac driver. A
recent patch '9575632 (dmaengine: make slave address physical)'
clarifies that DMA slave address provided by clients is the physical
address. This puts the task of mapping the DMA slave address from a
phys_addr_t to a dma_addr_t on the DMA engine.

Without an IOMMU this is easy since the phys_addr_t and dma_addr_t are
the same and no special care is needed. However if you have a IOMMU you
need to map the DMA slave phys_addr_t to a dma_addr_t using something
like this.

This series is based on top of v4.7-rc1. And I'm hoping to be able to collect a 
Ack from Russell King on patch 4/6 that adds the ARM specific part and then be 
able to take the whole series through the dmaengine tree. If this is not the 
best route I'm more then happy to do it another way.

It's tested on a Koelsch with CONFIG_IPMMU_VMSA and by enabling the
ipmmu_ds node in r8a7791.dtsi. I verified operation by interacting with
/dev/mmcblk1, i2c and the serial console which are devices behind the
iommu.

Furthermore I have audited to the best of my ability all call paths
involved to make sure that the dma_addr_t obtained from
dma_map_resource() to is not used in a way where it would be expected
for the mapping to be RAM (have a struct page). Many thanks to Christoph
Hellwig and Laurent Pinchart for there input in this effort.

  * drivers/dma/sh/rcar-dmac.c
    Once the phys_addr_t is mapped to a dma_addr_t using
    dma_map_resource() it is only used to check that the transferee do not
    cross 4GB boundaries and then only directly written to HW registers.

  * drivers/iommu/iommu.c
    - iommu_map()
      Check that it's align to min page size or return -EINVAL then calls
      domain->ops->map()

  * drivers/iommu/ipmmu-vmsa.c
    - ipmmu_map()
      No logic only calls domain->ops->map()

  * drivers/iommu/io-pgtable-arm.c
    - arm_lpae_map()
      No logic only calls __arm_lpae_map()
    - __arm_lpae_map()
      No logic only calls arm_lpae_init_pte()
    - arm_lpae_init_pte()
      Used to get a pte:
        pte |= pfn_to_iopte(paddr >> data->pg_shift, data);

  * drivers/iommu/io-pgtable-arm-v7s.c
    - arm_v7s_map()
      No logic only calls __arm_v7s_map()
    - __arm_v7s_map()
      No logic only calls arm_v7s_init_pte()
    - arm_v7s_init_pte
      Used to get a pte:
        pte |= paddr & ARM_V7S_LVL_MASK(lvl);

  * ARM dma-mapping
    - dma_unmap_*
      Only valid unmap is dma_unmap_resource() all others are an invalid
      use case.
    - dma_sync_single_*
      Invalid use case, memory that is mapped is device memory
    - dma_common_mmap() and dma_mmap_attrs()
      Invalid use case
    - dma_common_get_sgtable() and dma_get_sgtable_attrs()
      Invalid use case, only for dma_alloc_* allocated memory,
    - dma_mapping_error()
      OK

* Changes since v7
- Use size_t instead of int for length in arm_iommu_map_resource() and 
  arm_iommu_unmap_resource().
- Fix bug in arm_iommu_map_resource() where wrong variable where passed to 
  __alloc_iova(). Thanks to Russell King for pointing out both errors.

* Changes since v6
- Use offset_in_page() and __pfn_to_phys(). This fixed a bug in the
  lib/dma-debug.c. Thanks to Konrad Rzeszutek Wilk for finding it and Robin
  Murphy for suggesting offset_in_page().
- Rebased on top of v4.7-rc1.
- Dropped DT patches which enabled the IPMMU on Renesas Koelsch and Lager. Will
  post them separately at a later time.

* Changes since v5
- Add dma-debug work which adds a new mapping type for the resource
  mapping which correctly can be translated to a physical address.
- Drop patches from Robin Murphy since they now are accepted in the
  iommu repository and base the series on that tree instead.
- Add a review tag from Laurent.

* Changes since v4
- Move the mapping from phys_addr_t to dma_addr_t from slave_config to the
  prepare calls. This way we know the direction of the mapping and don't have
  to use DMA_BIDIRECTIONAL. Thanks Vinod for suggesting this.
- To be clear that the data type for slave addresses are changed add a patch
  that only changes the data type to phys_addr_t.
- Fixed up commit messages.

* Changes since v3
- Folded in a fix from Robin to his patch.
- Added a check to make sure dma_map_resource can not be used to map RAM as
  pointed out by Robin. I use BUG_ON to enforce this. It might not be the best
  method but I saw no other good way since DMA_ERROR_CODE might not be defined
  on all platforms.
- Added comment about that DTS changes will disable 2 DMA channels due to a HW
  (?) bug in the DMAC.
- Dropped the use of dma_attrs, no longer needed.
- Collected Acked-by and Reviewed-by from Laurent.
- Various indentation fix ups.

* Changes since v2
- Drop patch to add dma_{map,unmap}_page_attrs.
- Add dma_{map,unmap}_resource to handle the mapping without involving a
  'struct page'. Thanks Laurent and Robin for pointing this out.
- Use size instead of address to keep track of if a mapping exist or not
  since addr == 0 is valid. Thanks Laurent.
- Pick up patch from Robin with Laurents ack (hope it's OK for me to
  attach the ack?) to add IOMMU_MMIO.
- Fix bug in rcar_dmac_device_config where the error check where
  inverted.
- Use DMA_BIDIRECTIONAL in rcar_dmac_device_config since we at that
  point can't be sure what direction the mapping is going to be used.

* Changes since v1
- Add and use a dma_{map,unmap}_page_attrs to be able to map the page
  using attributes DMA_ATTR_NO_KERNEL_MAPPING and
  DMA_ATTR_SKIP_CPU_SYNC. Thanks Laurent.
- Drop check if dmac is part of a iommu group or not, let the DMA
  mapping api handle it.
- Move slave configuration data around in rcar-dmac to avoid code
  duplication.
- Fix build issue reported by 'kbuild test robot' regarding phys_to_page
  not availability on some configurations.
- Add DT information for r8a7791.

* Changes since RFC
- Switch to use the dma-mapping api instead of using the iommu_map()
  directly. Turns out the dma-mapper is much smarter then me...
- Dropped the patch to expose domain->ops->pgsize_bitmap from within the
  iommu api.
- Dropped the patch showing how I tested the RFC.

Niklas S?derlund (6):
  dma-mapping: add {map,unmap}_resource to dma_map_ops
  dma-debug: add support for resource mappings
  dma-mapping: add dma_{map,unmap}_resource
  arm: dma-mapping: add {map,unmap}_resource for iommu ops
  dmaengine: rcar-dmac: group slave configuration
  dmaengine: rcar-dmac: add iommu support for slave transfers

 Documentation/DMA-API.txt   |  22 +++++++--
 arch/arm/mm/dma-mapping.c   |  63 ++++++++++++++++++++++++
 drivers/dma/sh/rcar-dmac.c  | 116 +++++++++++++++++++++++++++++++++++---------
 include/linux/dma-debug.h   |  19 ++++++++
 include/linux/dma-mapping.h |  42 ++++++++++++++++
 lib/dma-debug.c             |  52 +++++++++++++++++++-
 6 files changed, 285 insertions(+), 29 deletions(-)

-- 
2.8.2

[PATCHv8 1/6] dma-mapping: add {map,unmap}_resource to dma_map_ops

[Niklas Söderlund]

Add methods to handle mapping of device resources from a physical
address. This is needed for example to be able to map MMIO FIFO
registers to a IOMMU.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/linux/dma-mapping.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index c980a92..a11ff9d 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -49,6 +49,12 @@ struct dma_map_ops {
 			 struct scatterlist *sg, int nents,
 			 enum dma_data_direction dir,
 			 struct dma_attrs *attrs);
+	dma_addr_t (*map_resource)(struct device *dev, phys_addr_t phys_addr,
+			       size_t size, enum dma_data_direction dir,
+			       struct dma_attrs *attrs);
+	void (*unmap_resource)(struct device *dev, dma_addr_t dma_handle,
+			   size_t size, enum dma_data_direction dir,
+			   struct dma_attrs *attrs);
 	void (*sync_single_for_cpu)(struct device *dev,
 				    dma_addr_t dma_handle, size_t size,
 				    enum dma_data_direction dir);
-- 
2.8.2

[PATCHv8 2/6] dma-debug: add support for resource mappings

[Niklas Söderlund]

A MMIO mapped resource can not be represented by a struct page so a new
debug type is needed to handle this. This patch add such type and
functionality to add/remove entries and how to translate them to a
physical address.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-debug.h | 19 +++++++++++++++++
 lib/dma-debug.c           | 52 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index fe8cb61..c7d844f 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -56,6 +56,13 @@ extern void debug_dma_alloc_coherent(struct device *dev, size_t size,
 extern void debug_dma_free_coherent(struct device *dev, size_t size,
 				    void *virt, dma_addr_t addr);
 
+extern void debug_dma_map_resource(struct device *dev, phys_addr_t addr,
+				   size_t size, int direction,
+				   dma_addr_t dma_addr);
+
+extern void debug_dma_unmap_resource(struct device *dev, dma_addr_t dma_addr,
+				     size_t size, int direction);
+
 extern void debug_dma_sync_single_for_cpu(struct device *dev,
 					  dma_addr_t dma_handle, size_t size,
 					  int direction);
@@ -141,6 +148,18 @@ static inline void debug_dma_free_coherent(struct device *dev, size_t size,
 {
 }
 
+static inline void debug_dma_map_resource(struct device *dev, phys_addr_t addr,
+					  size_t size, int direction,
+					  dma_addr_t dma_addr)
+{
+}
+
+static inline void debug_dma_unmap_resource(struct device *dev,
+					    dma_addr_t dma_addr, size_t size,
+					    int direction)
+{
+}
+
 static inline void debug_dma_sync_single_for_cpu(struct device *dev,
 						 dma_addr_t dma_handle,
 						 size_t size, int direction)
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 51a76af..ce75fba 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -43,6 +43,7 @@ enum {
 	dma_debug_page,
 	dma_debug_sg,
 	dma_debug_coherent,
+	dma_debug_resource,
 };
 
 enum map_err_types {
@@ -150,8 +151,9 @@ static const char *const maperr2str[] = {
 	[MAP_ERR_CHECKED] = "dma map error checked",
 };
 
-static const char *type2name[4] = { "single", "page",
-				    "scather-gather", "coherent" };
+static const char *type2name[5] = { "single", "page",
+				    "scather-gather", "coherent",
+				    "resource" };
 
 static const char *dir2name[4] = { "DMA_BIDIRECTIONAL", "DMA_TO_DEVICE",
 				   "DMA_FROM_DEVICE", "DMA_NONE" };
@@ -397,6 +399,9 @@ static void hash_bucket_del(struct dma_debug_entry *entry)
 
 static unsigned long long phys_addr(struct dma_debug_entry *entry)
 {
+	if (entry->type == dma_debug_resource)
+		return __pfn_to_phys(entry->pfn) + entry->offset;
+
 	return page_to_phys(pfn_to_page(entry->pfn)) + entry->offset;
 }
 
@@ -1493,6 +1498,49 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
 }
 EXPORT_SYMBOL(debug_dma_free_coherent);
 
+void debug_dma_map_resource(struct device *dev, phys_addr_t addr, size_t size,
+			    int direction, dma_addr_t dma_addr)
+{
+	struct dma_debug_entry *entry;
+
+	if (unlikely(dma_debug_disabled()))
+		return;
+
+	entry = dma_entry_alloc();
+	if (!entry)
+		return;
+
+	entry->type		= dma_debug_resource;
+	entry->dev		= dev;
+	entry->pfn		= __phys_to_pfn(addr);
+	entry->offset		= offset_in_page(addr);
+	entry->size		= size;
+	entry->dev_addr		= dma_addr;
+	entry->direction	= direction;
+	entry->map_err_type	= MAP_ERR_NOT_CHECKED;
+
+	add_dma_entry(entry);
+}
+EXPORT_SYMBOL(debug_dma_map_resource);
+
+void debug_dma_unmap_resource(struct device *dev, dma_addr_t dma_addr,
+			      size_t size, int direction)
+{
+	struct dma_debug_entry ref = {
+		.type           = dma_debug_resource,
+		.dev            = dev,
+		.dev_addr       = dma_addr,
+		.size           = size,
+		.direction      = direction,
+	};
+
+	if (unlikely(dma_debug_disabled()))
+		return;
+
+	check_unmap(&ref);
+}
+EXPORT_SYMBOL(debug_dma_unmap_resource);
+
 void debug_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
 				   size_t size, int direction)
 {
-- 
2.8.2

[PATCHv8 3/6] dma-mapping: add dma_{map,unmap}_resource

[Niklas Söderlund]

Map/Unmap a device MMIO resource from a physical address. If no dma_map_ops
method is available the operation is a no-op.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 Documentation/DMA-API.txt   | 22 +++++++++++++++++-----
 include/linux/dma-mapping.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 45ef3f2..c7e5f99 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -277,14 +277,26 @@ and <size> parameters are provided to do partial page mapping, it is
 recommended that you never use these unless you really know what the
 cache width is.
 
+dma_addr_t
+dma_map_resource(struct device *dev, phys_addr_t phys_addr, size_t size,
+		 enum dma_data_direction dir, struct dma_attrs *attrs)
+
+void
+dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
+		   enum dma_data_direction dir, struct dma_attrs *attrs)
+
+API for mapping and unmapping for MMIO resources. All the notes and
+warnings for the other mapping APIs apply here. The API should only be
+used to map device MMIO resources, mapping of RAM is not permitted.
+
 int
 dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
 
-In some circumstances dma_map_single() and dma_map_page() will fail to create
-a mapping. A driver can check for these errors by testing the returned
-DMA address with dma_mapping_error(). A non-zero return value means the mapping
-could not be created and the driver should take appropriate action (e.g.
-reduce current DMA mapping usage or delay and try again later).
+In some circumstances dma_map_single(), dma_map_page() and dma_map_resource()
+will fail to create a mapping. A driver can check for these errors by testing
+the returned DMA address with dma_mapping_error(). A non-zero return value
+means the mapping could not be created and the driver should take appropriate
+action (e.g. reduce current DMA mapping usage or delay and try again later).
 
 	int
 	dma_map_sg(struct device *dev, struct scatterlist *sg,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index a11ff9d..4c381ba 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -218,6 +218,42 @@ static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
 	debug_dma_unmap_page(dev, addr, size, dir, false);
 }
 
+static inline dma_addr_t dma_map_resource(struct device *dev,
+					  phys_addr_t phys_addr,
+					  size_t size,
+					  enum dma_data_direction dir,
+					  struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+	unsigned long pfn = __phys_to_pfn(phys_addr);
+	dma_addr_t addr;
+
+	BUG_ON(!valid_dma_direction(dir));
+
+	/* Don't allow RAM to be mapped */
+	BUG_ON(pfn_valid(pfn));
+
+	addr = phys_addr;
+	if (ops->map_resource)
+		addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
+
+	debug_dma_map_resource(dev, phys_addr, size, dir, addr);
+
+	return addr;
+}
+
+static inline void dma_unmap_resource(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_resource)
+		ops->unmap_resource(dev, addr, size, dir, attrs);
+	debug_dma_unmap_resource(dev, addr, size, dir);
+}
+
 static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
 					   size_t size,
 					   enum dma_data_direction dir)
-- 
2.8.2

[PATCHv8 4/6] arm: dma-mapping: add {map, unmap}_resource for iommu ops

[Niklas Söderlund]

Add methods to map/unmap device resources addresses for dma_map_ops that
are IOMMU aware. This is needed to map a device MMIO register from a
physical address.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mm/dma-mapping.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ff7ed56..7e9c8eb 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1951,6 +1951,63 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 	__free_iova(mapping, iova, len);
 }
 
+/**
+ * arm_iommu_map_resource - map a device resource for DMA
+ * @dev: valid struct device pointer
+ * @phys_addr: physical address of resource
+ * @size: size of resource to map
+ * @dir: DMA transfer direction
+ */
+static dma_addr_t arm_iommu_map_resource(struct device *dev,
+		phys_addr_t phys_addr, size_t size,
+		enum dma_data_direction dir, struct dma_attrs *attrs)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+	dma_addr_t dma_addr;
+	int ret, prot;
+	phys_addr_t addr = phys_addr & PAGE_MASK;
+	unsigned int offset = phys_addr & ~PAGE_MASK;
+	size_t len = PAGE_ALIGN(size + offset);
+
+	dma_addr = __alloc_iova(mapping, len);
+	if (dma_addr == DMA_ERROR_CODE)
+		return dma_addr;
+
+	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
+
+	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
+	if (ret < 0)
+		goto fail;
+
+	return dma_addr + offset;
+fail:
+	__free_iova(mapping, dma_addr, len);
+	return DMA_ERROR_CODE;
+}
+
+/**
+ * arm_iommu_unmap_resource - unmap a device DMA resource
+ * @dev: valid struct device pointer
+ * @dma_handle: DMA address to resource
+ * @size: size of resource to map
+ * @dir: DMA transfer direction
+ */
+static void arm_iommu_unmap_resource(struct device *dev, dma_addr_t dma_handle,
+		size_t size, enum dma_data_direction dir,
+		struct dma_attrs *attrs)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+	dma_addr_t iova = dma_handle & PAGE_MASK;
+	unsigned int offset = dma_handle & ~PAGE_MASK;
+	size_t len = PAGE_ALIGN(size + offset);
+
+	if (!iova)
+		return;
+
+	iommu_unmap(mapping->domain, iova, len);
+	__free_iova(mapping, iova, len);
+}
+
 static void arm_iommu_sync_single_for_cpu(struct device *dev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir)
 {
@@ -1994,6 +2051,9 @@ struct dma_map_ops iommu_ops = {
 	.unmap_sg		= arm_iommu_unmap_sg,
 	.sync_sg_for_cpu	= arm_iommu_sync_sg_for_cpu,
 	.sync_sg_for_device	= arm_iommu_sync_sg_for_device,
+
+	.map_resource		= arm_iommu_map_resource,
+	.unmap_resource		= arm_iommu_unmap_resource,
 };
 
 struct dma_map_ops iommu_coherent_ops = {
@@ -2007,6 +2067,9 @@ struct dma_map_ops iommu_coherent_ops = {
 
 	.map_sg		= arm_coherent_iommu_map_sg,
 	.unmap_sg	= arm_coherent_iommu_unmap_sg,
+
+	.map_resource	= arm_iommu_map_resource,
+	.unmap_resource	= arm_iommu_unmap_resource,
 };
 
 /**
-- 
2.8.2

[PATCHv8 5/6] dmaengine: rcar-dmac: group slave configuration

[Niklas Söderlund]

Group slave address and transfer size in own structs for source and
destination. This is in preparation for hooking up the dma-mapping API
to the slave addresses.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/dma/sh/rcar-dmac.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index dfb1792..b0c3bb2 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -118,14 +118,22 @@ struct rcar_dmac_desc_page {
 	sizeof(struct rcar_dmac_xfer_chunk))
 
 /*
+ * struct rcar_dmac_chan_slave - Slave configuration
+ * @slave_addr: slave memory address
+ * @xfer_size: size (in bytes) of hardware transfers
+ */
+struct rcar_dmac_chan_slave {
+	phys_addr_t slave_addr;
+	unsigned int xfer_size;
+};
+
+/*
  * struct rcar_dmac_chan - R-Car Gen2 DMA Controller Channel
  * @chan: base DMA channel object
  * @iomem: channel I/O memory base
  * @index: index of this channel in the controller
- * @src_xfer_size: size (in bytes) of hardware transfers on the source side
- * @dst_xfer_size: size (in bytes) of hardware transfers on the destination side
- * @src_slave_addr: slave source memory address
- * @dst_slave_addr: slave destination memory address
+ * @src: slave memory address and size on the source side
+ * @dst: slave memory address and size on the destination side
  * @mid_rid: hardware MID/RID for the DMA client using this channel
  * @lock: protects the channel CHCR register and the desc members
  * @desc.free: list of free descriptors
@@ -142,10 +150,8 @@ struct rcar_dmac_chan {
 	void __iomem *iomem;
 	unsigned int index;
 
-	unsigned int src_xfer_size;
-	unsigned int dst_xfer_size;
-	dma_addr_t src_slave_addr;
-	dma_addr_t dst_slave_addr;
+	struct rcar_dmac_chan_slave src;
+	struct rcar_dmac_chan_slave dst;
 	int mid_rid;
 
 	spinlock_t lock;
@@ -793,13 +799,13 @@ static void rcar_dmac_chan_configure_desc(struct rcar_dmac_chan *chan,
 	case DMA_DEV_TO_MEM:
 		chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_FIXED
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->src_xfer_size;
+		xfer_size = chan->src.xfer_size;
 		break;
 
 	case DMA_MEM_TO_DEV:
 		chcr = RCAR_DMACHCR_DM_FIXED | RCAR_DMACHCR_SM_INC
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->dst_xfer_size;
+		xfer_size = chan->dst.xfer_size;
 		break;
 
 	case DMA_MEM_TO_MEM:
@@ -1038,7 +1044,7 @@ rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, false);
 }
@@ -1093,7 +1099,7 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, true);
 
@@ -1110,10 +1116,10 @@ static int rcar_dmac_device_config(struct dma_chan *chan,
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src_slave_addr = cfg->src_addr;
-	rchan->dst_slave_addr = cfg->dst_addr;
-	rchan->src_xfer_size = cfg->src_addr_width;
-	rchan->dst_xfer_size = cfg->dst_addr_width;
+	rchan->src.slave_addr = cfg->src_addr;
+	rchan->dst.slave_addr = cfg->dst_addr;
+	rchan->src.xfer_size = cfg->src_addr_width;
+	rchan->dst.xfer_size = cfg->dst_addr_width;
 
 	return 0;
 }
-- 
2.8.2

[PATCHv8 6/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Niklas Söderlund]

Enable slave transfers to a device behind a IPMMU by mapping the slave
addresses using the dma-mapping API.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/dma/sh/rcar-dmac.c | 82 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index b0c3bb2..8592598 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -128,6 +128,18 @@ struct rcar_dmac_chan_slave {
 };
 
 /*
+ * struct rcar_dmac_chan_map - Map of slave device phys to dma address
+ * @addr: slave dma address
+ * @dir: direction of mapping
+ * @slave: slave configuration that is mapped
+ */
+struct rcar_dmac_chan_map {
+	dma_addr_t addr;
+	enum dma_data_direction dir;
+	struct rcar_dmac_chan_slave slave;
+};
+
+/*
  * struct rcar_dmac_chan - R-Car Gen2 DMA Controller Channel
  * @chan: base DMA channel object
  * @iomem: channel I/O memory base
@@ -152,6 +164,7 @@ struct rcar_dmac_chan {
 
 	struct rcar_dmac_chan_slave src;
 	struct rcar_dmac_chan_slave dst;
+	struct rcar_dmac_chan_map map;
 	int mid_rid;
 
 	spinlock_t lock;
@@ -1027,13 +1040,65 @@ rcar_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dma_dest,
 				      DMA_MEM_TO_MEM, flags, false);
 }
 
+static int rcar_dmac_map_slave_addr(struct dma_chan *chan,
+				    enum dma_transfer_direction dir)
+{
+	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+	struct rcar_dmac_chan_map *map = &rchan->map;
+	phys_addr_t dev_addr;
+	size_t dev_size;
+	enum dma_data_direction dev_dir;
+
+	if (dir == DMA_DEV_TO_MEM) {
+		dev_addr = rchan->src.slave_addr;
+		dev_size = rchan->src.xfer_size;
+		dev_dir = DMA_TO_DEVICE;
+	} else {
+		dev_addr = rchan->dst.slave_addr;
+		dev_size = rchan->dst.xfer_size;
+		dev_dir = DMA_FROM_DEVICE;
+	}
+
+	/* Reuse current map if possible. */
+	if (dev_addr == map->slave.slave_addr &&
+	    dev_size == map->slave.xfer_size &&
+	    dev_dir == map->dir)
+		return 0;
+
+	/* Remove old mapping if present. */
+	if (map->slave.xfer_size)
+		dma_unmap_resource(chan->device->dev, map->addr,
+				   map->slave.xfer_size, map->dir, NULL);
+	map->slave.xfer_size = 0;
+
+	/* Create new slave address map. */
+	map->addr = dma_map_resource(chan->device->dev, dev_addr, dev_size,
+				     dev_dir, NULL);
+
+	if (dma_mapping_error(chan->device->dev, map->addr)) {
+		dev_err(chan->device->dev,
+			"chan%u: failed to map %zx@%pap", rchan->index,
+			dev_size, &dev_addr);
+		return -EIO;
+	}
+
+	dev_dbg(chan->device->dev, "chan%u: map %zx@%pap to %pad dir: %s\n",
+		rchan->index, dev_size, &dev_addr, &map->addr,
+		dev_dir == DMA_TO_DEVICE ? "DMA_TO_DEVICE" : "DMA_FROM_DEVICE");
+
+	map->slave.slave_addr = dev_addr;
+	map->slave.xfer_size = dev_size;
+	map->dir = dev_dir;
+
+	return 0;
+}
+
 static struct dma_async_tx_descriptor *
 rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			unsigned int sg_len, enum dma_transfer_direction dir,
 			unsigned long flags, void *context)
 {
 	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
-	dma_addr_t dev_addr;
 
 	/* Someone calling slave DMA on a generic channel? */
 	if (rchan->mid_rid < 0 || !sg_len) {
@@ -1043,9 +1108,10 @@ rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		return NULL;
 	}
 
-	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
-	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
+	if (rcar_dmac_map_slave_addr(chan, dir))
+		return NULL;
+
+	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, rchan->map.addr,
 				      dir, flags, false);
 }
 
@@ -1059,7 +1125,6 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
 	struct dma_async_tx_descriptor *desc;
 	struct scatterlist *sgl;
-	dma_addr_t dev_addr;
 	unsigned int sg_len;
 	unsigned int i;
 
@@ -1071,6 +1136,9 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 		return NULL;
 	}
 
+	if (rcar_dmac_map_slave_addr(chan, dir))
+		return NULL;
+
 	sg_len = buf_len / period_len;
 	if (sg_len > RCAR_DMAC_MAX_SG_LEN) {
 		dev_err(chan->device->dev,
@@ -1098,9 +1166,7 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 		sg_dma_len(&sgl[i]) = period_len;
 	}
 
-	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
-	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
+	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, rchan->map.addr,
 				      dir, flags, true);
 
 	kfree(sgl);
-- 
2.8.2

[PATCHv8 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Niklas Söderlund]

Hi Russell,

I was wondering if you have time to look at this series?
Especially patch 4/6 which you had some good review comments on in v7.

On 2016-06-07 09:54:07 +0200, Niklas S?derlund wrote:
> Hi,
> 
> This series tries to solve the problem with DMA with device registers
> (MMIO registers) that are behind an IOMMU for the rcar-dmac driver. A
> recent patch '9575632 (dmaengine: make slave address physical)'
> clarifies that DMA slave address provided by clients is the physical
> address. This puts the task of mapping the DMA slave address from a
> phys_addr_t to a dma_addr_t on the DMA engine.
> 
> Without an IOMMU this is easy since the phys_addr_t and dma_addr_t are
> the same and no special care is needed. However if you have a IOMMU you
> need to map the DMA slave phys_addr_t to a dma_addr_t using something
> like this.
> 
> This series is based on top of v4.7-rc1. And I'm hoping to be able to collect a 
> Ack from Russell King on patch 4/6 that adds the ARM specific part and then be 
> able to take the whole series through the dmaengine tree. If this is not the 
> best route I'm more then happy to do it another way.
> 
> It's tested on a Koelsch with CONFIG_IPMMU_VMSA and by enabling the
> ipmmu_ds node in r8a7791.dtsi. I verified operation by interacting with
> /dev/mmcblk1, i2c and the serial console which are devices behind the
> iommu.
> 
> Furthermore I have audited to the best of my ability all call paths
> involved to make sure that the dma_addr_t obtained from
> dma_map_resource() to is not used in a way where it would be expected
> for the mapping to be RAM (have a struct page). Many thanks to Christoph
> Hellwig and Laurent Pinchart for there input in this effort.
> 
>   * drivers/dma/sh/rcar-dmac.c
>     Once the phys_addr_t is mapped to a dma_addr_t using
>     dma_map_resource() it is only used to check that the transferee do not
>     cross 4GB boundaries and then only directly written to HW registers.
> 
>   * drivers/iommu/iommu.c
>     - iommu_map()
>       Check that it's align to min page size or return -EINVAL then calls
>       domain->ops->map()
> 
>   * drivers/iommu/ipmmu-vmsa.c
>     - ipmmu_map()
>       No logic only calls domain->ops->map()
> 
>   * drivers/iommu/io-pgtable-arm.c
>     - arm_lpae_map()
>       No logic only calls __arm_lpae_map()
>     - __arm_lpae_map()
>       No logic only calls arm_lpae_init_pte()
>     - arm_lpae_init_pte()
>       Used to get a pte:
>         pte |= pfn_to_iopte(paddr >> data->pg_shift, data);
> 
>   * drivers/iommu/io-pgtable-arm-v7s.c
>     - arm_v7s_map()
>       No logic only calls __arm_v7s_map()
>     - __arm_v7s_map()
>       No logic only calls arm_v7s_init_pte()
>     - arm_v7s_init_pte
>       Used to get a pte:
>         pte |= paddr & ARM_V7S_LVL_MASK(lvl);
> 
>   * ARM dma-mapping
>     - dma_unmap_*
>       Only valid unmap is dma_unmap_resource() all others are an invalid
>       use case.
>     - dma_sync_single_*
>       Invalid use case, memory that is mapped is device memory
>     - dma_common_mmap() and dma_mmap_attrs()
>       Invalid use case
>     - dma_common_get_sgtable() and dma_get_sgtable_attrs()
>       Invalid use case, only for dma_alloc_* allocated memory,
>     - dma_mapping_error()
>       OK
> 
> * Changes since v7
> - Use size_t instead of int for length in arm_iommu_map_resource() and 
>   arm_iommu_unmap_resource().
> - Fix bug in arm_iommu_map_resource() where wrong variable where passed to 
>   __alloc_iova(). Thanks to Russell King for pointing out both errors.
> 
> * Changes since v6
> - Use offset_in_page() and __pfn_to_phys(). This fixed a bug in the
>   lib/dma-debug.c. Thanks to Konrad Rzeszutek Wilk for finding it and Robin
>   Murphy for suggesting offset_in_page().
> - Rebased on top of v4.7-rc1.
> - Dropped DT patches which enabled the IPMMU on Renesas Koelsch and Lager. Will
>   post them separately at a later time.
> 
> * Changes since v5
> - Add dma-debug work which adds a new mapping type for the resource
>   mapping which correctly can be translated to a physical address.
> - Drop patches from Robin Murphy since they now are accepted in the
>   iommu repository and base the series on that tree instead.
> - Add a review tag from Laurent.
> 
> * Changes since v4
> - Move the mapping from phys_addr_t to dma_addr_t from slave_config to the
>   prepare calls. This way we know the direction of the mapping and don't have
>   to use DMA_BIDIRECTIONAL. Thanks Vinod for suggesting this.
> - To be clear that the data type for slave addresses are changed add a patch
>   that only changes the data type to phys_addr_t.
> - Fixed up commit messages.
> 
> * Changes since v3
> - Folded in a fix from Robin to his patch.
> - Added a check to make sure dma_map_resource can not be used to map RAM as
>   pointed out by Robin. I use BUG_ON to enforce this. It might not be the best
>   method but I saw no other good way since DMA_ERROR_CODE might not be defined
>   on all platforms.
> - Added comment about that DTS changes will disable 2 DMA channels due to a HW
>   (?) bug in the DMAC.
> - Dropped the use of dma_attrs, no longer needed.
> - Collected Acked-by and Reviewed-by from Laurent.
> - Various indentation fix ups.
> 
> * Changes since v2
> - Drop patch to add dma_{map,unmap}_page_attrs.
> - Add dma_{map,unmap}_resource to handle the mapping without involving a
>   'struct page'. Thanks Laurent and Robin for pointing this out.
> - Use size instead of address to keep track of if a mapping exist or not
>   since addr == 0 is valid. Thanks Laurent.
> - Pick up patch from Robin with Laurents ack (hope it's OK for me to
>   attach the ack?) to add IOMMU_MMIO.
> - Fix bug in rcar_dmac_device_config where the error check where
>   inverted.
> - Use DMA_BIDIRECTIONAL in rcar_dmac_device_config since we at that
>   point can't be sure what direction the mapping is going to be used.
> 
> * Changes since v1
> - Add and use a dma_{map,unmap}_page_attrs to be able to map the page
>   using attributes DMA_ATTR_NO_KERNEL_MAPPING and
>   DMA_ATTR_SKIP_CPU_SYNC. Thanks Laurent.
> - Drop check if dmac is part of a iommu group or not, let the DMA
>   mapping api handle it.
> - Move slave configuration data around in rcar-dmac to avoid code
>   duplication.
> - Fix build issue reported by 'kbuild test robot' regarding phys_to_page
>   not availability on some configurations.
> - Add DT information for r8a7791.
> 
> * Changes since RFC
> - Switch to use the dma-mapping api instead of using the iommu_map()
>   directly. Turns out the dma-mapper is much smarter then me...
> - Dropped the patch to expose domain->ops->pgsize_bitmap from within the
>   iommu api.
> - Dropped the patch showing how I tested the RFC.
> 
> Niklas S?derlund (6):
>   dma-mapping: add {map,unmap}_resource to dma_map_ops
>   dma-debug: add support for resource mappings
>   dma-mapping: add dma_{map,unmap}_resource
>   arm: dma-mapping: add {map,unmap}_resource for iommu ops
>   dmaengine: rcar-dmac: group slave configuration
>   dmaengine: rcar-dmac: add iommu support for slave transfers
> 
>  Documentation/DMA-API.txt   |  22 +++++++--
>  arch/arm/mm/dma-mapping.c   |  63 ++++++++++++++++++++++++
>  drivers/dma/sh/rcar-dmac.c  | 116 +++++++++++++++++++++++++++++++++++---------
>  include/linux/dma-debug.h   |  19 ++++++++
>  include/linux/dma-mapping.h |  42 ++++++++++++++++
>  lib/dma-debug.c             |  52 +++++++++++++++++++-
>  6 files changed, 285 insertions(+), 29 deletions(-)
> 
> -- 
> 2.8.2
> 

-- 
Regards,
Niklas S?derlund