[U-Boot] CPU POST on MPC5200 fails with U-Boot built using GCC 4.6.1
Hi,
I noticed a problem while testing Wolfgang's patch
PPC: fix "Warning: FOO uses hard float, BAR uses soft float".
U-Boot built for TQM5200 using ELDK-5.1 powerpc toolchain seems to have issues in CPU POST test. I see similar problem both with and also without Wolfgang's patch applied:
U-Boot 2011.12-rc3-00002-gaf44f4b (Dec 23 2011 - 00:08:10)
CPU: MPC5200 v1.2, Core v1.1 at 396 MHz Bus 132 MHz, IPB 132 MHz, PCI 66 MHz Board: TQM5200 (TQ-Components GmbH) on a STK52xx carrier board I2C: 85 kHz, ready DRAM: 64 MiB Flash: 32 MiB In: serial Out: serial Err: serial Net: FEC POST i2c PASSED POST cpu Error at multi test ! FAILED ^^^^^^
IDE: Bus 0: OK Device 0: Model: 32MB CHH Firm: Rev 1.01 Ser#: 403303C48A30730031 Type: Removable Hard Disk Capacity: 30.5 MB = 0.0 GB (62592 x 512) Device 1: not available SRAM: 512 kB VGA: SMI501 (Voyager) with 8 MB PS/2: No device found Kbd: reset failed, no ACK
Type run flash_nfs to mount root filesystem over NFS
Anatolij
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
--- post/lib_powerpc/multi.c | 54 +++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/post/lib_powerpc/multi.c b/post/lib_powerpc/multi.c index 5845616..4b4b119 100644 --- a/post/lib_powerpc/multi.c +++ b/post/lib_powerpc/multi.c @@ -38,45 +38,41 @@
#if CONFIG_POST & CONFIG_SYS_POST_CPU
-extern void cpu_post_exec_02 (ulong *code, ulong op1, ulong op2); +extern void cpu_post_exec_02(ulong * code, ulong op1, ulong op2);
-int cpu_post_test_multi (void) +int cpu_post_test_multi(void) { - int ret = 0; - unsigned int i; - int flag = disable_interrupts(); + int ret = 0; + unsigned int i; + int flag = disable_interrupts();
- if (ret == 0) - { - ulong src [26], dst [26]; + if (ret == 0) { + ulong src[26], dst[26];
- ulong code[] = - { - ASM_LMW(5, 3, 0), - ASM_STMW(5, 4, 0), - ASM_BLR, - }; + ulong code[] = { + ASM_LMW(5, 3, 0), + ASM_STMW(5, 4, 0), + ASM_BLR, + };
- for (i = 0; i < ARRAY_SIZE(src); ++i) - { - src[i] = i; - dst[i] = 0; - } + for (i = 0; i < ARRAY_SIZE(src); ++i) { + src[i] = i; + dst[i] = 0; + }
- cpu_post_exec_02(code, (ulong)src, (ulong)dst); + cpu_post_exec_02(code, (ulong) src, (ulong) dst);
- ret = memcmp(src, dst, sizeof(dst)) == 0 ? 0 : -1; - } + ret = memcmp(src, dst, sizeof(dst)) == 0 ? 0 : -1; + }
- if (ret != 0) - { - post_log ("Error at multi test !\n"); - } + if (ret != 0) { + post_log("Error at multi test !\n"); + }
- if (flag) - enable_interrupts(); + if (flag) + enable_interrupts();
- return ret; + return ret; }
#endif
Clean up and document the code:
- get rid of unneeded code block - add comment which code is generated
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com --- post/lib_powerpc/multi.c | 27 ++++++++++++--------------- 1 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/post/lib_powerpc/multi.c b/post/lib_powerpc/multi.c index 4b4b119..b8619de 100644 --- a/post/lib_powerpc/multi.c +++ b/post/lib_powerpc/multi.c @@ -44,26 +44,23 @@ int cpu_post_test_multi(void) { int ret = 0; unsigned int i; + ulong src[26], dst[26]; int flag = disable_interrupts();
- if (ret == 0) { - ulong src[26], dst[26]; + ulong code[] = { + ASM_LMW(5, 3, 0), /* lmw r5, 0(r3) */ + ASM_STMW(5, 4, 0), /* stmr r5, 0(r4) */ + ASM_BLR, /* blr */ + };
- ulong code[] = { - ASM_LMW(5, 3, 0), - ASM_STMW(5, 4, 0), - ASM_BLR, - }; - - for (i = 0; i < ARRAY_SIZE(src); ++i) { - src[i] = i; - dst[i] = 0; - } + for (i = 0; i < ARRAY_SIZE(src); ++i) { + src[i] = i; + dst[i] = 0; + }
- cpu_post_exec_02(code, (ulong) src, (ulong) dst); + cpu_post_exec_02(code, (ulong) src, (ulong) dst);
- ret = memcmp(src, dst, sizeof(dst)) == 0 ? 0 : -1; - } + ret = memcmp(src, dst, sizeof(dst)) == 0 ? 0 : -1;
if (ret != 0) { post_log("Error at multi test !\n");
On Fri, 23 Dec 2011 12:29:11 +0100 Wolfgang Denk wd@denx.de wrote:
Clean up and document the code:
- get rid of unneeded code block
- add comment which code is generated
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
Acked-by: Anatolij Gustschin agust@denx.de
Dear Wolfgang Denk,
In message 1324639752-26856-2-git-send-email-wd@denx.de you wrote:
Clean up and document the code:
- get rid of unneeded code block
- add comment which code is generated
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
post/lib_powerpc/multi.c | 27 ++++++++++++--------------- 1 files changed, 12 insertions(+), 15 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
The code and comment disagreed: the comment claimed that r6...r31 were copied, and consequently the arrays for "src" and "dst" were declared with 26 entries, but the actual code ("lmw r5,0(r3)" and "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted in false "POST cpu Error at multi test" messages.
Fix the comment and the array sizes.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com --- post/lib_powerpc/multi.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/post/lib_powerpc/multi.c b/post/lib_powerpc/multi.c index b8619de..6642ee3 100644 --- a/post/lib_powerpc/multi.c +++ b/post/lib_powerpc/multi.c @@ -27,9 +27,9 @@ * CPU test * Load/store multiple word instructions: lmw, stmw * - * 26 consecutive words are loaded from a source memory buffer - * into GPRs r6 through r31. After that, 26 consecutive words are stored - * from the GPRs r6 through r31 into a target memory buffer. The contents + * 27 consecutive words are loaded from a source memory buffer + * into GPRs r5 through r31. After that, 27 consecutive words are stored + * from the GPRs r5 through r31 into a target memory buffer. The contents * of the source and target buffers are then compared. */
@@ -44,7 +44,7 @@ int cpu_post_test_multi(void) { int ret = 0; unsigned int i; - ulong src[26], dst[26]; + ulong src[27], dst[27]; int flag = disable_interrupts();
ulong code[] = {
Hello Wolfgang,
On Fri, 23 Dec 2011 12:29:12 +0100 Wolfgang Denk wd@denx.de wrote:
The code and comment disagreed: the comment claimed that r6...r31 were copied, and consequently the arrays for "src" and "dst" were declared with 26 entries, but the actual code ("lmw r5,0(r3)" and "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted in false "POST cpu Error at multi test" messages.
Great! Thanks for fixing this bug!
Acked-by: Anatolij Gustschin agust@denx.de Tested-by: Anatolij Gustschin agust@denx.de
But I wonder why didn't we see it with U-Boot built using older GCC versions.
Since only 26 words will be compared after the test, the issue only shows up if the destination buffer is placed at lower addresses on the stack than the source buffer. In this case the first word in the source buffer is overwritten. GCC 4.6.1 generated code which changed the order of src[] and dst[] on the stack and the hidden bug showed up.
Here is a partial dump of each buffer and additionally a dump of the following word. The buffer address is in parenthesis:
with GCC 4.2.2:
00: src(03e51c74) 0x00000000, dst(03e51cdc) 0x00000000 01: src(03e51c78) 0x00000001, dst(03e51ce0) 0x00000000 ... 25: src(03e51cd8) 0x00000019, dst(03e51d40) 0x00000000 26: src(03e51cdc) 0x00000000, dst(03e51d44) 0x00000000
Test result:
00: src(03e51c74) 0x00000000, dst(03e51cdc) 0x00000000 01: src(03e51c78) 0x00000001, dst(03e51ce0) 0x00000001 ... 25: src(03e51cd8) 0x00000019, dst(03e51d40) 0x00000019 26: src(03e51cdc) 0x00000000, dst(03e51d44) 0x00000000
with GCC 4.6.1:
00: src(03e57cf4) 0x00000000, dst(03e57c8c) 0x00000000 01: src(03e57cf8) 0x00000001, dst(03e57c90) 0x00000000 ... 25: src(03e57d58) 0x00000019, dst(03e57cf0) 0x00000000 26: src(03e57d5c) 0x03f9c3c0, dst(03e57cf4) 0x00000000
Test result: Error at multi test ! 00: src(03e57cf4) 0x03f9c3c0, dst(03e57c8c) 0x00000000 01: src(03e57cf8) 0x00000001, dst(03e57c90) 0x00000001 ... 25: src(03e57d58) 0x00000019, dst(03e57cf0) 0x00000019 26: src(03e57d5c) 0x03f9c3c0, dst(03e57cf4) 0x03f9c3c0
Thanks, Anatolij
Dear Anatolij Gustschin,
In message 20111223170640.18cfe56f@wker you wrote:
The code and comment disagreed: the comment claimed that r6...r31 were copied, and consequently the arrays for "src" and "dst" were declared with 26 entries, but the actual code ("lmw r5,0(r3)" and "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted in false "POST cpu Error at multi test" messages.
Great! Thanks for fixing this bug!
Thanks for testing and reporting it!
But I wonder why didn't we see it with U-Boot built using older GCC versions.
Yes, I was surprised,too, and suspected a compiler problem instead...
Since only 26 words will be compared after the test, the issue only shows up if the destination buffer is placed at lower addresses on the stack than the source buffer. In this case the first word in the source buffer is overwritten. GCC 4.6.1 generated code which changed the order of src[] and dst[] on the stack and the hidden bug showed up.
This matches my own analysis. Actually the code generated by gcc 4.5.1 and 4.6.1 looks _really_ different in a lot of places; it seems a lot has been changed in GCC again.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
In message 1324639752-26856-3-git-send-email-wd@denx.de you wrote:
The code and comment disagreed: the comment claimed that r6...r31 were copied, and consequently the arrays for "src" and "dst" were declared with 26 entries, but the actual code ("lmw r5,0(r3)" and "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted in false "POST cpu Error at multi test" messages.
Fix the comment and the array sizes.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
post/lib_powerpc/multi.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Hello Wolfgang,
Looks good. A minor commend below.
On Fri, 23 Dec 2011 12:29:10 +0100 Wolfgang Denk wd@denx.de wrote:
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
post/lib_powerpc/multi.c | 54 +++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/post/lib_powerpc/multi.c b/post/lib_powerpc/multi.c index 5845616..4b4b119 100644 --- a/post/lib_powerpc/multi.c +++ b/post/lib_powerpc/multi.c @@ -38,45 +38,41 @@
#if CONFIG_POST & CONFIG_SYS_POST_CPU
-extern void cpu_post_exec_02 (ulong *code, ulong op1, ulong op2); +extern void cpu_post_exec_02(ulong * code, ulong op1, ulong op2);
IIRC checkpatch complains like "foo * bar" should be "foo *bar", There is surely no need to resubmit the patch, it could be changed when applying. Otherwise
Acked-by: Anatolij Gustschin agust@denx.de
Thanks, Anatolij
Dear Anatolij Gustschin,
In message 20111223133952.5e09a8b4@wker you wrote:
Looks good. A minor commend below.
...
-extern void cpu_post_exec_02 (ulong *code, ulong op1, ulong op2); +extern void cpu_post_exec_02(ulong * code, ulong op1, ulong op2);
IIRC checkpatch complains like "foo * bar" should be "foo *bar", There is surely no need to resubmit the patch, it could be changed when applying. Otherwise
You are right. Fixed this. Thanks for the review!
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
In message 1324639752-26856-1-git-send-email-wd@denx.de you wrote:
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Anatolij Gustschin agust@denx.de Cc: Stefan Roese sr@denx.de Cc: Kumar Gala galak@kernel.crashing.org Cc: Kim Phillips kim.phillips@freescale.com Cc: Andy Fleming afleming@gmail.com
post/lib_powerpc/multi.c | 54 +++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 29 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (2)
-
Anatolij Gustschin -
Wolfgang Denk