Re: [Letux-kernel] [Bug]: mtd: onenand: omap2plus: kernel panic with OneNAND on OMAP3 (DM3730) device GTA04A5

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Wed, 11 Apr 2018 10:27:46 +0200
Ladislav Michl <ladis@linux-mips.org> wrote:

> On Wed, Apr 11, 2018 at 10:08:06AM +0200, Boris Brezillon wrote:
> > On Wed, 11 Apr 2018 09:36:56 +0200
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> >   
> > > Hi Boris,
> > > 
> > > On Wed, Apr 11, 2018 at 09:15:28AM +0200, Boris Brezillon wrote:  
> [...]
> > > > Not sure this approach is safe on all archs: if the cache is VIVT or
> > > > VIPT, you may have several entries pointing to the same phys page, and
> > > > then, when dma_map_page() does its cache maintenance operations, it's
> > > > only taking one of these entries into account.    
> > > 
> > > Hmm, I used the same approach Samsung OneNAND driver does since commit
> > > dcf08227e964a53a2cb39130b74842c7dcb6adde.
> > > Both TI OMAP3630 and Samsung S5PC110 are using Cortex-A8 which
> > > is VIPT. In that case samsung's driver code has the same problem.
> > >   
> > > > In other parts of the MTD subsystem, we tend to not do DMA on buffers
> > > > that have been vmalloc-ed.
> > > > 
> > > > You can do something like
> > > > 
> > > > 		if (virt_addr_valid(buf))
> > > > 			/* Use DMA */
> > > > 		else
> > > > 			/*
> > > > 			 * Do not use DMA, or use a bounce buffer
> > > > 			 * allocated with kmalloc
> > > > 			 */    
> > > 
> > > Okay, I'll use this approach then, but first I'd like to be sure above is
> > > correct. Anyone?  
> > 
> > See this discussion [1]. The problem came up a few times already, so
> > might find other threads describing why it's not safe.
> > 
> > [1]https://lists.linuxfoundation.org/pipermail/iommu/2016-March/016240.html  
> 
> Question was more likely whenever there might exist more that one mapping
> of the same page.

I'm clearly not an expert, so I might be wrong, but I think a physical
page can be both in the identity mapping and mapped in the vmalloc
space. Now, is there a real risk that both ends are accessed in
parallel thus making different cache entries pointing to the same phys
page dirty, I'm not sure. Or maybe there are other corner case, but
you'll have to ask Russell (or any other ARM expert) to get a clearer
answer.

> But okay, I'll disable DMA for highmem. Patch will follow.
> 
> 	ladis