[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Andreas Bießmann]

This is only a patch to test if BSS was accessed in any function in init
function array!

DO NOT APPLY TO MAINLINE!

To run the test adopt the bss_mirror value to point somwhere in RAM with
sufficient space!

A step by step guide is like this:
 * apply the patch
 * adopt bss_mirror in arch/arm/lib/board.c:board_init_f()
 * build and run like always
 * search for something like: 'BSS got corrupted after init_fnc @ 0x20100478'
   after banner
 * use objdump to search the symbol
  ---8<--- 
  arm-unknown-linux-gnueabi-objdump -S ../build_uboot-at91rm9200ek/u-boot | grep 20100478
   20100478 <timer_init>:
   20100478:	e59f3064 	ldr	r3, [pc, #100]	; 201004e4 <timer_init+0x6c>
  --->8---

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
 arch/arm/lib/board.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 1fd5f83..890e6f1 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -274,6 +274,10 @@ void board_init_f (ulong bootflag)
 	init_fnc_t **init_fnc_ptr;
 	gd_t *id;
 	ulong addr, addr_sp;
+	uint32_t *bss_mirror;
+	uint32_t *bss_start;
+	size_t bss_length;
+	init_fnc_t *break_bss = NULL;
 
 	/* Pointer is writable since we allocated a register for it */
 	gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR);
@@ -284,12 +288,26 @@ void board_init_f (ulong bootflag)
 
 	gd->mon_len = _bss_end_ofs;
 
+	/* take a location in SDRAM which might be free */
+	bss_mirror = (uint32_t*)(CONFIG_SYS_SDRAM_BASE + 0x600000);
+	bss_start = (uint32_t*)(_bss_start_ofs + _TEXT_BASE);
+	bss_length = _bss_end_ofs - _bss_start_ofs;
+	memcpy((void*)bss_mirror, (void*)bss_start, bss_length);
+
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
 		if ((*init_fnc_ptr)() != 0) {
 			hang ();
 		}
+		if (!break_bss)
+			if (memcmp((void*)bss_mirror, bss_start, bss_length) != 0)
+				break_bss = *init_fnc_ptr;
 	}
 
