On Monday 19 May 2014 16:56:08 Catalin Marinas wrote:
On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote:
On Monday 19 May 2014 10:03:40 Catalin Marinas wrote:
On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote:
The more important question is what happens to high buffers allocated elsewhere that get passed into dma_map_sg by a device driver. Depending on the DT properties of the device and its parents, this needs to do one of three things:
a) translate the 64-bit virtual address into a 64-bit bus address b) create an IOMMU entry for the 64-bit address and pass the 32-bit IOMMU address to the driver c) use the swiotlb code to create a bounce buffer at a 32-bit DMA address and copy the data around
It's definitely wrong to just hardcode a DMA mask in the driver because that code doesn't know which of the three cases is being used. Moreover, you can't do it using an #ifdef CONFIG_ARM64, because it's completely independent of the architecture, and we need to do the exact same logic on ARM32 and any other architecture.
I agree.
The problem we currently have is system topology description to pass the DMA mask and in a hierarchical way. I can see Santosh's patches introducing dma-ranges but the coherent dma mask still set as 32-bit. We can use the dma-ranges to infer a mask but that's only specific to the device and the driver doesn't know whether it goes through an iommu or not.
We definitely have to fix this very quickly, before people start building real arm64 systems and shipping them.
We should not merge any hacks that attempt to work around the problem, but try to come to a conclusion how to handle them properly. My hope was that we could just always set the dma mask to whatever the DT says it should be to keep the burden from device drivers, unless they want to restrict it further (e.g. when the specific peripheral hardware has a bug that prevents us from using high addresses, even though the SoC in theory supports it everywhere).
I agree.
Rob Herring argued that we should always mimic PCI and call dma_set_mask() in drivers but default to a 32-bit mask otherwise, independent of whether the hardware can do more or less than that, IIRC.
Can we not default to something built up from dma-ranges? Or 32-bit if dma-ranges property is missing?
We probably want to default to 32-bit for arm32 in the absence of dma-ranges. For arm64, I'd prefer if we could always mandate dma-ranges to be present for each bus, just like we mandate ranges to be present. I hope it's not too late for that.
dma_set_mask should definitely look at the dma-ranges properties, and the helper that Santosh just introduced should give us all the information we need. We just need to decide on the correct behavior.
While we currently don't have a set of swiotlb DMA ops on ARM32, we do have it on ARM64, and I think we should be using them properly. It should really not be hard to implement a proper dma_set_mask() function for ARM64 that gets is able to set up the swiotlb based on the dma-ranges properties and always returns success but leaves the mask unchanged.
The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise we don't have any guarantees. Since we can't honour random masks anyway, we stick to ZONE_DMA which is currently in the 4G limit. But the driver calls dma_set_mask() too late for any further swiotlb setup.
With IOMMU we can be more flexible around dma_set_mask(), can be done at run-time.
What we can do with swiotlb is to check if the mask is smaller than ZONE_DMA. If it ever is, we have to fail dma_set_mask and hope the driver can fall back to PIO mode or it will have to fail its probe() function.
For dma_set_coherent_mask(), we also have to fail any call that tries to set a mask larger than what the device hardware can do. Unlike that, dma_set_mask() can succeed with any mask, we just have to enable swiotlb if the mask that the driver wants is larger than what the hardware can do.
Arnd