Re: [PATCHv7 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
On Thu, Jun 02, 2016 at 02:58:07PM +0200, Niklas Söderlund wrote: > Hi Vinod, > > On 2016-06-01 23:36:11 +0530, Vinod Koul wrote: > > On Wed, Jun 01, 2016 at 05:22:23PM +0200, Niklas Söderlund wrote: > > > Hi, > > > > > > [In this v7 series I have tried to address the questions raised by > > > Christoph > > > Hellwig and I hope it can awnser your concernes regarding dma-debug.] > > > > > > 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. > > > > The dmanegine bits looks okay to me. Btw how is the merge planned for this? > > Do you wnat this to be merged thru dmaengine tree or something else? > > Yes, since the arm specific patch are depending on other parts of the > series I was hoping to be able to get Russells Ack on it and then try to > get it all in through the dmaengine tree. Sounds good to me.. > If you see a better way I'm happy to do it that way, let me know what > you think. I hold off v8 that adresses the issues Russell brought up a > few days untill I know what you think is best. -- ~Vinod
Re: [PATCHv7 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
Hi Vinod, On 2016-06-01 23:36:11 +0530, Vinod Koul wrote: > On Wed, Jun 01, 2016 at 05:22:23PM +0200, Niklas Söderlund wrote: > > Hi, > > > > [In this v7 series I have tried to address the questions raised by > > Christoph > > Hellwig and I hope it can awnser your concernes regarding dma-debug.] > > > > 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. > > The dmanegine bits looks okay to me. Btw how is the merge planned for this? > Do you wnat this to be merged thru dmaengine tree or something else? Yes, since the arm specific patch are depending on other parts of the series I was hoping to be able to get Russells Ack on it and then try to get it all in through the dmaengine tree. If you see a better way I'm happy to do it that way, let me know what you think. I hold off v8 that adresses the issues Russell brought up a few days untill I know what you think is best. -- Regards, Niklas Söderlund
[PATCHv7 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
Hi,
[In this v7 series I have tried to address the questions raised by Christoph
Hellwig and I hope it can awnser your concernes regarding dma-debug.]
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.
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 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
(?) but 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
