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

3 messages, 2 authors, 2017-02-01 · open the first message on its own page

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

From: Chris.Brandt@renesas.com (Chris Brandt)
Date: 2017-02-01 15:57:26

As of commit 

8ef418c7178f "ARM: l2c: trial at enabling some Cortex-A9 optimisations"

from linux-3.16, my Renesas RZ/A1 (R7s72100) has never been able to use
its PL310 r3p2. It enables it fine, but then crashes later on in boot
because of corrupted memory.


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");
+		}


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+ */



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.


So I get that if I want to use this feature, I first need to get it
enabled in the L2C-310, then later on enabled it in the CA9. That
seems to be done by l2c310_starting_cpu() in today's kernel:

	if (aux & L310_AUX_CTRL_FULL_LINE_ZERO)
		cpuhp_setup_state(CPUHP_AP_ARM_L2X0_STARTING,
				  "arm/l2x0:starting", l2c310_starting_cpu,
				  l2c310_dying_cpu);


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"


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");
+		}

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,



(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?

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.
As in, this simple change will also allow me to boot:

 static int l2c310_starting_cpu(unsigned int cpu)
 {
-	set_auxcr(get_auxcr() | BIT(3) | BIT(2) | BIT(1));
+	set_auxcr(get_auxcr() | BIT(2) | BIT(1));
 	return 0;
 }


So, before I go firing off patches that will effect a bunch of systems, I
would like to understand if there is some logic here that I'm not seeing.


Thank you,
Chris

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

From: linux@armlinux.org.uk (Russell King - ARM Linux)
Date: 2017-02-01 17:43:11

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.

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

From: Chris.Brandt@renesas.com (Chris Brandt)
Date: 2017-02-01 18:23:36

Hello Russell,

Thank you for your reply. This at least fills in some missing info
for me.


On Wednesday, February 01, 2017, Russell King wrote:
quoted
(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?
quoted
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.
Hmmm, I guess the purpose of this witch-hunt is to find out if it is
SW or HW.

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.
Oh, it's an out-of-band signal. The PL310 documentation says:

"When the L2C-310 receives such a write transaction it ignores the AXI
attributes attached to the transaction, size, length, data, and strobes
for example, because the whole cache line must be reset. This behavior
is not compatible with the AXI protocol, it is disabled by default"

Which now I understand that it doesn't go across the AXI. Thank you for
clearing that up.

 
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.
Damn! I was afraid of that.

I'm going to take this up with the chip designers and confirm whether or
not this is the case.

quoted
(2) Bits 1 and 2 of the Auxiliary Control Register are "L1 Prefetch
enable"
quoted
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.
So if FLZ in the SoC is screwed up, I can at least keep prefetch.

quoted
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.
Well crap.
Assuming (c) is false on my system, obviously you probably don't want to go
hacking up cache-l2x0.c just for one bad apple.

Any thoughts of how I can fix it outside the driver?

Maybe I have to change from DT back to calling l2x0_init() directly,
then immediately after do a:

	set_auxcr(get_auxcr() & ~BIT(3));

and assume that no kzalloc calls will be made in between.


Chris
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help