Messages in this thread | | | Date | Mon, 19 Mar 2018 18:01:41 +0000 | From | Catalin Marinas <> | Subject | Re: [PATCH 12/14] dma-direct: handle the memory encryption bit in common code |
| |
On Mon, Mar 19, 2018 at 05:03:43PM +0100, Christoph Hellwig wrote: > On Mon, Mar 19, 2018 at 03:48:33PM +0000, Will Deacon wrote: > > Why can't we just resolve the conflict by adding the underscores? > > We can solve the conflict easily that way. But that's not the point. > > The point is that I've been fighting hard to consolidate dma code > given that the behavior really is common and not arch specific. And > this one is another case like that: the fact that the non-coherent > dma boundary is bigger than the exposed size is something that can > easily happen elsewhere, so there is no need to duplicate a lot > of code for that.
I don't particularly like maintaining an arm64-specific dma-direct.h either but arm64 seems to be the only architecture that needs to potentially force a bounce when cache_line_size() > ARCH_DMA_MINALIGN and the device is non-coherent. Note that lib/swiotlb.c doesn't even deal with non-coherent DMA (e.g. map_sg doesn't have arch callbacks for cache maintenance), so not disrupting lib/swiotlb.c seems to be the least intrusive option.
> Nevermind that the commit should at least be three different patches: > > (1) revert the broken original commit > (2) increase the dma min alignment
Reverting the original commit could, on its own, break an SoC which expects ARCH_DMA_MINALIGN == 128. So these two should be a single commit (my patch only reverts the L1_CACHE_BYTES change rather than ARCH_DMA_MINALIGN, the latter being correct as 128).
Anyway, it's queued already and we try not to rebase the branches we published. Fix-ups on top are fine though.
> (3) put the swiotlb workaround in place
As I said above, adding a check in swiotlb.c for !is_device_dma_coherent(dev) && (ARCH_DMA_MINALIGN < cache_line_size()) feels too architecture specific. Adding yet another hook like arch_dma_capable() doesn't feel right either since we already have the possibility to override dma_capable() by selecting ARCH_HAS_PHYS_TO_DMA.
The "cleanest" I came up with for swiotlb.c was a new DMA_ATTR_FORCE_BOUNCE attribute. However, it required more changes to the arm64 dma-mapping.c than simply implementing an arch-specific dma_capable().
-- Catalin
| |