[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