[U-Boot] ARM: Incorrect ROM protection range?
Hi all,
I am using relocation fixed a320evb (arm) u-boot.
I noticed something weird about the output of command flinfo.
The size of u-boot.bin is 129156 (0x1F884), but the protected range is only 0 ~ 0x1bfff.
I guess that it is because u-boot protects _start ~ __bss_start, but there are some other things in u-boot.bin after __bss_start, e.g. .rel.dyn section and .dynsym section
That is quite strange. According to arch/arm/cpu/arm920t/u-boot.lds, .rel.dyn and .dynsym sections should be placed before __bss_start. However, objdump shows that they are not at where they should be.
Do I understand correctly? Does anybody have similar situation?
BTW, the toolchain I am using is arm-none-linux-gnueabi-gcc (Sourcery G++ Lite 2010q1-202) 4.4.1.
0001be8c g O .u_boot_cmd 00000018 __u_boot_cmd_printenv 0001bea4 g O .u_boot_cmd 00000018 __u_boot_cmd_setenv 0001bebc g O .u_boot_cmd 00000018 __u_boot_cmd_run 0001bed4 g O .u_boot_cmd 00000018 __u_boot_cmd_source 0001beec g O .u_boot_cmd 00000018 __u_boot_cmd_version 0001bf04 g O .u_boot_cmd 00000018 __u_boot_cmd_imxtract 0001bf1c g *ABS* 00000000 __u_boot_cmd_end 0001bf1c g .bss 00000000 __bss_start 0001bf1c g O .bss 00000004 monitor_flash_len 0001bf1c g .rel.dyn 00000000 __rel_dyn_start 0001bf1c l d .bss 00000000 .bss 0001bf1c l d .rel.dyn 00000000 .rel.dyn 0001bf20 l O .bss 0000012c images 0001c04c l O .bss 00000004 os_data_state 0001c050 l O .bss 00000004 os_data_addr 0001c054 l O .bss 00000004 bin_start_address 0001c058 l O .bss 00000004 k_data_escape 0001c05c g O .bss 00000004 os_data_init 0001c060 l O .bss 00000004 k_data_escape_saved 0001c064 l O .bss 00000004 os_data_state_saved [snip] 0001dcac l O .bss 00000004 NetRestarted 0001dcb0 g O .bss 00000004 NetRestartWrap 0001dcb4 l O .bss 00000004 NetDevExists 0001dcb8 g O .bss 00001e20 PktBuf 0001f73c g .dynsym 00000000 __dynsym_start 0001f73c g .rel.dyn 00000000 __rel_dyn_end 0001f73c l d .dynsym 00000000 .dynsym 0001fad8 g O .bss 00000010 NetRxPackets 0001fae8 g O .bss 00000620 NetArpWaitPacketBuf 00020108 l O .bss 00000004 env_changed_id.3244
U-Boot 2011.03-rc1-00134-ga898a11 (Feb 23 2011 - 16:53:13)
DRAM: 64 MiB Flash: 32.5 MiB In: serial Out: serial Err: serial Net: FTMAC100 Hit any key to stop autoboot: 0 A320 # fli 1
Bank # 1: SST 39LF040 flash (8 x 8) Size: 512 kB in 128 Sectors AMD Legacy command set, Manufacturer ID: 0xBF, Device ID: 0xD7 Erase timeout: 30000 ms, write timeout: 100 ms
Sector Start Addresses: 00000000 RO 00001000 RO 00002000 RO 00003000 RO 00004000 RO 00005000 RO 00006000 RO 00007000 RO 00008000 RO 00009000 RO 0000A000 RO 0000B000 RO 0000C000 RO 0000D000 RO 0000E000 RO 0000F000 RO 00010000 RO 00011000 RO 00012000 RO 00013000 RO 00014000 RO 00015000 RO 00016000 RO 00017000 RO 00018000 RO 00019000 RO 0001A000 RO 0001B000 RO 0001C000 0001D000 0001E000 0001F000 00020000 00021000 00022000 00023000 00024000 00025000 00026000 00027000 00028000 00029000 0002A000 0002B000 0002C000 0002D000 0002E000 0002F000 00030000 00031000 00032000 00033000 00034000 00035000 00036000 00037000 00038000 00039000 0003A000 0003B000 0003C000 0003D000 0003E000 0003F000 00040000 00041000 00042000 00043000 00044000 00045000 00046000 00047000 00048000 00049000 0004A000 0004B000 0004C000 0004D000 0004E000 0004F000 00050000 00051000 00052000 00053000 00054000 00055000 00056000 00057000 00058000 00059000 0005A000 0005B000 0005C000 0005D000 0005E000 0005F000 00060000 RO 00061000 RO 00062000 RO 00063000 RO 00064000 RO 00065000 RO 00066000 RO 00067000 RO 00068000 RO 00069000 RO 0006A000 RO 0006B000 RO 0006C000 RO 0006D000 RO 0006E000 RO 0006F000 RO 00070000 RO 00071000 RO 00072000 RO 00073000 RO 00074000 RO 00075000 RO 00076000 RO 00077000 RO 00078000 RO 00079000 RO 0007A000 RO 0007B000 RO 0007C000 RO 0007D000 RO 0007E000 RO 0007F000 RO
best regards, Po-Yu Chuang
Hi Po-Yu Chuang,
Le 24/02/2011 07:06, Po-Yu Chuang a écrit :
Hi all,
I am using relocation fixed a320evb (arm) u-boot.
I noticed something weird about the output of command flinfo.
The size of u-boot.bin is 129156 (0x1F884), but the protected range is only 0 ~ 0x1bfff.
I guess that it is because u-boot protects _start ~ __bss_start, but there are some other things in u-boot.bin after __bss_start, e.g. .rel.dyn section and .dynsym section
You're most probable right about this -- this size error has to be fixed. All reasers please see the 'BTW' at the end of my post.
That is quite strange. According to arch/arm/cpu/arm920t/u-boot.lds, .rel.dyn and .dynsym sections should be placed before __bss_start. However, objdump shows that they are not at where they should be.
Do I understand correctly?
Not quite. Actually, relocation sections should start from the same location as BSS -- they are overlaid at the same location, and this is voluntary.
The relocation sections are only needed and useful before relocation, where BSS should not be used anyway.
BSS only exists after relocation, where relocation tables are no more useful.
Thus, to minimize RAM and FLASH footprints, the two are overlaid at the same location.
Does anybody have similar situation?
Just about all people who use ARM U-Boot since the overlay was introduced. :)
0001bf1c g .bss 00000000 __bss_start
0001bf1c g .rel.dyn 00000000 __rel_dyn_start
0001f73c g .dynsym 00000000 __dynsym_start 0001f73c g .rel.dyn 00000000 __rel_dyn_end
This is normal as far as layout is concerned: BSS and .rel.dyn start at the same offset, and .dynsym follows .rel.dyn.
You're right that U-Boot protection should cover the whole of U-Boot, including the relocation tables. I *think* protection uses a monitor length define for this. Can you verify this point, and check what your "monitor length" define amounts to? Maybe it does not cover the relocation tables any more.
BTW,
Would it not be better to compute the actual image size rather than rely on a define?
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
For code other than the U-Boot image itself (loaders, utilities), a 'du -b u-boot.bin | cut -f 1' should be ok, provided the image is built first, which I think is already the case.
Amicalement,
Dear Albert ARIBAUD,
In message 4D660030.409@free.fr you wrote:
You're right that U-Boot protection should cover the whole of U-Boot, including the relocation tables...
True. This was overlooked during all thie relocation rework.
... I *think* protection uses a monitorlength define for this. Can you verify this point, and check what your "monitor length" define amounts to? Maybe it does not cover the relocation tables any more.
I'm not sure if all boards do the same; the common CFI driver ("drivers/mtd/cfi_flash.c", search for "Monitor protection ON by default) protects an area of "monitor_flash_len" bytes starting at CONFIG_SYS_MONITOR_BASE plus the environment sector(s).
monitor_flash_len gets normally computed in archh/*/lib/board.c; on ARM, we have this:
monitor_flash_len = _bss_start_ofs;
Would it not be better to compute the actual image size rather than rely on a define?
This is already the case. It's just that the computation is not correct any more.
Best regards,
Wolfgang Denk
Hi Albert,
On Thu, Feb 24, 2011 at 2:52 PM, Albert ARIBAUD albert.aribaud@free.fr wrote:
Hi Po-Yu Chuang,
Le 24/02/2011 07:06, Po-Yu Chuang a écrit :
That is quite strange. According to arch/arm/cpu/arm920t/u-boot.lds, .rel.dyn and .dynsym sections should be placed before __bss_start. However, objdump shows that they are not at where they should be.
Do I understand correctly?
Not quite. Actually, relocation sections should start from the same location as BSS -- they are overlaid at the same location, and this is voluntary.
The relocation sections are only needed and useful before relocation, where BSS should not be used anyway.
BSS only exists after relocation, where relocation tables are no more useful.
Thus, to minimize RAM and FLASH footprints, the two are overlaid at the same location.
I got it. Thanks for your explanation.
Does anybody have similar situation?
Just about all people who use ARM U-Boot since the overlay was introduced. :)
0001bf1c g .bss 00000000 __bss_start
0001bf1c g .rel.dyn 00000000 __rel_dyn_start
0001f73c g .dynsym 00000000 __dynsym_start 0001f73c g .rel.dyn 00000000 __rel_dyn_end
This is normal as far as layout is concerned: BSS and .rel.dyn start at the same offset, and .dynsym follows .rel.dyn.
You're right that U-Boot protection should cover the whole of U-Boot, including the relocation tables. I *think* protection uses a monitor length define for this. Can you verify this point, and check what your "monitor length" define amounts to? Maybe it does not cover the relocation tables any more.
The monitor length is not defined by macro. It is calculated.
In drivers/mtd/cfi_flash.c:
/* Monitor protection ON by default */ #if (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) && \ (!defined(CONFIG_MONITOR_IS_IN_RAM)) flash_protect (FLAG_PROTECT_SET, CONFIG_SYS_MONITOR_BASE, CONFIG_SYS_MONITOR_BASE + monitor_flash_len - 1, flash_get_info(CONFIG_SYS_MONITOR_BASE)); #endif
monitor_flash_len is defined in arch/arm/lib/board.c and is calculated by board_init_r():
monitor_flash_len = _bss_start_ofs;
Which is in arch/arm/cpu/arm920t/start.S in my case:
.globl _bss_start_ofs _bss_start_ofs: .word __bss_start - _start
So I guess we should use another way to calculate monitor_flash_len.
BTW,
Would it not be better to compute the actual image size rather than rely on a define?
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
But _end means end of bss section, right? I think _end is not the right choice.
For code other than the U-Boot image itself (loaders, utilities), a 'du -b u-boot.bin | cut -f 1' should be ok, provided the image is built first, which I think is already the case.
best regards, Po-Yu Chuang
Hello Albert,
Albert ARIBAUD wrote:
Le 24/02/2011 07:06, Po-Yu Chuang a écrit :
Hi all,
I am using relocation fixed a320evb (arm) u-boot.
I noticed something weird about the output of command flinfo.
The size of u-boot.bin is 129156 (0x1F884), but the protected range is only 0 ~ 0x1bfff.
I guess that it is because u-boot protects _start ~ __bss_start, but there are some other things in u-boot.bin after __bss_start, e.g. .rel.dyn section and .dynsym section
You're most probable right about this -- this size error has to be fixed. All reasers please see the 'BTW' at the end of my post.
That is quite strange. According to arch/arm/cpu/arm920t/u-boot.lds, .rel.dyn and .dynsym sections should be placed before __bss_start. However, objdump shows that they are not at where they should be.
Do I understand correctly?
Not quite. Actually, relocation sections should start from the same location as BSS -- they are overlaid at the same location, and this is voluntary.
The relocation sections are only needed and useful before relocation, where BSS should not be used anyway.
BSS only exists after relocation, where relocation tables are no more useful.
Thus, to minimize RAM and FLASH footprints, the two are overlaid at the same location.
Does anybody have similar situation?
Just about all people who use ARM U-Boot since the overlay was introduced. :)
0001bf1c g .bss 00000000 __bss_start
0001bf1c g .rel.dyn 00000000 __rel_dyn_start
0001f73c g .dynsym 00000000 __dynsym_start 0001f73c g .rel.dyn 00000000 __rel_dyn_end
This is normal as far as layout is concerned: BSS and .rel.dyn start at the same offset, and .dynsym follows .rel.dyn.
You're right that U-Boot protection should cover the whole of U-Boot, including the relocation tables. I *think* protection uses a monitor length define for this. Can you verify this point, and check what your "monitor length" define amounts to? Maybe it does not cover the relocation tables any more.
The bin length is calculated in arch/arm/lib/board.c, but it seems to me not correct ... :-(
in board_init_f():
gd->mon_len = _bss_end_ofs;
that seems correct to me, but later in board_init_r()
monitor_flash_len = _bss_start_ofs;
which is used for example in ./drivers/mtd/cfi_flash.c for protecting the flash sectors ... so this should be fixed.
BTW,
Would it not be better to compute the actual image size rather than rely on a define?
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
For code other than the U-Boot image itself (loaders, utilities), a 'du -b u-boot.bin | cut -f 1' should be ok, provided the image is built first, which I think is already the case.
Amicalement,
bye, Heiko
Hi Heiko,
On Thu, Feb 24, 2011 at 3:33 PM, Heiko Schocher hs@denx.de wrote:
Albert ARIBAUD wrote: The bin length is calculated in arch/arm/lib/board.c, but it seems to me not correct ... :-(
in board_init_f():
gd->mon_len = _bss_end_ofs;
that seems correct to me, but later in board_init_r()
monitor_flash_len = _bss_start_ofs;
which is used for example in ./drivers/mtd/cfi_flash.c for protecting the flash sectors ... so this should be fixed.
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I guess we need a __dynsym_end in all u-boot.lds files.
For code other than the U-Boot image itself (loaders, utilities), a 'du -b u-boot.bin | cut -f 1' should be ok, provided the image is built first, which I think is already the case.
best regards, Po-Yu Chuang
Le 24/02/2011 08:40, Po-Yu Chuang a écrit :
Hi Heiko,
On Thu, Feb 24, 2011 at 3:33 PM, Heiko Schocherhs@denx.de wrote:
Albert ARIBAUD wrote: The bin length is calculated in arch/arm/lib/board.c, but it seems to me not correct ... :-(
in board_init_f():
gd->mon_len = _bss_end_ofs;
that seems correct to me, but later in board_init_r()
monitor_flash_len = _bss_start_ofs;
which is used for example in ./drivers/mtd/cfi_flash.c for protecting the flash sectors ... so this should be fixed.
Correct.
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I *knew* this name did not pop up in my mind without a reason. :)
Apologies for not having checked.
I guess we need a __dynsym_end in all u-boot.lds files.
I'd rather go for "_end", which does not tie the solution to dynsym being the last section in the image -- imagine for some reason we move dynsym inside the image rather than at the end, end the image size will be wrong again. With _end, and a suitable comment in the LD file... this will be averted where-ever dynsym ends up.
After all, the bug you uncovered was due to using the wrong symbol, a BSS related one rather than an image-related one, in the first place; so let's try and learn from past mistakes.
best regards, Po-Yu Chuang
Amicalement,
Dear Albert ARIBAUD,
In message 4D6611A7.5050802@free.fr you wrote:
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I *knew* this name did not pop up in my mind without a reason. :)
Apologies for not having checked.
I guess we need a __dynsym_end in all u-boot.lds files.
I'd rather go for "_end", which does not tie the solution to dynsym being the last section in the image -- imagine for some reason we move dynsym inside the image rather than at the end, end the image size will be wrong again. With _end, and a suitable comment in the LD file... this will be averted where-ever dynsym ends up.
After all, the bug you uncovered was due to using the wrong symbol, a BSS related one rather than an image-related one, in the first place; so let's try and learn from past mistakes.
I think the location of _end in the linker scripts needs to be changed - so far it covers the maximum of dynsym and bss, but it should refer to the end of dynsym only.
Best regards,
Wolfgang Denk
Le 24/02/2011 10:39, Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message4D6611A7.5050802@free.fr you wrote:
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I *knew* this name did not pop up in my mind without a reason. :)
Apologies for not having checked.
I guess we need a __dynsym_end in all u-boot.lds files.
I'd rather go for "_end", which does not tie the solution to dynsym being the last section in the image -- imagine for some reason we move dynsym inside the image rather than at the end, end the image size will be wrong again. With _end, and a suitable comment in the LD file... this will be averted where-ever dynsym ends up.
After all, the bug you uncovered was due to using the wrong symbol, a BSS related one rather than an image-related one, in the first place; so let's try and learn from past mistakes.
I think the location of _end in the linker scripts needs to be changed
- so far it covers the maximum of dynsym and bss, but it should refer
to the end of dynsym only.
Po-Yu Chuang, can you verify Wolfgang's suggestion (and make sure no other place depends on _end) and submit a patch? As this is a fix, a quick patch could still be pulled in for the upcoming release.
Best regards,
Wolfgang Denk
Amicalement,
Hi Albert,
On Thu, Feb 24, 2011 at 7:58 PM, Albert ARIBAUD albert.aribaud@free.fr wrote:
Le 24/02/2011 10:39, Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message4D6611A7.5050802@free.fr you wrote:
In the U-Boot image itself, knowing the image size could be achieved in ARM by using a general _end symbol that would be set after the last image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I *knew* this name did not pop up in my mind without a reason. :)
Apologies for not having checked.
I guess we need a __dynsym_end in all u-boot.lds files.
I'd rather go for "_end", which does not tie the solution to dynsym being the last section in the image -- imagine for some reason we move dynsym inside the image rather than at the end, end the image size will be wrong again. With _end, and a suitable comment in the LD file... this will be averted where-ever dynsym ends up.
After all, the bug you uncovered was due to using the wrong symbol, a BSS related one rather than an image-related one, in the first place; so let's try and learn from past mistakes.
I think the location of _end in the linker scripts needs to be changed
- so far it covers the maximum of dynsym and bss, but it should refer
to the end of dynsym only.
Po-Yu Chuang, can you verify Wolfgang's suggestion (and make sure no other place depends on _end) and submit a patch? As this is a fix, a quick patch could still be pulled in for the upcoming release.
I think there is a problem in Wolfgang's suggestion.
Those start.S files assume that BSS section is from __bss_start to _end. If we change _end to the end of .dynsym, then we will not clear BSS correctly.
I agree with your comment about _end is a better choice than __dynsym_end, but if we changed _end, maybe we need __bss_end? Either way, we need to modify all the u-boot.lds and/or start.S.
I still don't know what is the best solution.
best regards, Po-Yu Chuang
Le 24/02/2011 13:58, Po-Yu Chuang a écrit :
Hi Albert,
On Thu, Feb 24, 2011 at 7:58 PM, Albert ARIBAUDalbert.aribaud@free.fr wrote:
Le 24/02/2011 10:39, Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message4D6611A7.5050802@free.fr you wrote:
> In the U-Boot image itself, knowing the image size could be achieved > in > ARM by using a general _end symbol that would be set after the last > image output section, so _end-_start would equal the image size.
we have such a "_end" in u-boot.lds files.
I *knew* this name did not pop up in my mind without a reason. :)
Apologies for not having checked.
I guess we need a __dynsym_end in all u-boot.lds files.
I'd rather go for "_end", which does not tie the solution to dynsym being the last section in the image -- imagine for some reason we move dynsym inside the image rather than at the end, end the image size will be wrong again. With _end, and a suitable comment in the LD file... this will be averted where-ever dynsym ends up.
After all, the bug you uncovered was due to using the wrong symbol, a BSS related one rather than an image-related one, in the first place; so let's try and learn from past mistakes.
I think the location of _end in the linker scripts needs to be changed
- so far it covers the maximum of dynsym and bss, but it should refer
to the end of dynsym only.
Po-Yu Chuang, can you verify Wolfgang's suggestion (and make sure no other place depends on _end) and submit a patch? As this is a fix, a quick patch could still be pulled in for the upcoming release.
I think there is a problem in Wolfgang's suggestion.
Those start.S files assume that BSS section is from __bss_start to _end. If we change _end to the end of .dynsym, then we will not clear BSS correctly.
I agree with your comment about _end is a better choice than __dynsym_end, but if we changed _end, maybe we need __bss_end? Either way, we need to modify all the u-boot.lds and/or start.S.
I still don't know what is the best solution.
Thanks for pointing this out: indeed, _end is actually used as BSS's end, not as the image's end -- I did not find any other use of it.
However, not only ARM uses _end: how do we want to handle the change overall?
best regards, Po-Yu Chuang
Amicalement,
Dear Albert ARIBAUD,
In message 4D665969.1@free.fr you wrote:
Thanks for pointing this out: indeed, _end is actually used as BSS's end, not as the image's end -- I did not find any other use of it.
However, not only ARM uses _end: how do we want to handle the change overall?
Sounds as if we need a little fix/cleanup for _all_ systems...
Best regards,
Wolfgang Denk
Le 24/02/2011 14:41, Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message4D665969.1@free.fr you wrote:
Thanks for pointing this out: indeed, _end is actually used as BSS's end, not as the image's end -- I did not find any other use of it.
However, not only ARM uses _end: how do we want to handle the change overall?
Sounds as if we need a little fix/cleanup for _all_ systems...
Do you mean a first commit to turn _end into _bss_end everywhere in U-Boot, then a second commit to reintroduce _end and fix the issue with monitor protection? Or even maybe a single commit for both?
Best regards,
Wolfgang Denk
Amicalement,
Dear Albert ARIBAUD,
In message 4D6680E4.1070405@free.fr you wrote:
Do you mean a first commit to turn _end into _bss_end everywhere in U-Boot, then a second commit to reintroduce _end and fix the issue with monitor protection? Or even maybe a single commit for both?
We should re-check if the current use of _end; excluding the linker scripts, I see these uses:
arch/powerpc/cpu/74xx_7xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/74xx_7xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc512x/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc512x/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc5xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc5xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc5xxx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc5xxx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8220/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8220/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc824x/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc824x/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8260/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8260/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc83xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc83xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc85xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc85xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc86xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc86xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/ppc4xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/ppc4xx/start.S: lwz r4,GOT(_end) arch/powerpc/lib/board.c:extern ulong _end; arch/powerpc/lib/board.c: len = (ulong)&_end - CONFIG_SYS_MONITOR_BASE; arch/arm/cpu/arm1136/start.S: .word _end - _start arch/arm/cpu/arm1176/start.S: .word _end - _start arch/arm/cpu/arm720t/start.S: .word _end - _start arch/arm/cpu/arm920t/start.S: .word _end - _start arch/arm/cpu/arm925t/start.S: .word _end - _start arch/arm/cpu/arm926ejs/start.S: .word _end - _start arch/arm/cpu/arm946es/start.S: .word _end - _start arch/arm/cpu/arm_intcm/start.S: .word _end - _start arch/arm/cpu/ixp/start.S: .word _end - _start arch/arm/cpu/lh7a40x/start.S: .word _end - _start arch/arm/cpu/pxa/start.S: .word _end - _start arch/arm/cpu/s3c44b0/start.S: .word _end - _start arch/arm/cpu/sa1100/start.S: .word _end - _start arch/arm/cpu/armv7/start.S: .word _end - _start arch/avr32/cpu/start.S: lda.w r9, _end arch/avr32/include/asm/sections.h:extern char _end[]; arch/avr32/lib/board.c: (unsigned long)_data, (unsigned long)_end); arch/avr32/lib/board.c: monitor_len = _end - _text; arch/m68k/lib/board.c:extern ulong _end; arch/m68k/lib/board.c: len = (ulong)&_end - CONFIG_SYS_MONITOR_BASE; arch/microblaze/cpu/start.S: addi r4, r0, __end arch/microblaze/cpu/start.S: rsub r4, r5, r4 /* size = __end - __text_start */ arch/nios2/cpu/start.S: * and between __bss_start and _end. arch/nios2/cpu/start.S: movhi r6, %hi(_end) arch/nios2/cpu/start.S: ori r6, r6, %lo(_end) board/mousse/u-boot.lds.ram: _end = . ; board/mousse/u-boot.lds.rom: _end = . ; examples/standalone/stubs.c:extern unsigned long __bss_start, _end; examples/standalone/stubs.c: while (cp < (unsigned char *)&_end) {
As far as I can tell, arch/powerpc/lib/board.c , arch/avr32/lib/board.c and examples/standalone/stubs.c actually all mean __bss_end, as well as all the start.S files, so this seems to be consistently used at least.
Best regards,
Wolfgang Denk
Le 24/02/2011 19:38, Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message4D6680E4.1070405@free.fr you wrote:
Do you mean a first commit to turn _end into _bss_end everywhere in U-Boot, then a second commit to reintroduce _end and fix the issue with monitor protection? Or even maybe a single commit for both?
We should re-check if the current use of _end; excluding the linker scripts, I see these uses:
arch/powerpc/cpu/74xx_7xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/74xx_7xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc512x/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc512x/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc5xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc5xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc5xxx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc5xxx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8220/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8220/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc824x/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc824x/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8260/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8260/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc83xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc83xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc85xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc85xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc86xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc86xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/mpc8xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/mpc8xx/start.S: lwz r4,GOT(_end) arch/powerpc/cpu/ppc4xx/start.S: GOT_ENTRY(_end) arch/powerpc/cpu/ppc4xx/start.S: lwz r4,GOT(_end) arch/powerpc/lib/board.c:extern ulong _end; arch/powerpc/lib/board.c: len = (ulong)&_end - CONFIG_SYS_MONITOR_BASE; arch/arm/cpu/arm1136/start.S: .word _end - _start arch/arm/cpu/arm1176/start.S: .word _end - _start arch/arm/cpu/arm720t/start.S: .word _end - _start arch/arm/cpu/arm920t/start.S: .word _end - _start arch/arm/cpu/arm925t/start.S: .word _end - _start arch/arm/cpu/arm926ejs/start.S: .word _end - _start arch/arm/cpu/arm946es/start.S: .word _end - _start arch/arm/cpu/arm_intcm/start.S: .word _end - _start arch/arm/cpu/ixp/start.S: .word _end - _start arch/arm/cpu/lh7a40x/start.S: .word _end - _start arch/arm/cpu/pxa/start.S: .word _end - _start arch/arm/cpu/s3c44b0/start.S: .word _end - _start arch/arm/cpu/sa1100/start.S: .word _end - _start arch/arm/cpu/armv7/start.S: .word _end - _start arch/avr32/cpu/start.S: lda.w r9, _end arch/avr32/include/asm/sections.h:extern char _end[]; arch/avr32/lib/board.c: (unsigned long)_data, (unsigned long)_end); arch/avr32/lib/board.c: monitor_len = _end - _text; arch/m68k/lib/board.c:extern ulong _end; arch/m68k/lib/board.c: len = (ulong)&_end - CONFIG_SYS_MONITOR_BASE; arch/microblaze/cpu/start.S: addi r4, r0, __end arch/microblaze/cpu/start.S: rsub r4, r5, r4 /* size = __end - __text_start */ arch/nios2/cpu/start.S: * and between __bss_start and _end. arch/nios2/cpu/start.S: movhi r6, %hi(_end) arch/nios2/cpu/start.S: ori r6, r6, %lo(_end) board/mousse/u-boot.lds.ram: _end = . ; board/mousse/u-boot.lds.rom: _end = . ; examples/standalone/stubs.c:extern unsigned long __bss_start, _end; examples/standalone/stubs.c: while (cp< (unsigned char *)&_end) {
As far as I can tell, arch/powerpc/lib/board.c , arch/avr32/lib/board.c and examples/standalone/stubs.c actually all mean __bss_end, as well as all the start.S files, so this seems to be consistently used at least.
I'd come to the same conclusion on my side. However you don't say how you would like the change to be done. Would a single commit for renaming _end into _bss_end and then creating _end after .dynsym be ok for you?
Best regards,
Wolfgang Denk
Amicalement,
Dear Albert ARIBAUD,
In message 4D66A7CD.9020309@free.fr you wrote:
As far as I can tell, arch/powerpc/lib/board.c , arch/avr32/lib/board.c and examples/standalone/stubs.c actually all mean __bss_end, as well as all the start.S files, so this seems to be consistently used at least.
I'd come to the same conclusion on my side. However you don't say how you would like the change to be done. Would a single commit for renaming _end into _bss_end and then creating _end after .dynsym be ok for you?
Ah, sorry.
Well, these are two logically separate things: the first is just a renamer that should basicly result in the very same images built on all systems (which can easily be verified semi-automatically). The second step would be adding _end at the end of the used data area (end of .dynsym) on ARM only.
Best regards,
Wolfgang Denk
Dear Wolfgang and Albert,
On Fri, Feb 25, 2011 at 4:17 AM, Wolfgang Denk wd@denx.de wrote:
Dear Albert ARIBAUD,
In message 4D66A7CD.9020309@free.fr you wrote:
As far as I can tell, arch/powerpc/lib/board.c , arch/avr32/lib/board.c and examples/standalone/stubs.c actually all mean __bss_end, as well as all the start.S files, so this seems to be consistently used at least.
I'd come to the same conclusion on my side. However you don't say how you would like the change to be done. Would a single commit for renaming _end into _bss_end and then creating _end after .dynsym be ok for you?
Ah, sorry.
Well, these are two logically separate things: the first is just a renamer that should basicly result in the very same images built on all systems (which can easily be verified semi-automatically). The second step would be adding _end at the end of the used data area (end of .dynsym) on ARM only.
I don't have the environment to build other platforms, so... does any of you please to provide such patches?
regards, Po-Yu Chuang
Hi Po-Yu Chuang,
Le 25/02/2011 03:41, Po-Yu Chuang a écrit :
Dear Wolfgang and Albert,
On Fri, Feb 25, 2011 at 4:17 AM, Wolfgang Denkwd@denx.de wrote:
Dear Albert ARIBAUD,
In message4D66A7CD.9020309@free.fr you wrote:
As far as I can tell, arch/powerpc/lib/board.c , arch/avr32/lib/board.c and examples/standalone/stubs.c actually all mean __bss_end, as well as all the start.S files, so this seems to be consistently used at least.
I'd come to the same conclusion on my side. However you don't say how you would like the change to be done. Would a single commit for renaming _end into _bss_end and then creating _end after .dynsym be ok for you?
Ah, sorry.
Well, these are two logically separate things: the first is just a renamer that should basicly result in the very same images built on all systems (which can easily be verified semi-automatically). The second step would be adding _end at the end of the used data area (end of .dynsym) on ARM only.
I don't have the environment to build other platforms, so... does any of you please to provide such patches?
I suggest you do the changes for all of U-Boot and test them on your specific board, then submit the patchset and Cc: all architecture custodians, who will run tests on their respective architectures.
http://www.denx.de/wiki/U-Boot/Patches will give you all info on preparing and sending patches -- remember that's a two-patch set. IMO you don't necessarily need a cover letter as long as each patch has a clear commit message.
http://www.denx.de/wiki/U-Boot/Custodians will give you the list of all custodians; you can pick the architecture ones there.
All this, of course, if Wolfgang agrees to this suggestion.
Amicalement,
Dear Albert ARIBAUD,
In message 4D674D85.40904@free.fr you wrote:
All this, of course, if Wolfgang agrees to this suggestion.
I fully agree.
Best regards,
Wolfgang Denk
Dear Wolfgang and Albert,
On Fri, Feb 25, 2011 at 3:15 PM, Wolfgang Denk wd@denx.de wrote:
Dear Albert ARIBAUD,
In message 4D674D85.40904@free.fr you wrote:
All this, of course, if Wolfgang agrees to this suggestion.
I fully agree.
All right, but please give me some time. I need to study how to use sed to do this. :-p
best regards, Po-Yu Chuang
Dear Wolfgang,
On Fri, Feb 25, 2011 at 4:00 PM, Po-Yu Chuang ratbert.chuang@gmail.com wrote:
Dear Wolfgang and Albert,
On Fri, Feb 25, 2011 at 3:15 PM, Wolfgang Denk wd@denx.de wrote:
Dear Albert ARIBAUD,
In message 4D674D85.40904@free.fr you wrote:
All this, of course, if Wolfgang agrees to this suggestion.
I fully agree.
All right, but please give me some time. I need to study how to use sed to do this. :-p
I tried the following command and it seems work:
for f in `find * -type f`; do sed -i 's/<_end>/__bss_end/g' $f; done
but when I tried to build, I got some error in standalone example:
make[1]: Entering directory `/home/ratbert/linux/u-boot/examples/standalone' arm-none-linux-gnueabi-gcc -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0 -I/home/ratbert/linux/u-boot/include -fno-builtin -ffreestanding -nostdinc -isystem /home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv4 -Wall -Wstrict-prototypes -fno-stack-protector -fno-toplevel-reorder -o hello_world.o hello_world.c -c arm-none-linux-gnueabi-gcc -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0 -I/home/ratbert/linux/u-boot/include -fno-builtin -ffreestanding -nostdinc -isystem /home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv4 -Wall -Wstrict-prototypes -fno-stack-protector -fno-toplevel-reorder -o stubs.o stubs.c -c arm-none-linux-gnueabi-ld -r -o libstubs.o stubs.o arm-none-linux-gnueabi-ld -g -Ttext 0xc100000 \ -o hello_world -e hello_world hello_world.o libstubs.o \
-L/home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/fa526 -lgcc libstubs.o: In function `app_startup': /home/ratbert/linux/u-boot/examples/standalone/stubs.c:206: undefined reference to `__bss_end' make[1]: *** [hello_world] Error 1 make[1]: Leaving directory `/home/ratbert/linux/u-boot/examples/standalone' make: *** [examples/standalone] Error 2
It looks like that _end as end of BSS section is a convention to GNU toolchain. Should I simply fix it by hand, or should we reconsider the whole naming thing?
best regards, Po-Yu Chuang
Le 01/03/2011 09:31, Po-Yu Chuang a écrit :
Dear Wolfgang,
On Fri, Feb 25, 2011 at 4:00 PM, Po-Yu Chuangratbert.chuang@gmail.com wrote:
Dear Wolfgang and Albert,
On Fri, Feb 25, 2011 at 3:15 PM, Wolfgang Denkwd@denx.de wrote:
Dear Albert ARIBAUD,
In message4D674D85.40904@free.fr you wrote:
All this, of course, if Wolfgang agrees to this suggestion.
I fully agree.
All right, but please give me some time. I need to study how to use sed to do this. :-p
I tried the following command and it seems work:
for f in `find * -type f`; do sed -i 's/<_end>/__bss_end/g' $f; done
but when I tried to build, I got some error in standalone example:
make[1]: Entering directory `/home/ratbert/linux/u-boot/examples/standalone' arm-none-linux-gnueabi-gcc -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0 -I/home/ratbert/linux/u-boot/include -fno-builtin -ffreestanding -nostdinc -isystem /home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv4 -Wall -Wstrict-prototypes -fno-stack-protector -fno-toplevel-reorder -o hello_world.o hello_world.c -c arm-none-linux-gnueabi-gcc -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DCONFIG_SYS_TEXT_BASE=0 -I/home/ratbert/linux/u-boot/include -fno-builtin -ffreestanding -nostdinc -isystem /home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv4 -Wall -Wstrict-prototypes -fno-stack-protector -fno-toplevel-reorder -o stubs.o stubs.c -c arm-none-linux-gnueabi-ld -r -o libstubs.o stubs.o arm-none-linux-gnueabi-ld -g -Ttext 0xc100000 \ -o hello_world -e hello_world hello_world.o libstubs.o \
-L/home/ratbert/linux/arm-none-linux-gnueabi-4.4.0_ARMv4/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.0/fa526 -lgcc libstubs.o: In function `app_startup': /home/ratbert/linux/u-boot/examples/standalone/stubs.c:206: undefined reference to `__bss_end' make[1]: *** [hello_world] Error 1 make[1]: Leaving directory `/home/ratbert/linux/u-boot/examples/standalone' make: *** [examples/standalone] Error 2
It looks like that _end as end of BSS section is a convention to GNU toolchain. Should I simply fix it by hand, or should we reconsider the whole naming thing?
Indeed, since the link command line does not mention a linker file, then the default toolchain linker file was invoked. You can see the linker script using ld --verbose (be sure to use the toolchain's ld, not your host's). Mine (ELDK4.2 and CodeSourcery 2010q1) define __bss_start and __bss_start__ before the BSS section, and _bss_end__ and __bss_end__ after it, then __end__ and _end after that. No surprise that __bss_end did not work.
But examples (ang more generally, standalone applications) are not supposed to use symbols from U-Boot linker files; so there is no reason to replace __bss_end in these.
Still, we *could* align on the ELDK/CS linkers' output and use _bss_end__ and/or __bss_end__ rather than __bss_end for U-Boot, unless some other toolchain has radically different default linker scripts. Comments anyone?
best regards, Po-Yu Chuang
Amicalement,
Dear Albert,
On Tue, Mar 1, 2011 at 8:22 PM, Albert ARIBAUD albert.aribaud@free.fr wrote:
Le 01/03/2011 09:31, Po-Yu Chuang a écrit :
On Fri, Feb 25, 2011 at 4:00 PM, Po-Yu Chuangratbert.chuang@gmail.com wrote:
I tried the following command and it seems work:
for f in `find * -type f`; do sed -i 's/<_end>/__bss_end/g' $f; done
but when I tried to build, I got some error in standalone example:
[snip]
libstubs.o: In function `app_startup': /home/ratbert/linux/u-boot/examples/standalone/stubs.c:206: undefined reference to `__bss_end' make[1]: *** [hello_world] Error 1 make[1]: Leaving directory `/home/ratbert/linux/u-boot/examples/standalone' make: *** [examples/standalone] Error 2
It looks like that _end as end of BSS section is a convention to GNU toolchain. Should I simply fix it by hand, or should we reconsider the whole naming thing?
Indeed, since the link command line does not mention a linker file, then the default toolchain linker file was invoked. You can see the linker script using ld --verbose (be sure to use the toolchain's ld, not your host's). Mine (ELDK4.2 and CodeSourcery 2010q1) define __bss_start and __bss_start__ before the BSS section, and _bss_end__ and __bss_end__ after it, then __end__ and _end after that. No surprise that __bss_end did not work.
But examples (ang more generally, standalone applications) are not supposed to use symbols from U-Boot linker files; so there is no reason to replace __bss_end in these.
Still, we *could* align on the ELDK/CS linkers' output and use _bss_end__ and/or __bss_end__ rather than __bss_end for U-Boot, unless some other toolchain has radically different default linker scripts. Comments anyone?
I prefer the __bss_end__ solution. Not only because it doesn't require manual fixes after sed replacement command, but it also makes the semantics of app_startup() of the standalone example program clearer. If there is no other comment, I will submit patches using __bss_end__ later.
best regards, Po-Yu Chuang
participants (4)
-
Albert ARIBAUD -
Heiko Schocher -
Po-Yu Chuang -
Wolfgang Denk