[PATCH] ARM: decompressor: clear SCTLR.A bit for v7 cores

[nicolas.pitre@linaro.org (Nicolas Pitre)]

On Thu, 11 Oct 2012, Russell King - ARM Linux wrote:

> On Thu, Oct 11, 2012 at 08:31:47AM -0500, Rob Herring wrote:
> > On 10/11/2012 08:09 AM, Russell King - ARM Linux wrote:
> > > On Thu, Oct 11, 2012 at 07:43:22AM -0500, Rob Herring wrote:
> > >> The contents of this were already reviewed on this thread, so I sent this
> > >> to the patch system and this was Russell's reply:
> > > 
> > > So that's why I couldn't find it - the mailing list thread has a different
> > > subject line to the patch.  Don't do that.  Given the amount of list
> > > traffic we have today, that's as good as not having been posted at all.
> > > 
> > >>> NAK for two reasons.
> > >>>
> > >>> 1. It hasn't been on the list (I can't find a match for "clear SCTLR.A"
> > >>> in my mailbox)
> > >>>
> > >>> 2. The behaviour of unaligned accesses vary depending on CPU.  Some
> > >>> fix-up the access, others load the word and then rotate it.  If we have
> > >>> decompressors which perform unaligned accesses, we need to fix this
> > >>> properly to avoid the CPU specific behaviour, rather than tweaking
> > >>> control bits to hide the problem.
> > >>
> > >> I'm simply matching the behavior of the kernel itself. The A bit is cleared
> > >> for v7 kernels and compilers only generate unaligned accesses for v7.
> > >> Without this the initial state of the A bit is undefined as a bootloader
> > >> could have cleared it already. We should document the required state or set
> > >> it to what we want.
> > > 
> > > Irrespective of this, (2) still stands.  Unaligned accesses in the
> > > decompressor without a fixup (which will be very hard to provide)
> > > will return different data depending on the CPU as I mention in point
> > > 2.
> > 
> > This only affects v7 cores. It should not vary for v7 cores as unaligned
> > access is a required feature. So how is it going to vary on v7 CPUs?
> > We've got bigger problems if there are v7 cores that don't handle
> > unaligned accesses.
> 
> Rob,
> 
> Your patch may only affect v7 cores, but you've raised the issue of the
> decompressor performing unaligned accesses in general.  Shall I re-repeat
> my point over that or is the problem here going to finally sink in?

The decompressor is not performing direct unaligned accesses.  It uses 
the get_unaligned() and put_unaligned() accessors.  That means that 
we're in control of how this is happening.

So let's talk about the how.  On pre ARMv7, those accesses are performed 
with a series of byte accesses.  When compiling for ARMv7, gcc knows and 
that the hardware can do unaligned accesses, and it does optimize its 
output by using ldr/str instructions.  But the A bit has to be cleared 
in that case, and only in that case.  This is why the patch clears the A 
bit only for ARMv7.

So this patch is only setting up the hardware to match gcc's 
expectations when generating code from the use of get_unaligned() and 
put_unaligned() when optimizing for ARMv7.

As always, any code doing unaligned access and _not_ using those 
accessors is broken.


Nicolas