| Message ID | 1291191050-46774-1-git-send-email-andreas.devel@googlemail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
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,
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
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,
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
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,
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
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,
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 !!
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(-)