Skip to content

dmaengine: rcar-dmac: add iommu support for slave transfers

Laurent Pinchart linux-arm-kernel linux-iommu linux-renesas-soc 3 versions New

Get series with b4
b4 am 2426718.0qODmfDriD@avalon

Versions newest first · linked by author and title

  1. v9 Latest posting 3/31 of 6 patches 0 repliesNew
  2. v9 posting 2/30 of 6 patches 0 repliesSuperseded
  3. v9 posting 1/34 of 6 patches 0 repliesSuperseded

1 of 6 patches is in this archive. The cover letter announces 6.

Patches in v9 posting 3/3 No applies recorded yet

#Subject and filesSizeRepliesApplied
6/6dmaengine: rcar-dmac: add iommu support for slave transfers+0 -01 reply

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v9 yet.

Read the v9 thread

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

Hi Robin,

Quoted 44 lines · On Friday 16 Sep 2016 13:49:21 Robin Murphy wrote:

On 16/09/16 13:05, Laurent Pinchart wrote:
[...]

Quoted 25 lines
Quoted 19 lines
Quoted 10 lines
Quoted 5 lines

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.

Quoted 8 lines

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

Quoted 5 lines

I think at that point we just create the channels as child devices of the main dmaengine device so they each get their own DMA ops, and can do whatever. The Qualcomm HIDMA driver already does that for a very similar reason (so that the IOMMU can map individual channels into different guest VMs).

Quoted 11 lines

That's another option, but it seems more like a workaround to me, instead of a proper solution to fix the more global problem of multiple memory paths within a single device. I have other hardware devices that can act as bus masters through different paths (for instance a display-related device that fetches data and commands through different paths). Luckily so far all those paths are served by the same IOMMU, but there's no guarantee this will remain true in the future. Furthermore, even today, the IOMMU connected to that device has the ability to selectively enable and disable its ports. I have to keep them all enabled due to the lack of channel information in the DMA mapping and IOMMU APIs, leading to increased power consumption.

Indeed, I think both the Exynos and Rockchip IOMMU drivers already do cater for a device mastering though multiple discrete IOMMUs, not being the fancy multi-port multi-context ones like yours and mine.

Quoted 7 lines

I guess what we could really do with is a decent abstraction of multi-master peripherals at the device level; a "threads within the same process" sort of granularity, as it were. I'd envisage it more along the lines of how we handle NUMA, i.e. dma_map_page_attrs(...) becomes a wrapper for dma_map_page_attrs_multi(..., CHANNEL_ALL), and trickier users can call the latter with the a more specific channel(s) argument (maybe it's a bitmask rather than an index).

That's pretty much what I've discussed with Arnd in the past, except that we were planning to add the channel to struct dma_attrs. Hence my disappointment seeing the structure go away.

Quoted 4 lines

Meanwhile, dev->archdata.dma_ops may point to a device-specific array of dma_map_ops, which the DMA API backend iterates over if necessary.

Strangely, that doesn't actually sound too horrible.

Signature · 3 lines
Regards,

Laurent Pinchart