linux-kernel - Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 21 May 2020 22:55:20 +0800
From:   Feng Tang <feng.tang@...el.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Serge Semin <fancer.lancer@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Grant Likely <grant.likely@...retlab.ca>,
        Vinod Koul <vkoul@...nel.org>, Alan Cox <alan@...ux.intel.com>,
        Linus Walleij <linus.walleij@...ricsson.com>,
        Georgy Vlasov <Georgy.Vlasov@...kalelectronics.ru>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Arnd Bergmann <arnd@...db.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rob Herring <robh+dt@...nel.org>, linux-mips@...r.kernel.org,
        devicetree@...r.kernel.org,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Clement Leger <cleger@...ray.eu>, linux-spi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the
 MID DMA

Hi Serge,

On Thu, May 21, 2020 at 02:47:36PM +0300, Serge Semin wrote:
> Hello Feng,
> 
> On Thu, May 21, 2020 at 11:09:24AM +0800, Feng Tang wrote:
> > Hi Serge,
> > 
> > On Thu, May 21, 2020 at 04:21:51AM +0300, Serge Semin wrote:
> 
> [nip]
> 
> > >  /*
> > >   * dws->dma_chan_busy is set before the dma transfer starts, callback for rx
> > >   * channel will clear a corresponding bit.
> > > @@ -200,6 +267,8 @@ static void dw_spi_dma_rx_done(void *arg)
> > >  {
> > >  	struct dw_spi *dws = arg;
> > >  
> > > +	dw_spi_dma_wait_rx_done(dws);
> > 
> > I can understand the problem about TX, but I don't see how RX
> > will get hurt, can you elaborate more? thanks
> > 
> > - Feng
> 
> Your question is correct. You are right with your hypothesis. Ideally upon the
> dw_spi_dma_rx_done() execution Rx FIFO must be already empty. That's why the
> commit log signifies the error being mostly related with Tx FIFO. But
> practically there are many reasons why Rx FIFO might be left with data:
> DMA engine failures, incorrect DMA configuration (if DW SPI or DW DMA driver
> messed something up), controller hanging up, and so on. It's better to catch
> an error at this stage while propagating it up to the SPI device drivers.
> Especially seeing the wait-check implementation doesn't gives us much of the
> execution overhead in normal conditions. So by calling dw_spi_dma_wait_rx_done()
> we make sure that all the data has been fetched and we may freely get the
> buffers back to the client driver.

I see your point about checking RX. But I still don't think checking
RX FIFO level is the right way to detect error. Some data left in
RX FIFO doesn't always mean a error, say for some case if there is
20 words in RX FIFO, and the driver starts a DMA request for 16
words, then after a sucessful DMA transaction, there are 4 words
left without any error.

Thanks,
Feng

> 
> -Sergey