I've observed kernel crashes on Sequoia board several times.
Kernel often crashes with high network/memory loads.
This is due to 440EPx h/w issue.
According to the errata:
CHIP_8: Incorrect Write to DDR SDRAM.
Category: 3
Overview:
The write address can be corrupted during writes to DDR SDRAM when write
pipelining is enabled on PLB0. The probability of encountering the
problem increases with the amount of back-pressure applied by the SDRAM.
Impact: Data can be written to the wrong address in SDRAM.
Workaround:
Disable write pipelining to DDR SDRAM by setting PLB4A0_ACR[WRP] = 0.
Only primary write transfers are broadcast and the write address
corruption is avoided.
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
Thanks,
Valentine.
I've observed kernel crashes on Sequoia board several times.
Kernel often crashes with high network/memory loads.
This is due to 440EPx h/w issue.
According to the errata:
CHIP_8: Incorrect Write to DDR SDRAM.
Category: 3
Overview:
The write address can be corrupted during writes to DDR SDRAM when write
pipelining is enabled on PLB0. The probability of encountering the
problem increases with the amount of back-pressure applied by the SDRAM.
Impact: Data can be written to the wrong address in SDRAM.
Workaround:
Disable write pipelining to DDR SDRAM by setting PLB4A0_ACR[WRP] = 0.
Only primary write transfers are broadcast and the write address
corruption is avoided.
Ew. That sounds nasty.
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
Hm, do you happen to know if AMCC is going to release a newer revision
of 440EPx with that fixed? If so, we should probably do it
conditionally based off of PVR.
As for where to do it, I'd rather it be in sequoia_setup_arch() than
the bootwrapper. Eventually bootloaders like U-Boot will take a raw
vmlinux so the wrapper won't even be there anymore.
I don't see any reason to use the dcr_map stuff either. Just check the
PVR, check if the PLB is already set up correctly, and if not set it
with the regular mfdcr/mtdcr functions. And a comment about why that's
being done would be good.
That's my $0.02 anyway.
josh
From: Stefan Roese <sr@denx.de> Date: 2007-09-19 19:19:47
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
Best regards,
Stefan
From: Olof Johansson <hidden> Date: 2007-09-19 19:28:19
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
-Olof
On Wed, 19 Sep 2007 14:30:24 -0500
Olof Johansson [off-list ref] wrote:
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
quoted
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
On Wed, 19 Sep 2007 14:30:24 -0500
Olof Johansson [off-list ref] wrote:
quoted
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
quoted
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
Yes, agreed.
I was thinking about it. Looks like it's the best place, but the code
that actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Thanks,
Valentine.
From: Olof Johansson <hidden> Date: 2007-09-20 17:22:52
On Thu, Sep 20, 2007 at 08:56:32PM +0400, Valentine Barshak wrote:
I was thinking about it. Looks like it's the best place, but the code that
actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Sounds like something that went wrong at the merge of ppc and ppc64.
Take out the ifdef, even if there's fallout we should deal with it
instead of adding more complex ifdefs.
-Olof
On Thu, 20 Sep 2007 12:25:06 -0500
Olof Johansson [off-list ref] wrote:
On Thu, Sep 20, 2007 at 08:56:32PM +0400, Valentine Barshak wrote:
quoted
I was thinking about it. Looks like it's the best place, but the code that
actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Sounds like something that went wrong at the merge of ppc and ppc64.
Take out the ifdef, even if there's fallout we should deal with it
instead of adding more complex ifdefs.
Yeah. Looks like BenH did this in commit:
42c4aaadb737e0e672b3fb86b2c41ff59f0fb8bc
Ben, any reason you ifdef'd it for ppc64?
josh
From: Milton Miller <hidden> Date: 2007-09-20 17:38:31
On Fri Sep 21 02:56:32 EST 2007, Valentine Barshak wrote:
Josh Boyer wrote:
quoted
On Wed, 19 Sep 2007 14:30:24 -0500
Olof Johansson <olof at lixom.net> wrote:
quoted
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
quoted
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
Yes, agreed.
I was thinking about it. Looks like it's the best place, but the code
that actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
head_32.S calls call_setup_cpu in misc_32.S to call the cpu setup functon.
Note that these functions are called before the kernel is copied down to
0, so on ppc32 you will need the PTRRELOC type stuff. Also the callsite
implies that the cpu number is available in r24, which may or may not be
true when called from C.
Its probably easier to just call call_setup_cpu in the other 32 bit
head_xxx files.
milton
From: Olof Johansson <hidden> Date: 2007-09-20 18:01:03
On Thu, Sep 20, 2007 at 12:25:06PM -0500, Olof Johansson wrote:
On Thu, Sep 20, 2007 at 08:56:32PM +0400, Valentine Barshak wrote:
quoted
I was thinking about it. Looks like it's the best place, but the code that
actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Sounds like something that went wrong at the merge of ppc and ppc64.
Take out the ifdef, even if there's fallout we should deal with it
instead of adding more complex ifdefs.
See comment from Milton, my bad. It's just called from head_32.S right now.
It'll need to be added to the 4xx head as well, or an ifdef added. One or
the other, I'll leave it to Josh to pick.
-Olof
This adds cpu_setup functionality to PowerPC 44x platform.
The cpu_setup callback is invoked by head_32 code and
the identify_cpu() function at early init and is used to
initialize FPU on 440EP(x) processors. The FPU initialization
was previously done in head_44x.S. Also a workaround for
the incorrect write to DDR SDRAM 440EPx/440GRx errata added.
Data can be written to wrong address in SDRAM when write
pipelining is enabled on plb0. The setup_cpu function
for these processors disables write pipelining.
Signed-off-by: Valentine Barshak <redacted>
---
arch/powerpc/kernel/Makefile | 1
arch/powerpc/kernel/cpu_setup_44x.S | 54 ++++++++++++++++++++++++++++++++++++
arch/powerpc/kernel/cputable.c | 25 ++++++++++------
arch/powerpc/kernel/head_44x.S | 10 ------
4 files changed, 70 insertions(+), 20 deletions(-)
diff -ruN linux-2.6.orig/arch/powerpc/kernel/cpu_setup_44x.S linux-2.6/arch/powerpc/kernel/cpu_setup_44x.S
On Thu, 20 Sep 2007 12:32:56 -0500 (CDT)
Milton Miller [off-list ref] wrote:
On Fri Sep 21 02:56:32 EST 2007, Valentine Barshak wrote:
quoted
Josh Boyer wrote:
quoted
On Wed, 19 Sep 2007 14:30:24 -0500
Olof Johansson <olof at lixom.net> wrote:
quoted
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
quoted
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
Yes, agreed.
I was thinking about it. Looks like it's the best place, but the code
that actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
head_32.S calls call_setup_cpu in misc_32.S to call the cpu setup functon.
Note that these functions are called before the kernel is copied down to
0, so on ppc32 you will need the PTRRELOC type stuff. Also the callsite
implies that the cpu number is available in r24, which may or may not be
true when called from C.
Its probably easier to just call call_setup_cpu in the other 32 bit
head_xxx files.
Hm. I'll have to see how well that would work for 4xx. Seems 8xx and
FSL BookE are in a similar situation.
josh
This adds cpu_setup functionality to PowerPC 44x platform.
The cpu_setup callback is invoked by head_32 code and
the identify_cpu() function at early init and is used to
initialize FPU on 440EP(x) processors. The FPU initialization
was previously done in head_44x.S. Also a workaround for
the incorrect write to DDR SDRAM 440EPx/440GRx errata added.
Data can be written to wrong address in SDRAM when write
pipelining is enabled on plb0. The setup_cpu function
for these processors disables write pipelining.
Wow, you're fast ;)
I'd like to see this split up a bit so there's not so much in one
patch. Essentially, introduce cpu_setup_44x.S, move the FPU init to
use it, and then add the PLB workaround.
I still need to look at the code too :)
josh
On Thu, 20 Sep 2007 12:32:56 -0500 (CDT)
Milton Miller [off-list ref] wrote:
quoted
On Fri Sep 21 02:56:32 EST 2007, Valentine Barshak wrote:
quoted
Josh Boyer wrote:
quoted
On Wed, 19 Sep 2007 14:30:24 -0500
Olof Johansson <olof at lixom.net> wrote:
quoted
On Wed, Sep 19, 2007 at 09:19:47PM +0200, Stefan Roese wrote:
quoted
Hi Valentine,
On Wednesday 19 September 2007, Valentine Barshak wrote:
quoted
Disabling write pipelining really helps.
Josh, David, what is the right place to put this workaround to?
Is it OK to do mtdcr(PLB4A0_ACR, mfdcr(PLB4A0_ACR) & ~PLB4_WRP); in
arch/powerpc/boot/cuboot-sequoia.c:sequoia_fixups()?
or
should this be done in
arch/powerpc/platforms/44x/sequoia.c:sequoia_setup_arch()
with dcr_map, dcr_read/write stuff?
I vote for putting it into sequoia.c, since it's very likely to happen that
Sequoia will at one point be booted without the bootwrapper. Or perhaps it
should go into some common code checking the PVR and disabling it when this
440EPx/GRx is detected, since all those boards are affected.
This is what we have setup_cpu functions in the cpu table for. Please
put it there instead of in board code.
Yes, agreed.
I was thinking about it. Looks like it's the best place, but the code
that actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
head_32.S calls call_setup_cpu in misc_32.S to call the cpu setup functon.
Note that these functions are called before the kernel is copied down to
0, so on ppc32 you will need the PTRRELOC type stuff. Also the callsite
implies that the cpu number is available in r24, which may or may not be
true when called from C.
Its probably easier to just call call_setup_cpu in the other 32 bit
head_xxx files.
Hm. I'll have to see how well that would work for 4xx. Seems 8xx and
FSL BookE are in a similar situation.
I started preparing the patch after Olof's "take out the ifdef" :)
I've tested it on 4xx. seems to work fine. FPU works OK and EPX/GRX
workaround is fine also, but it has to be tested on 8xx and fsl.
Adding call_setup_cpu to head_44x is no problem.
This adds cpu_setup functionality to PowerPC 44x platform.
The cpu_setup callback is invoked by head_32 code and
the identify_cpu() function at early init and is used to
initialize FPU on 440EP(x) processors. The FPU initialization
was previously done in head_44x.S. Also a workaround for
the incorrect write to DDR SDRAM 440EPx/440GRx errata added.
Data can be written to wrong address in SDRAM when write
pipelining is enabled on plb0. The setup_cpu function
for these processors disables write pipelining.
Wow, you're fast ;)
I'd like to see this split up a bit so there's not so much in one
patch. Essentially, introduce cpu_setup_44x.S, move the FPU init to
use it, and then add the PLB workaround.
@@ -1318,18 +1327,14 @@ for (i = 0; i < ARRAY_SIZE(cpu_specs); i++,s++) if ((pvr & s->pvr_mask) == s->pvr_value) {+ cpu_setup_t setup_func = PTRRELOC(s->cpu_setup);+ *cur = cpu_specs + i;-#ifdef CONFIG_PPC64- /* ppc64 expects identify_cpu to also call setup_cpu- * for that processor. I will consolidate that at a- * later time, for now, just use our friend #ifdef.- * we also don't need to PTRRELOC the function pointer- * on ppc64 as we are running at 0 in real mode.+ /* ppc expects identify_cpu to also call setup_cpu+ * for that processor. */- if (s->cpu_setup) {- s->cpu_setup(offset, s);- }-#endif /* CONFIG_PPC64 */+ if (setup_func)+ setup_func(offset, s); return s; }
This should just be something like:
#if defined(CONFIG_PPC64) || defined(CONFIG_BOOKE)
if (s->cpu_setup)
...
#endif
we know reloc_offset is always 0 in book-e, plus fixup the comment.
- k
@@ -1318,18 +1327,14 @@ for (i = 0; i < ARRAY_SIZE(cpu_specs); i++,s++) if ((pvr & s->pvr_mask) == s->pvr_value) {+ cpu_setup_t setup_func = PTRRELOC(s->cpu_setup);+ *cur = cpu_specs + i;-#ifdef CONFIG_PPC64- /* ppc64 expects identify_cpu to also call setup_cpu- * for that processor. I will consolidate that at a- * later time, for now, just use our friend #ifdef.- * we also don't need to PTRRELOC the function pointer- * on ppc64 as we are running at 0 in real mode.+ /* ppc expects identify_cpu to also call setup_cpu+ * for that processor. */- if (s->cpu_setup) {- s->cpu_setup(offset, s);- }-#endif /* CONFIG_PPC64 */+ if (setup_func)+ setup_func(offset, s); return s; }
This should just be something like:
#if defined(CONFIG_PPC64) || defined(CONFIG_BOOKE)
if (s->cpu_setup)
...
#endif
we know reloc_offset is always 0 in book-e, plus fixup the comment.
- k
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2007-09-23 08:21:28
On Thu, 2007-09-20 at 12:29 -0500, Josh Boyer wrote:
On Thu, 20 Sep 2007 12:25:06 -0500
Olof Johansson [off-list ref] wrote:
quoted
On Thu, Sep 20, 2007 at 08:56:32PM +0400, Valentine Barshak wrote:
quoted
I was thinking about it. Looks like it's the best place, but the code that
actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Sounds like something that went wrong at the merge of ppc and ppc64.
Take out the ifdef, even if there's fallout we should deal with it
instead of adding more complex ifdefs.
Yeah. Looks like BenH did this in commit:
42c4aaadb737e0e672b3fb86b2c41ff59f0fb8bc
Ben, any reason you ifdef'd it for ppc64?
I'll have to check on monday what's up there, but isn't setup_cpu called
from a different place on 32 bits? There are some subtle difference with
the way the cpu feature stuff is initialized /done between 32 and 64
bits that we haven't fully reconciled yet.
Ben.
On Thu, 2007-09-20 at 12:29 -0500, Josh Boyer wrote:
quoted
On Thu, 20 Sep 2007 12:25:06 -0500
Olof Johansson [off-list ref] wrote:
quoted
On Thu, Sep 20, 2007 at 08:56:32PM +0400, Valentine Barshak wrote:
quoted
I was thinking about it. Looks like it's the best place, but the code that
actually calls setup_cpu is under ifdef CONFIG_PPC64, while lots of
cpu_setup functions are defined for ppc32 processors.
Is it OK to remove this ifdef, or should I do CONFIG_PPC64 || CONFIG_44x?
Sounds like something that went wrong at the merge of ppc and ppc64.
Take out the ifdef, even if there's fallout we should deal with it
instead of adding more complex ifdefs.
Yeah. Looks like BenH did this in commit:
42c4aaadb737e0e672b3fb86b2c41ff59f0fb8bc
Ben, any reason you ifdef'd it for ppc64?
I'll have to check on monday what's up there, but isn't setup_cpu called
from a different place on 32 bits? There are some subtle difference with
the way the cpu feature stuff is initialized /done between 32 and 64
bits that we haven't fully reconciled yet.
Ben.
From what I've seen, setup_cpu is never called for BOOKE.
Currently It's called from cputable.c for PPC64 and from head_32.S for 6xx.
Thanks,
Valentine.