RE: igb driver can cause cache invalidation of non-owned memory?

From: David Laight
Date: Thu Oct 13 2016 - 06:40:25 EST


From: Alexander Duyck
> Sent: 12 October 2016 19:30
> On Wed, Oct 12, 2016 at 11:12 AM, Nikita Yushchenko
> <nikita.yoush@xxxxxxxxxxxxxxxxxx> wrote:
> >> It would make more sense to update the DMA API for
> >> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if
> >> the direction is DMA_FROM_DEVICE.
> >
> > No, in generic case it's unsafe.
> >
> > If CPU issued a write to a location, and sometime later that location is
> > used as DMA buffer, there is danger that write is still in cache only,
> > and writeback is pending. Later this writeback can overwrite data
> > written to memory via DMA, causing corruption.
>
> Okay so if I understand it correctly then the invalidation in
> sync_for_device is to force any writes to be flushed out, and the
> invalidation in sync_for_cpu is to flush out any speculative reads.
> So without speculative reads then the sync_for_cpu portion is not
> needed. You might want to determine if the core you are using
> supports the speculative reads, if not you might be able to get away
> without having to do the sync_for_cpu at all.

For reads the sync_for_device invalidates (ie discards the contents of)
the cache lines to ensure the cpu won't write back dirty lines.
The sync_for_cpu invalidates the cache to ensure the cpu actually
reads from memory.

For writes you need to flush (write back) cache lines prior to the DMA.
I don't think anything is needed when the transfer finishes.

If headers/trailers might be added the sync_for_device must cover
the entire area the frame will be written to.
Clearly the sync_for_cpu need only cover the valid frame data.
I suspect you can safely skip the sync_for_device in the 'copybreak'
path but not if the frame is passed up the protocol stack.

David