| Message ID | 1288909332-26220-1-git-send-email-albert.aribaud@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
Le 04/11/2010 23:22, Albert Aribaud a écrit : > older ld emitted all ELF relocations in input sections named > ..rel.dyn, whereas newer ld uses names of the form .rel*. The > linker script only collected .rel.dyn input sections. Rewrite > to collect all .rel* input sections and overlay with .bss. > > Signed-off-by: Albert Aribaud<albert.aribaud@free.fr> Tested on openrd_base, boots fine, works fine. Wolfgang, please test with tx25. Alexander, please test above your patches in place of previous patch v3. Others who are willing to test on other ARMs are welcome too. :) Amicalement,
Dear Albert Aribaud, > older ld emitted all ELF relocations in input sections named > .rel.dyn, whereas newer ld uses names of the form .rel*. The > linker script only collected .rel.dyn input sections. Rewrite > to collect all .rel* input sections and overlay with .bss. > > Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> > --- Thank you, works fine with: gcc 4.2.4 (binary size 258700) gcc 4.3.5 (binary size 251600 - nice!) (ARM9 AT91SAM9XE512, top9000, TOT u-boot-atmel at91cleanup-rework branch) Signed-off-by: Reinhard Meyer <u-boot@emk-elektronik.de>
Le 05/11/2010 09:38, Reinhard Meyer a écrit : > Dear Albert Aribaud, >> older ld emitted all ELF relocations in input sections named >> .rel.dyn, whereas newer ld uses names of the form .rel*. The >> linker script only collected .rel.dyn input sections. Rewrite >> to collect all .rel* input sections and overlay with .bss. >> >> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr> >> --- > Thank you, > works fine with: > > gcc 4.2.4 (binary size 258700) > gcc 4.3.5 (binary size 251600 - nice!) > > (ARM9 AT91SAM9XE512, top9000, TOT u-boot-atmel at91cleanup-rework branch) Thanks for testing. The size reduction is due to the gcc ARM backend which has obviously undergone many improvements since gcc 4.2.x. Openrd_base goes from 376 to 360k, and edminiv2 (untested yet, though) from 152 to 144k when using the CS arm-2010q1-202 toolchain (4.4.1) instead of the ELDK 4.2 one (gcc 4.2.2). > Signed-off-by: Reinhard Meyer<u-boot@emk-elektronik.de> Did you mean 'Tested-by:'? Amicalement,
Le 04/11/2010 23:27, Albert ARIBAUD a écrit : > Le 04/11/2010 23:22, Albert Aribaud a écrit : >> older ld emitted all ELF relocations in input sections named >> ..rel.dyn, whereas newer ld uses names of the form .rel*. The >> linker script only collected .rel.dyn input sections. Rewrite >> to collect all .rel* input sections and overlay with .bss. >> >> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr> > > Tested on openrd_base, boots fine, works fine. > > Wolfgang, please test with tx25. > > Alexander, please test above your patches in place of previous patch v3. > > Others who are willing to test on other ARMs are welcome too. :) Did testers manage to try this patch set? I'd like to make sure it solves current issues with master for ARM relocation. Amicalement,
Le 09/11/2010 19:24, Daniel Hobi a écrit : > Hi Albert, > > On 04.11.2010 23:22, Albert Aribaud wrote: >> older ld emitted all ELF relocations in input sections named >> .rel.dyn, whereas newer ld uses names of the form .rel*. The >> linker script only collected .rel.dyn input sections. Rewrite >> to collect all .rel* input sections and overlay with .bss. > > Tested-by: Daniel Hobi<daniel.hobi@schmid-telecom.ch> Thanks Daniel for the test. > Thank you. This patch is required to get Kirkwood-based boards working > again when using the CodeSourcery 2009q3 toolchain. (can't find the 2010q3 Lite toolchain on CodeSourcery's site, latest is 2010q1 apparently... Can you tell me which gcc and which ld version is used in 2010q3?) I think this V4 of my patchset could be now committed to u-boot-arm/master, and if possible even on u-boot-arm for the december release of u-boot, as it is a bugfix. > But shouldn't this change be applied to all ARM linker scripts, ie > arch/arm/cpu/*/u-boot.lds? Yes, it should. :) > And on many ARM platforms (including Kirkwood), the timer implementation > is still accessing BSS variables before relocation. That is fixed in the orion5x code; a git blame on the timer.c file will tell you which commit of mine did the fix, and from then you can port to kirkwood as the orion5x code comes from the kirkwood one. > Is someone working on this? Candidates are: > > $ git grep "static ulong timestamp" > arch/arm/cpu/arm1136/mx31/timer.c:static ulong timestamp; > arch/arm/cpu/arm1136/omap24xx/timer.c:static ulong timestamp; > arch/arm/cpu/arm1176/tnetv107x/timer.c:static ulong timestamp; > arch/arm/cpu/arm720t/interrupts.c:static ulong timestamp; > arch/arm/cpu/arm920t/a320/timer.c:static ulong timestamp; > arch/arm/cpu/arm920t/at91rm9200/timer.c:static ulong timestamp; > arch/arm/cpu/arm920t/s3c24x0/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/davinci/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/kirkwood/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/mx25/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/mx27/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/omap/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/orion5x/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/spear/timer.c:static ulong timestamp; > arch/arm/cpu/arm926ejs/versatile/timer.c:static ulong timestamp; > arch/arm/cpu/armv7/mx5/timer.c:static ulong timestamp; > arch/arm/cpu/armv7/omap-common/timer.c:static ulong timestamp; > arch/arm/cpu/lh7a40x/timer.c:static ulong timestamp; > arch/arm/cpu/s3c44b0/timer.c:static ulong timestamp; Normally, the board maintainers should handle this during the window after next version... Dunno if that is practical, but OTOH it would easily show which boards are still maintained and which are not. :) > Best regards, > Daniel Amicalement,
Le 09/11/2010 19:47, Albert ARIBAUD a écrit : > I think this V4 of my patchset could be now committed to > u-boot-arm/master, and if possible even on u-boot-arm for the december > release of u-boot, as it is a bugfix. ... "even on u-boot", not "u-boot-arm", of course. Amicalement,
Dear Daniel Hobi, Am 09.11.2010 um 19:24 schrieb Daniel Hobi: > And on many ARM platforms (including Kirkwood), the timer implementation > is still accessing BSS variables before relocation. > > Is someone working on this? Candidates are: > > $ git grep "static ulong timestamp" > arch/arm/cpu/arm1136/mx31/timer.c:static ulong timestamp; > arch/arm/cpu/arm1136/omap24xx/timer.c:static ulong timestamp; > arch/arm/cpu/arm1176/tnetv107x/timer.c:static ulong timestamp; > arch/arm/cpu/arm720t/interrupts.c:static ulong timestamp; > arch/arm/cpu/arm920t/a320/timer.c:static ulong timestamp; > arch/arm/cpu/arm920t/at91rm9200/timer.c:static ulong timestamp; arch/arm/cpu/arm920t/at91rm9200/ is dead an will be removed soon. arch/arm/cpu/arm920t/at91/ still misses such an implementation but will be something like arch/arm/cpu/arm926ejs/at91/ BTW couldn't the stuff in #ifdef CONFIG_AT91FAMILY in global_data generalized in some way to be used for all arm boards? regards Andreas Bießmann
Dear Albert Ardibaud, Am 09.11.2010 um 19:47 schrieb Albert ARIBAUD: >> Thank you. This patch is required to get Kirkwood-based boards working >> again when using the CodeSourcery 2009q3 toolchain. > > (can't find the 2010q3 Lite toolchain on CodeSourcery's site, latest is > 2010q1 apparently... Can you tell me which gcc and which ld version is > used in 2010q3?) I think http://www.codesourcery.com/sgpp/lite/arm/portal/release1033 was meant ... regards Andreas Bießmann
Le 09/11/2010 20:27, Andreas Bießmann a écrit :
> BTW couldn't the stuff in #ifdef CONFIG_AT91FAMILY in global_data generalized in some way to be used for all arm boards?
Don't know for others, but regarding the timer, there is no need for
timestamp before relocation, so it needs not join gd stuff.
Amicalement,
I have been periodically rebasing my patches for the Seagate DockStar
on master. But ever since the elf_reloc changes were merged, I have
been unable to get a working build. First I saw symptoms similar to
what Alexander Holler reported (failing during NAND initialization due
to incorrect BSS relocation), but the latest arm926ejs ld script
patches did not fix it. The system would hang here:
U-Boot 2010.12-rc1 (Nov 09 2010 - 17:52:38)
Seagate FreeAgent DockStar
SoC: Kirkwood 88F6281_A0
DRAM: 128 MiB
NAND:
This was built with the CodeSourcery 2010q1 toolchain. I also
included Alexander's patch to double-check the relocation, and no
error message gets printed.
Today, I decided to try out Sebastien Carlier's patch to change to
partial linking, and to my surprise it now works.
I have the two ELF images in case anyone can suggest what I should
look for to see why one is working and the other isn't. But I don't
know much about interpreting readelf or objdump output.
Also, I'd like to resubmit the latest version of the DockStar patch to
the list. Should I base it on the partial-linking patch, or on master
(knowing that it may not produce a working build in this case)?
Le 10/11/2010 00:43, Eric Cooper a écrit : > I have been periodically rebasing my patches for the Seagate DockStar > on master. But ever since the elf_reloc changes were merged, I have > been unable to get a working build. First I saw symptoms similar to > what Alexander Holler reported (failing during NAND initialization due > to incorrect BSS relocation), but the latest arm926ejs ld script > patches did not fix it. The system would hang here: > > U-Boot 2010.12-rc1 (Nov 09 2010 - 17:52:38) > Seagate FreeAgent DockStar > > SoC: Kirkwood 88F6281_A0 > DRAM: 128 MiB > NAND: > > This was built with the CodeSourcery 2010q1 toolchain. I also > included Alexander's patch to double-check the relocation, and no > error message gets printed. > > Today, I decided to try out Sebastien Carlier's patch to change to > partial linking, and to my surprise it now works. > > I have the two ELF images in case anyone can suggest what I should > look for to see why one is working and the other isn't. But I don't > know much about interpreting readelf or objdump output. Can you make the ELF images available? I'll do the readlf/objdump analysis and feed back on what I find. Amicalement,
Hi Albert, On 09.11.2010 19:47, Albert ARIBAUD wrote: > Le 09/11/2010 19:24, Daniel Hobi a écrit : >> Thank you. This patch is required to get Kirkwood-based boards working >> again when using the CodeSourcery 2009q3 toolchain. > > (can't find the 2010q3 Lite toolchain on CodeSourcery's site, latest is > 2010q1 apparently... Can you tell me which gcc and which ld version is > used in 2010q3?) Andreas is correct: I'm still using Sourcery G++ Lite 2009q3-67. > I think this V4 of my patchset could be now committed to > u-boot-arm/master, and if possible even on u-boot-arm for the december > release of u-boot, as it is a bugfix. Since all ARM boards are broken when using a recent toolchain, the patch should go in as fast as possible. >> But shouldn't this change be applied to all ARM linker scripts, ie >> arch/arm/cpu/*/u-boot.lds? > > Yes, it should. :) Can you please provide such a patch? >> And on many ARM platforms (including Kirkwood), the timer implementation >> is still accessing BSS variables before relocation. >> >> Is someone working on this? Candidates are: >> >> $ git grep "static ulong timestamp" >> arch/arm/cpu/arm1136/mx31/timer.c:static ulong timestamp; >> arch/arm/cpu/arm1136/omap24xx/timer.c:static ulong timestamp; >> arch/arm/cpu/arm1176/tnetv107x/timer.c:static ulong timestamp; >> arch/arm/cpu/arm720t/interrupts.c:static ulong timestamp; >> arch/arm/cpu/arm920t/a320/timer.c:static ulong timestamp; >> arch/arm/cpu/arm920t/at91rm9200/timer.c:static ulong timestamp; >> arch/arm/cpu/arm920t/s3c24x0/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/davinci/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/kirkwood/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/mx25/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/mx27/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/omap/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/orion5x/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/spear/timer.c:static ulong timestamp; >> arch/arm/cpu/arm926ejs/versatile/timer.c:static ulong timestamp; >> arch/arm/cpu/armv7/mx5/timer.c:static ulong timestamp; >> arch/arm/cpu/armv7/omap-common/timer.c:static ulong timestamp; >> arch/arm/cpu/lh7a40x/timer.c:static ulong timestamp; >> arch/arm/cpu/s3c44b0/timer.c:static ulong timestamp; > > Normally, the board maintainers should handle this during the window > after next version... Dunno if that is practical, but OTOH it would > easily show which boards are still maintained and which are not. :) Since accessing BSS variables before relocation is a severe bug, we should handle this now for all SoCs. But right now, there are two approaches: - Perform initialization of static variables after relocation, and thus forbid using functions which access such variables before relocation (reset_timer*, get_timer*, set_timer). You patched arch/arm/cpu/arm926ejs/orion5x/timer.c to use this approach. - Move static variables to struct global_data, so they can be used before relocation. Used by AT91 timers and proposed for A320 and S3C64xx in: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88095 http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88160 I hope we can solve this problem in the same way for all ARM timers. And if we use the second approach, we probably can generalize the AT91 data in struct global_data as proposed by Andreas. Best regards, Daniel
Le 10/11/2010 13:31, Daniel Hobi a écrit : > Since all ARM boards are broken when using a recent toolchain, the patch > should go in as fast as possible. > >>> But shouldn't this change be applied to all ARM linker scripts, ie >>> arch/arm/cpu/*/u-boot.lds? >> >> Yes, it should. :) > > Can you please provide such a patch? I could, but I tend to provide patches only for boards that I can test, which basically covers only arm926ejs, or that I can get tested; I'd prefer not to provide patches for HW that I cannot test, and thus I would prefer that patches for other cpus be handled by people who actually own boards with these cpus and can test their patching. After all, this very bugfix is due to ELF relocations having been tested with too poor a range of toolchains. > Since accessing BSS variables before relocation is a severe bug, we > should handle this now for all SoCs. But right now, there are two > approaches: > > - Perform initialization of static variables after relocation, and > thus forbid using functions which access such variables before > relocation (reset_timer*, get_timer*, set_timer). > > You patched arch/arm/cpu/arm926ejs/orion5x/timer.c to use this > approach. > > - Move static variables to struct global_data, so they can be used > before relocation. Used by AT91 timers and proposed for A320 and > S3C64xx in: > > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88095 > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88160 > > I hope we can solve this problem in the same way for all ARM timers. And > if we use the second approach, we probably can generalize the AT91 data > in struct global_data as proposed by Andreas. Either way is ok with me. > Best regards, > Daniel Amicalement,
Dear concerned, > - Move static variables to struct global_data, so they can be used > before relocation. Used by AT91 timers and proposed for A320 and > S3C64xx in: > > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88095 > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88160 > > I hope we can solve this problem in the same way for all ARM timers. And > if we use the second approach, we probably can generalize the AT91 data > in struct global_data as proposed by Andreas. The AT91 global data for clock/timer use has 2 parts: #ifdef CONFIG_AT91FAMILY /* "static data" needed by at91's clock.c */ unsigned long cpu_clk_rate_hz; unsigned long main_clk_rate_hz; unsigned long mck_rate_hz; unsigned long plla_rate_hz; unsigned long pllb_rate_hz; unsigned long at91_pllb_usb_init; /* "static data" needed by at91's timer.c */ unsigned long timer_rate_hz; unsigned long tbl; unsigned long tbu; unsigned long long timer_reset_value; #endif I have removed reset_timer() from the AT91 timer now, making the global variable "timer_reset_value" obsolete. For that all occurences of the sequence reset_timer(); ... loop .. if .. get_timer(0) >= timeout has to be replaced by u32 start_time = get_timer(0); ... loop .. if .. get_timer(start_time) >= timeout Currently, for AT91, that is only in the file drivers/mtd/nand/nand_base.c In that view, we could put unsigned long timer_rate_hz; unsigned long tbl; unsigned long tbu; outside the AT91FAMILY ifdef. We can even split the timer implementation into two parts: part 1 (hardware specific) implements a method to increment tbu/tbl and exports the function get_ticks(), and part 2 (hardware independant) uses get_ticks() and timer_rate_hz to implement udelay() and get_timer(). Best Regards, Reinhard
On 10.11.2010 13:48, Albert ARIBAUD wrote: > Le 10/11/2010 13:31, Daniel Hobi a écrit : >>>> But shouldn't this change be applied to all ARM linker scripts, ie >>>> arch/arm/cpu/*/u-boot.lds? >>> >>> Yes, it should. :) >> >> Can you please provide such a patch? > > I could, but I tend to provide patches only for boards that I can test, > which basically covers only arm926ejs, or that I can get tested; I'd > prefer not to provide patches for HW that I cannot test, and thus I > would prefer that patches for other cpus be handled by people who > actually own boards with these cpus and can test their patching. After > all, this very bugfix is due to ELF relocations having been tested with > too poor a range of toolchains. I prefer a single patch to solve one problem in all places. And since you probably have most experience with the new ARM relocation, that patch should come from you. The ARM architecture and board maintainers will test your patch during the current release cycle. Best regards, Daniel
On Wed, Nov 10, 2010 at 08:53:03AM +0100, Albert ARIBAUD wrote: > Can you make the ELF images available? I'll do the readlf/objdump > analysis and feed back on what I find. I've uploaded them here: http://www.cs.cmu.edu/~ecc/u-boot.{good,bad} Thanks.
Le 10/11/2010 15:20, Eric Cooper a écrit : > On Wed, Nov 10, 2010 at 08:53:03AM +0100, Albert ARIBAUD wrote: >> Can you make the ELF images available? I'll do the readlf/objdump >> analysis and feed back on what I find. > > I've uploaded them here: > http://www.cs.cmu.edu/~ecc/u-boot.{good,bad} > Thanks. I haven't found any obvious issue in the files for either good or bad case: relocation tables are generated and correct with only types 23 and 2 records, and relocation symbols are produced and with the correct values. the only one case I had where the code after correct relocation did not function properly was with tx25 where the hard-coded binary size in the config file was smaller than the actual binary size, causing only part of the relocation table to be read back from NAND to RAM, thus skipping a good deal of actual relocations. But here the "bad" case is smaller than the "good" case, so even if you're using a hard-coded binary size, *most probably* it is still ok. Which means I cannot see why this code would not work with relocations, all the more so since Sébastien's patches do not touch compiler or linker flags unless I missed something. :/ However much I hate taking a "let's pretend it did not happen" stance, I think the most effective approach here is to consider that if no-one has issues with the ELF relocation V4 patch (Wolfgang? Alexander?) on currently supported boards, then it only causes problems on a not-yet-pulled-in board, so: - V4 should be integrated now as a bug fix for existing supported boards; - Dockstar support should be applied only after Sébastien's patches are. Any comments? Amicalement,
diff --git a/arch/arm/cpu/arm926ejs/u-boot.lds b/arch/arm/cpu/arm926ejs/u-boot.lds index 72f45f8..28c91f9 100644 --- a/arch/arm/cpu/arm926ejs/u-boot.lds +++ b/arch/arm/cpu/arm926ejs/u-boot.lds @@ -45,24 +45,30 @@ SECTIONS . = ALIGN(4); - __rel_dyn_start = .; - .rel.dyn : { *(.rel.dyn) } - __rel_dyn_end = .; - - __dynsym_start = .; - .dynsym : { *(.dynsym) } - - . = ALIGN(4); - . = .; __u_boot_cmd_start = .; .u_boot_cmd : { *(.u_boot_cmd) } __u_boot_cmd_end = .; . = ALIGN(4); - __bss_start = .; - .bss (NOLOAD) : { *(.bss) . = ALIGN(4); } - _end = .; + + .rel.dyn : { + __rel_dyn_start = .; + *(.rel*) + __rel_dyn_end = .; + } + + .dynsym : { + __dynsym_start = .; + *(.dynsym) + } + + .bss __rel_dyn_start (OVERLAY) : { + __bss_start = .; + *(.bss) + . = ALIGN(4); + _end = .; + } /DISCARD/ : { *(.dynstr*) } /DISCARD/ : { *(.dynamic*) }
older ld emitted all ELF relocations in input sections named .rel.dyn, whereas newer ld uses names of the form .rel*. The linker script only collected .rel.dyn input sections. Rewrite to collect all .rel* input sections and overlay with .bss. Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> --- V1 Initial submission V2 arm926ejs: added ALIGN between bss and .rel.dyn sections tx25: removed GOT and datarel output sections tx25: fixed typo in config file commit message V3 arm926ejs: overlaid .bss and .rel.dyn sections tx25: overlaid .bss and .rel.dyn sections V4 arm926ejs and tx25: fixed overlay tx25: removed third patch as u-boot size remains small enough arch/arm/cpu/arm926ejs/u-boot.lds | 30 ++++++++++++++++++------------ 1 files changed, 18 insertions(+), 12 deletions(-)