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

Hi Arnd,

On Friday 16 Sep 2016 14:02:35 Arnd Bergmann wrote:

On Friday, September 16, 2016 12:48:23 PM CEST Laurent Pinchart wrote:

quoted

On Friday 16 Sep 2016 11:07:48 Arnd Bergmann wrote:

quoted

On Thursday, September 15, 2016 9:56:51 PM CEST Vinod Koul wrote:

quoted

On Wed, Aug 10, 2016 at 11:07:10PM +0530, Vinod Koul wrote:

quoted

On Wed, Aug 10, 2016 at 01:22:13PM +0200, Niklas S?derlund wrote:

I had not looked at the series earlier, but this version looks entirely
reasonable to me, so

Acked-by: Arnd Bergmann <arnd@arndb.de>


One concern I have is that we might get an awkward situation if we ever
encounter one DMA engine hardware that is used in different systems that
all have an IOMMU, but on some of them the connection between the DMA
master and the slave FIFO bypasses the IOMMU while on others the IOMMU
is required.

Do you mean systems where some of the channels of a specific DMA engine go
through the IOMMU while others do not ? We indeed have no solution today
for such a situation.

I wasn't thinking quite that far, though that is also a theoretical
problem. However, the simple solution would be to have a bit in the DMA
specifier let the driver know whether translation is needed or not.

The simpler case I was thinking of is where the entire DMA engine
either goes through an IOMMU or doesn't (depending on the integration
into the SoC), so we'd have to find out through some DT property
or compatible string in the DMA enginen driver.

Don't we already get that information from the iommus DT property ? If the DMA 
engine goes through an IOMMU the property will be set, otherwise it will not.

quoted

The problem is a bit broader than that, we'll also have an issue with DMA
engines that have different channels served by different IOMMUs.

Do you mean a theoretical problem, or a chip that you already know exists?

That's theoretical. The problem I'm facing today is a DMA engine whose 
channels are served by different ports of the same IOMMU. This works in a 
suboptimal way because I have to keep all the IOMMU ports enabled regardless 
of whether they're used or not, as the DMA engine and IOMMU APIs don't carry 
channel information.

quoted

I recall discussing this in the past with you, and the solution you
proposed was to add a channel index to struct dma_attrs seems good to me.
To support the case where some channels don't go through an IOMMU we would
only need support for null entries in the IOMMUs list associated with a
device (for instance in the DT case null entries in the iommus property).

Now I see that struct dma_attrs has been replaced by unsigned long in

commit 00085f1efa387a8ce100e3734920f7639c80caa3
Author: Krzysztof Kozlowski [off-list ref]
Date:   Wed Aug 3 13:46:00 2016 -0700

    dma-mapping: use unsigned long for dma_attrs

We still have enough bits to reserve some of them for a channel number,
but I'm not very happy with that patch as I can see how a future proposal
to handle the channel number through the DMA attributes will get rejected
on the grounds of bits starvation then :-(

Agreed, that can become interesting.

Does the above-mentioned patch really fix a performance, memory consumption or 
other issue ?

-- 
Regards,

Laurent Pinchart