Russell,

On Mon, 12 Jan 2015 12:36:13 +0000, Russell King - ARM Linux wrote:

On Sat, Jan 10, 2015 at 08:57:21PM +0100, Thomas Petazzoni wrote:

quoted

We will indeed need to do more extensive testing and review. However, I
don't agree that this should prevent this patch from going to stable:
the current situation in the kernel (and also past kernels) is known to
be broken: DMA coherent mappings allocated by dma_alloc_coherent() are
*not* coherent in the current situation. Writes made by the device to
the memory are not guaranteed to be immediately visible to the CPU,
unless an explicit I/O sync barrier is done, which obviously is never
done for DMA coherent mappings since those are assumed by Linux to be
coherent, and therefore not require any cache maintenance operation.

That's actually an incorrect statement.

On all ARMv6+ where DMA coherent memory is "normal memory, non-cached,
write combine" but because it's "normal memory", memory barriers are
required.  This is why we have the memory barriers in readl() and
writel().

Except that a readl() or writel() do *not* imply the I/O
synchronization barrier that is needed on I/O coherent Marvell CPUs to
ensure that the CPU sees the changes made by a DMA master to the
memory. At least in the current kernel code, at least.

One option would maybe have been to add this I/O synchronization
barrier in the memory barrier implementation, but it seems a lot
simpler to rely on the automatic I/O synchronization mechanism instead.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com