[U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler

[Bin Meng]

Hi Stefan,

On Thu, Dec 13, 2018 at 9:51 PM Stefan Theil <Stefan.Theil at mixed-mode.de> wrote:
>
> > -----Ursprüngliche Nachricht-----
> > Von: Michal Simek [mailto:monstr at monstr.eu]
> > Gesendet: Donnerstag, 13. Dezember 2018 14:48
> > An: Stefan Theil; Bin Meng
> > Cc: U-Boot Mailing List
> > Betreff: Re: [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX
> > packet to receive handler
> >
> > On 13. 12. 18 14:41, Stefan Theil wrote:
> > >
> > >
> > >> -----Ursprüngliche Nachricht-----
> > >> Von: Bin Meng [mailto:bmeng.cn at gmail.com]
> > >> Gesendet: Donnerstag, 13. Dezember 2018 14:37
> > >> An: Stefan Theil
> > >> Cc: U-Boot Mailing List
> > >> Betreff: Re: Re: [U-Boot] [PATCH] zynq-gem: Flush cache before
> > >> handing RX packet to receive handler
> > >>
> > >> Hi Stefan,
> > >>
> > >> On Thu, Dec 13, 2018 at 9:26 PM Stefan Theil <Stefan.Theil at mixed-
> > >> mode.de> wrote:
> > >>>
> > >>> Hmm good question. I went with flush because that's what's done in
> > >>> the
> > >> transmit function:
> > >>>
> > >>> addr = (ulong) ptr;
> > >>> addr &= ~(ARCH_DMA_MINALIGN - 1);
> > >>> size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr,
> > >> addr
> > >>> + size);
> > >>>
> > >>> addr = (ulong)priv->rxbuffers;
> > >>> addr &= ~(ARCH_DMA_MINALIGN - 1);
> > >>> size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);
> > >>> flush_dcache_range(addr, addr + size); barrier();
> > >>>
> > >>> But since we actually want the uncached data invalidation seems
> > >>> logical. I
> > >> have to admit though, I don't have much experience with caches. This
> > >> patch completely fixed my problem... Maybe somebody with a bit more
> > >> expertise can add their opinion?
> > >>
> > >> It should be 'invalidate' primitive when it comes to the RX path. For
> > >> TX path, it should be 'flush'.
> > >>
> > >> Regards,
> > >> Bin
> > >
> > > Then why are all receive buffers flushed before sending a packet? In any
> > case, I'll try it with invalidate and submit an updated version.
> >
> > When you creating packet, cpu is used and caches are filled with data which
> > you are asking by DMA to send. That's why you need to flush them to DDR
> > for dma to take it. If you don't do that DMA can work with incorrect data.
>
> I know, but you don't create *receive* packets when you're sending a packet, are you? Why would you need to flush those before sending a packet? That's what I'm wondering about.
>

Why did you mention *receive* packets and sending a packet? The
receive packets and send packets are two different buffers.

> > On RX path if cpu touches that memory space before DMA receive data to
> > the memory, cpu can work with incorrect data.
> > That's why you need to invalidate that range to ensure that data which you
> > will read is that one got from DMA.
> >
> > It is kind of interesting that we didn't observe this issue.
>
> Probably because most people don't receive enough packets between sending packets, so the receive buffers are flushed often enough.

Regards,
Bin