Re: [U-Boot] [PATCH] mx6: Set shared override bit in PL310 AUX_CTRL register

2015-03-12 Thread Catalin Marinas
On Wed, Mar 11, 2015 at 08:12:12PM +, Fabio Estevam wrote:
 From: Fabio Estevam fabio.este...@freescale.com
 
 Having bit 22 cleared in the PL310 Auxiliary Control register (shared
 attribute override enable) has the side effect of transforming Normal
 Shared Non-cacheable reads into Cacheable no-allocate reads.
 
 Coherent DMA buffers in Linux always have a Cacheable alias via the
 kernel linear mapping and the processor can speculatively load cache
 lines into the PL310 controller. With bit 22 cleared, Non-cacheable
 reads would unexpectedly hit such cache lines leading to buffer
 corruption.
 
 This was inspired by a patch from Catalin Marinas [1] and also from recent 
 discussions in the linux-arm-kernel list [2] where Russell King and Rob 
 Herring 
 suggested that bootloaders should initialize the cache. 
 
 [1] 
 http://lists.infradead.org/pipermail/linux-arm-kernel/2010-November/031810.html
 [2] https://lkml.org/lkml/2015/2/20/199
 
 Signed-off-by: Fabio Estevam fabio.este...@freescale.com

Acked-by: Catalin Marinas catalin.mari...@arm.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6: Set shared override bit in PL310 AUX_CTRL register

2015-03-12 Thread Catalin Marinas
On Thu, Mar 12, 2015 at 01:04:31AM +, Russell King - ARM Linux wrote:
 On Wed, Mar 11, 2015 at 05:12:12PM -0300, Fabio Estevam wrote:
  From: Fabio Estevam fabio.este...@freescale.com
  
  Having bit 22 cleared in the PL310 Auxiliary Control register (shared
  attribute override enable) has the side effect of transforming Normal
  Shared Non-cacheable reads into Cacheable no-allocate reads.
  
  Coherent DMA buffers in Linux always have a Cacheable alias via the
  kernel linear mapping and the processor can speculatively load cache
  lines into the PL310 controller.
 
 No, this is wrong.  They do not.  CMA remaps pages to be non-cacheable
 rather than the old technique where the above statement was true.
 
 There's some corner cases which make that less effective than it once
 was, and as I've already said, those need to be fixed.  The reason
 that these were missed is because all the ARM CMA work bypassed me -
 CMA on ARM has had zero review from the point of view of the ARM
 architecture, so it's not surprising it gets stuff like this wrong.
 
 Once that's fixed, setting bit 22 is not necessary.

And I strongly disagree with your statement. Seriously, there are so
many assumption about when this bit will no longer be required like the
platform always using CMA, having fixed the CMA code in Linux. That's a
boot loader patch and even though it's used mostly (only) to load Linux,
we should not make this assumption.

Most importantly, not setting this bit makes your SoC non-compliant with
the ARM ARM clarifications on mismatched aliases. It was a hardware
mistake to have it cleared out of reset but just let firmware or
boot-loaders deal with it.

(there are some very specific use-cases for this bit that the hw guys
had in mind but none of them apply to Linux)

-- 
Catalin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot