[RFC] ARM: l2c: Adding L310_AUX_CTRL_FULL_LINE_ZERO broke my system

[Russell King - ARM Linux]

On Wed, Feb 01, 2017 at 03:57:26PM +0000, Chris Brandt wrote:
> It basically boils down to these lines in the commit:
> 
> +		if (!(aux & L310_AUX_CTRL_FULL_LINE_ZERO) && !outer_cache.write_sec) {
> +			aux |= L310_AUX_CTRL_FULL_LINE_ZERO;
> +			pr_info("L2C-310 full line of zeros enabled for Cortex-A9\n");
> +		}

... which is correct.  It's saying

"If this is a Cortex-A9, FLZ was not enabled, and write_sec function is
not present, then enable FLZ.

> As reference, going into this line I have the following variable values:
> 
>   acr = 0x00000000
>   aux = 0x42020000
>   aux_cur = 0x00000000
>   outer_cache.write_sec = (null)
> 
> Also as reference from cache-l2x0.h:
> 
> #define L310_AUX_CTRL_FULL_LINE_ZERO            BIT(0)  /* R2P0+ */

Which is probably what everyone has.  This is not a feature that can be
enabled by the boot loader or firmware, it can only be done once the MMU
and caches are up and running.

> According to the PL310 documentation:
> 
> Full line of zero write
>  
> To enable this feature, perform the following steps:
> 1. enable Full line of zero feature in the L2C-310
> 2. turn on L2C-310
> 3. enable Full line of zero feature in A9.

... which we follow.

> However, in the code snippet from the patch, the "!" in the if statement
> is saying:
> 
>   "if you didn't want this feature, then we are turning it on"

No.  There's no "want this feature".  The bit is almost certainly
guaranteed to be zero there - the bit isn't there to request the
feature.

> Of course changing this line to the following works fine because zero fill
> will not get enabled.
> 
> +		if (aux & L310_AUX_CTRL_FULL_LINE_ZERO && !outer_cache.write_sec) {
> +			aux |= L310_AUX_CTRL_FULL_LINE_ZERO;
> +			pr_info("L2C-310 full line of zeros enabled for Cortex-A9\n");
> +		}

And so results with it always being off everywhere.

> So here's my questions:
> 
> (1) I see that the PL310 is enabled on multiple platforms, so how as this not
> broken any systems since it was introduced in 2014? What am I missing?
> 
> I see that only 2 platforms explicitly set this aux bit (both have CA9):
>   mach-tegra/tegra.c:	.l2c_aux_val	= 0x3c400001,
>   mach-exynos/exynos.c:	.l2c_aux_val	= 0x3c400001,

I think a better question is:

 * Why does enabling FLZ break your system.

It can only be that your system doesn't wire the L2C-310 up to the Cortex
A9 correctly, missing the out-of-band signals that inform the L2C-310 that
the Cortex A9 wants it to zero a full cache line.

BTW, the bit in the auxilary control register merely _enables_ the
reception of that signal at the L2C-310.  If the signal is permanently
deasserted, the L2C-310 does nothing with it, and the bit is a no-op.

Where it does matter is at the Cortex-A9.  That changes the behaviour of
the Cortex-A9 from issuing multiple bus cycles zeroing the cache line
word by word, to merely signalling that it wants the L2C-310 to zero the
cache line.

So, if _merely_ enabling the FLZ bit in the 310's auxiliary control
register _without_ setting the bit in the Cortex-A9 as well leads to
memory corruption, it definitely means the 310 in your SoC is miswired,
and we need a DT property to reflect that.

> (2) Bits 1 and 2 of the Auxiliary Control Register are "L1 Prefetch enable"
> and " L2 Prefetch hint enable". So if I want those enabled, it looks like
> I have to enable full-line-zero, correct?

Prefetching is entirely separate from FLZ.

> It seems the issue on my SoC is that when I enable FULL_LINE_ZERO in the CA9,
> it doesn't seem to work very well.

That will definitely lead to data corruption.  The Cortex-A9 must _never_
have FLZ enabled on the CPU without (a) a compatible L2 cache controller
and (b) the feature _already_ enabled on the L2 cache controller and (c)
the cache controller is correctly wired to the Cortex-A9.

It sounds to me like you have a rare (the only?) system where (c) is
false.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.