linux-kernel - Re: [RFC PATCH 2/2] mtd: devices: m25p80: Enable spi-nor bounce buffer support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170302162556.76b0ae8c@bbrezillon>
Date:   Thu, 2 Mar 2017 16:25:56 +0100
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Frode Isaksen <fisaksen@...libre.com>
Cc:     Vignesh R <vigneshr@...com>, Mark Brown <broonie@...nel.org>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-omap@...r.kernel.org, linux-spi@...r.kernel.org
Subject: Re: [RFC PATCH 2/2] mtd: devices: m25p80: Enable spi-nor bounce
 buffer support

On Thu, 2 Mar 2017 16:03:17 +0100
Frode Isaksen <fisaksen@...libre.com> wrote:

> On 02/03/2017 15:29, Boris Brezillon wrote:
> > On Thu, 2 Mar 2017 19:24:43 +0530
> > Vignesh R <vigneshr@...com> wrote:
> >  
> >>>>>>       
> >>>>> Not really, I am debugging another issue with UBIFS on DRA74 EVM (ARM
> >>>>> cortex-a15) wherein pages allocated by vmalloc are in highmem region
> >>>>> that are not addressable using 32 bit addresses and is backed by LPAE.
> >>>>> So, a 32 bit DMA cannot access these buffers at all.
> >>>>> When dma_map_sg() is called to map these pages by spi_map_buf() the
> >>>>> physical address is just truncated to 32 bit in pfn_to_dma() (as part of
> >>>>> dma_map_sg() call). This results in random crashes as DMA starts
> >>>>> accessing random memory during SPI read.
> >>>>>
> >>>>> IMO, there may be more undiscovered caveat with using dma_map_sg() for
> >>>>> non kmalloc'd buffers and its better that spi-nor starts handling these
> >>>>> buffers instead of relying on spi_map_msg() and working around every
> >>>>> time something pops up.
> >>>>>    
> >>>> Ok, I had a closer look at the SPI framework, and it seems there's a
> >>>> way to tell to the core that a specific transfer cannot use DMA
> >>>> (->can_dam()). The first thing you should do is fix the spi-davinci
> >>>> driver:
> >>>>
> >>>> 1/ implement ->can_dma()
> >>>> 2/ patch davinci_spi_bufs() to take the decision to do DMA or not on a
> >>>>    per-xfer basis and not on a per-device basis
> >>>>    
> >> This would lead to poor perf defeating entire purpose of using DMA.  
> > Hm, that's not really true. For all cases where you have a DMA-able
> > buffer it would still use DMA. For other cases (like the UBI+SPI-NOR
> > case we're talking about here), yes, it will be slower, but slower is
> > still better than buggy.
> > So, in any case, I think the fixes pointed by Frode are needed.  
> Also, I think the UBIFS layer only uses vmalloc'ed buffers during
> mount/unmount and not for read/write, so the performance hit is not
> that big.

It's a bit more complicated than that. You may have operations running
in background that are using those big vmalloc-ed buffers at runtime.
To optimize things, we really need to split LEB/PEB buffers into
multiple ->max_write_size (or ->min_io_size) kmalloc-ed buffers.

> In most cases the buffer is the size of the erase block, but I've seen
> vmalloc'ed buffer of size only 11 bytes ! So, to optimize this, the
> best solution is probably to change how the UBIFS layer is using
> vmalloc'ed vs kmalloc'ed buffers, since vmalloc'ed should only be used
> for large (> 128K) buffers.

Hm, the buffer itself is bigger than 11 bytes, it's just that the
same buffer is used in different use cases, and sometime we're only
partially filling it.