+	if (break_bss)
+		printf("BSS got corrupted after init_fnc @ 0x%p\n", *break_bss);
+	else
+		printf("BSS is still OK after init_fnc\n");
+
 	debug ("monitor len: %08lX\n", gd->mon_len);
 	/*
 	 * Ram is setup, size stored in gd !!
-- 
1.7.3.2

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Albert ARIBAUD]

Le 01/12/2010 09:10, Andreas Bie?mann a ?crit :
> This is only a patch to test if BSS was accessed in any function in init
> function array!
>
> DO NOT APPLY TO MAINLINE!

Nice helping tool. Thanks! One small question

>   * adopt bss_mirror in arch/arm/lib/board.c:board_init_f()

What do you mean exactly by this?

Amicalement,
-- 
Albert.

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Andreas Bießmann]

Dear Albert ARIBAUD,

Am 01.12.2010 10:37, schrieb Albert ARIBAUD:
> Le 01/12/2010 09:10, Andreas Bie?mann a ?crit :

>>   * adopt bss_mirror in arch/arm/lib/board.c:board_init_f()
> 
> What do you mean exactly by this?

this parameter needs to be adopted by SoC/board tester.

In short words, adopt that line to your needs:

--->8---
+	bss_mirror = (uint32_t*)(CONFIG_SYS_SDRAM_BASE + 0x600000);
---8<---

The tester have to place the 'bss_mirror' space to an appropriate value.
at91rm9200ek does have 32MiB of SDRAM, the topmost part is always used
for stack, GD, u-boot after relocation a.s.o. I usually use the RAMBOOT
configuration of at91rm9200ek. In that case TEXT_BASE is SDRAM_BASE +
0x100000. Therefore I use here 0x600000 (u-boot size plus safety).
I can not recommend a proper place for all boards cause this parameter
is highly is board specific.

regards

Andreas Bie?mann

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Albert ARIBAUD]

hi Andreas,

Le 01/12/2010 11:05, Andreas Bie?mann a ?crit :

> I can not recommend a proper place for all boards cause this parameter
> is highly is board specific.

Understood -- each board maintainer must select an adapted location for 
the mirror.

> regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Andreas Bießmann]

Dear Albert ARIBAUD,

Am 01.12.2010 12:36, schrieb Albert ARIBAUD:
> hi Andreas,
> 
> Le 01/12/2010 11:05, Andreas Bie?mann a ?crit :
> 
>> I can not recommend a proper place for all boards cause this parameter
>> is highly is board specific.
> 
> Understood -- each board maintainer must select an adapted location for
> the mirror.

Yes, but I have one thing to mention. This test does only detect writing
to BSS values in one of the init_sequence[] functions.

In my case I detected arch/arm/cpu/arm920t/at91/timer.c as faulty. But I
still have some issues booting at91rm9200ek from NOR flash. None of the
init_sequence[] functions writes to a BSS value in that case ... So if
the 'corrupted .dyn.rel' thing is my problem in NOR booting case
.dyn.rel got corrupted before board_init_f() or by some other function
call between for loop of init_sequence[] and relocate_code() ... I don't
know if I get this until release of v2010.12.

Maybe another maintainer is more lucky ...

regards

Andreas Bie?mann

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Albert ARIBAUD]

Hi Andreas,

Le 01/12/2010 13:12, Andreas Bie?mann a ?crit :

> But I
> still have some issues booting at91rm9200ek from NOR flash. None of the
> init_sequence[] functions writes to a BSS value in that case ... So if
> the 'corrupted .dyn.rel' thing is my problem in NOR booting case
> .dyn.rel got corrupted before board_init_f() or by some other function
> call between for loop of init_sequence[] and relocate_code() ... I don't
> know if I get this until release of v2010.12.

Please post a new message (or an update to the existing thread) for this 
issue; indicate the exact symptom, the build chain in case it matters, 
which commit/branch/tag you last tried and any patches necessary.

I'll try to reproduce the build and analyze the readelf/objdump to see 
if (and then hopefully why) the .rel.dyn and .dynsym tables get corrupted.

> regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4CF63345.9020006@free.fr> you wrote:
> > I can not recommend a proper place for all boards cause this parameter
> > is highly is board specific.
> 
> Understood -- each board maintainer must select an adapted location
> for the mirror.

I'm not conviced that ech board maintainer should do that, and I
somewhat doubt the approach taken.

The main purpose of the patch (as I see it) is to find out which
functions might be candidates to write to bss too early.  There is not
that much board specific code involved there.

If we can generate a slist of functions that are called before
relocation, we can use nm to check if these refernce any sumbols in
bss. We could even generate black / white lists.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Roses are red
Violets are blue
Some poems rhyme

[U-Boot] [TEST] arm:board_init_f(): mirror BSS and check after each init_fnc()

[Albert ARIBAUD]

Le 01/12/2010 13:35, Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
>
> In message<4CF63345.9020006@free.fr>  you wrote:
>>> I can not recommend a proper place for all boards cause this parameter
>>> is highly is board specific.
>>
>> Understood -- each board maintainer must select an adapted location
>> for the mirror.
>
> I'm not conviced that ech board maintainer should do that, and I
> somewhat doubt the approach taken.
>
> The main purpose of the patch (as I see it) is to find out which
> functions might be candidates to write to bss too early.  There is not
> that much board specific code involved there.
>
> If we can generate a slist of functions that are called before
> relocation, we can use nm to check if these refernce any sumbols in
> bss. We could even generate black / white lists.

Just to clarify:

I see Andreas' proposal as a debug tool, notintended to ever be applied 
to a branch, but to be kept and applied to local developer repositories 
during debug.

It seems you, Wolfgang, are considering this more like a tool that would 
end up in the u-boot source tree and be e.g. run systematically after a 
build, even if the builder had no intent of debugging. Am I right?

If so, then yes, a tool that would analyze the ELF binary would be a 
good approach.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.