Re: [PATCH 2/8] powerpc/dma: properly wire up the unmap_page and unmap_sg methods

[Christoph Hellwig <hch@lst.de>]

On Mon, Dec 17, 2018 at 07:41:54AM +0100, Christophe Leroy wrote:
>
>
> Le 16/12/2018 à 18:19, Christoph Hellwig a écrit :
>> The unmap methods need to transfer memory ownership back from the device
>> to the cpu by identical means as dma_sync_*_to_cpu.  I'm not sure powerpc
>> needs to do any work in this transfer direction, but given that it does
>> invalidate the caches in dma_sync_*_to_cpu already we should make sure
>> we also do so on unmapping.
>
> Why do we have to do that on unmapping ? If we are unmapping it means we 
> are retiring the area, so why would we need to use CPU for syncing an area 
> than won't be used anymore ?

So in general we need the cache maintaince only at map time if the CPUs
gurantee to never ѕpeculate into memory that might be under DMA, which
for modern CPUs is increasingly rate.  If the CPUs could speculate into
the area that was DMA mapped we need to do another round of cache
maintainance on unmap to make sure we really do not have any data
in the cache.  powerpc currently does that for dma_sync_*_to_cpu, but
not for the unmap case, which not only looks odd but also seems to be
worked around in drivers (see the ppc44x crypto patch).

Note that there are some way to optimize the amount of cache flushing
done even when the cpu speculates, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arc/mm/dma.c#n93

But for that I need help with people that actually understand the
non-coherent powerpc SOCs and who can test it.  For now this patch
is conservative.