public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] ARM: Incorrect ROM protection range?
@ 2011-02-24  6:06 Po-Yu Chuang
  2011-02-24  6:52 ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-24  6:06 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  6:06 [U-Boot] ARM: Incorrect ROM protection range? Po-Yu Chuang
@ 2011-02-24  6:52 ` Albert ARIBAUD
  2011-02-24  7:07   ` Wolfgang Denk
                     ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24  6:52 UTC (permalink / raw)
  To: u-boot

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  6:52 ` Albert ARIBAUD
@ 2011-02-24  7:07   ` Wolfgang Denk
  2011-02-24  7:11   ` Po-Yu Chuang
  2011-02-24  7:33   ` Heiko Schocher
  2 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-24  7:07 UTC (permalink / raw)
  To: u-boot

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 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.

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

-- 
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
Those who do not  understand  Unix  are  condemned  to  reinvent  it,
poorly.              - Henry Spencer, University of Toronto Unix hack

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  6:52 ` Albert ARIBAUD
  2011-02-24  7:07   ` Wolfgang Denk
@ 2011-02-24  7:11   ` Po-Yu Chuang
  2011-02-24  7:33   ` Heiko Schocher
  2 siblings, 0 replies; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-24  7:11 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  6:52 ` Albert ARIBAUD
  2011-02-24  7:07   ` Wolfgang Denk
  2011-02-24  7:11   ` Po-Yu Chuang
@ 2011-02-24  7:33   ` Heiko Schocher
  2011-02-24  7:40     ` Po-Yu Chuang
  2 siblings, 1 reply; 23+ messages in thread
From: Heiko Schocher @ 2011-02-24  7:33 UTC (permalink / raw)
  To: u-boot

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  7:33   ` Heiko Schocher
@ 2011-02-24  7:40     ` Po-Yu Chuang
  2011-02-24  8:07       ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-24  7:40 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  7:40     ` Po-Yu Chuang
@ 2011-02-24  8:07       ` Albert ARIBAUD
  2011-02-24  9:39         ` Wolfgang Denk
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24  8:07 UTC (permalink / raw)
  To: u-boot

Le 24/02/2011 08:40, Po-Yu Chuang a ?crit :
> 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.

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  8:07       ` Albert ARIBAUD
@ 2011-02-24  9:39         ` Wolfgang Denk
  2011-02-24 11:58           ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-24  9:39 UTC (permalink / raw)
  To: u-boot

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

-- 
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
If in any problem you find yourself doing an immense amount of  work,
the answer can be obtained by simple inspection.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24  9:39         ` Wolfgang Denk
@ 2011-02-24 11:58           ` Albert ARIBAUD
  2011-02-24 12:58             ` Po-Yu Chuang
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24 11:58 UTC (permalink / raw)
  To: u-boot

Le 24/02/2011 10:39, Wolfgang Denk a ?crit :
> 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.

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 11:58           ` Albert ARIBAUD
@ 2011-02-24 12:58             ` Po-Yu Chuang
  2011-02-24 13:13               ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-24 12:58 UTC (permalink / raw)
  To: u-boot

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 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.
>
> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 12:58             ` Po-Yu Chuang
@ 2011-02-24 13:13               ` Albert ARIBAUD
  2011-02-24 13:41                 ` Wolfgang Denk
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24 13:13 UTC (permalink / raw)
  To: u-boot

Le 24/02/2011 13:58, Po-Yu Chuang a ?crit :
> 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 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.
>>
>> 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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 13:13               ` Albert ARIBAUD
@ 2011-02-24 13:41                 ` Wolfgang Denk
  2011-02-24 16:01                   ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-24 13:41 UTC (permalink / raw)
  To: u-boot

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

-- 
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
panic: kernel trap (ignored)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 13:41                 ` Wolfgang Denk
@ 2011-02-24 16:01                   ` Albert ARIBAUD
  2011-02-24 18:38                     ` Wolfgang Denk
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24 16:01 UTC (permalink / raw)
  To: u-boot

Le 24/02/2011 14:41, Wolfgang Denk a ?crit :
> 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...

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 16:01                   ` Albert ARIBAUD
@ 2011-02-24 18:38                     ` Wolfgang Denk
  2011-02-24 18:47                       ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-24 18:38 UTC (permalink / raw)
  To: u-boot

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

-- 
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@denx.de
Another dream that failed.  There's nothing sadder.
	-- Kirk, "This side of Paradise", stardate 3417.3

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 18:38                     ` Wolfgang Denk
@ 2011-02-24 18:47                       ` Albert ARIBAUD
  2011-02-24 20:17                         ` Wolfgang Denk
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-24 18:47 UTC (permalink / raw)
  To: u-boot

Le 24/02/2011 19:38, Wolfgang Denk a ?crit :
> 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.

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 18:47                       ` Albert ARIBAUD
@ 2011-02-24 20:17                         ` Wolfgang Denk
  2011-02-25  2:41                           ` Po-Yu Chuang
  0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-24 20:17 UTC (permalink / raw)
  To: u-boot

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

-- 
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
G's Third Law:             In spite of all evidence  to  the  contra-
ry,  the  entire  universe  is composed of only two basic substances:
magic and bullshit.
H's Dictum:                There is no magic ...

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-24 20:17                         ` Wolfgang Denk
@ 2011-02-25  2:41                           ` Po-Yu Chuang
  2011-02-25  6:34                             ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-25  2:41 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-25  2:41                           ` Po-Yu Chuang
@ 2011-02-25  6:34                             ` Albert ARIBAUD
  2011-02-25  7:15                               ` Wolfgang Denk
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-02-25  6:34 UTC (permalink / raw)
  To: u-boot

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 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?

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-25  6:34                             ` Albert ARIBAUD
@ 2011-02-25  7:15                               ` Wolfgang Denk
  2011-02-25  8:00                                 ` Po-Yu Chuang
  0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Denk @ 2011-02-25  7:15 UTC (permalink / raw)
  To: u-boot

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

-- 
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
The inappropriate cannot be beautiful.
             - Frank Lloyd Wright _The Future of Architecture_ (1953)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-25  7:15                               ` Wolfgang Denk
@ 2011-02-25  8:00                                 ` Po-Yu Chuang
  2011-03-01  8:31                                   ` Po-Yu Chuang
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-02-25  8:00 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-02-25  8:00                                 ` Po-Yu Chuang
@ 2011-03-01  8:31                                   ` Po-Yu Chuang
  2011-03-01 12:22                                     ` Albert ARIBAUD
  0 siblings, 1 reply; 23+ messages in thread
From: Po-Yu Chuang @ 2011-03-01  8:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-03-01  8:31                                   ` Po-Yu Chuang
@ 2011-03-01 12:22                                     ` Albert ARIBAUD
  2011-03-02  2:56                                       ` Po-Yu Chuang
  0 siblings, 1 reply; 23+ messages in thread
From: Albert ARIBAUD @ 2011-03-01 12:22 UTC (permalink / raw)
  To: u-boot

Le 01/03/2011 09:31, Po-Yu Chuang a ?crit :
> 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?

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ARM: Incorrect ROM protection range?
  2011-03-01 12:22                                     ` Albert ARIBAUD
@ 2011-03-02  2:56                                       ` Po-Yu Chuang
  0 siblings, 0 replies; 23+ messages in thread
From: Po-Yu Chuang @ 2011-03-02  2:56 UTC (permalink / raw)
  To: u-boot

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 Chuang<ratbert.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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2011-03-02  2:56 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-24  6:06 [U-Boot] ARM: Incorrect ROM protection range? Po-Yu Chuang
2011-02-24  6:52 ` Albert ARIBAUD
2011-02-24  7:07   ` Wolfgang Denk
2011-02-24  7:11   ` Po-Yu Chuang
2011-02-24  7:33   ` Heiko Schocher
2011-02-24  7:40     ` Po-Yu Chuang
2011-02-24  8:07       ` Albert ARIBAUD
2011-02-24  9:39         ` Wolfgang Denk
2011-02-24 11:58           ` Albert ARIBAUD
2011-02-24 12:58             ` Po-Yu Chuang
2011-02-24 13:13               ` Albert ARIBAUD
2011-02-24 13:41                 ` Wolfgang Denk
2011-02-24 16:01                   ` Albert ARIBAUD
2011-02-24 18:38                     ` Wolfgang Denk
2011-02-24 18:47                       ` Albert ARIBAUD
2011-02-24 20:17                         ` Wolfgang Denk
2011-02-25  2:41                           ` Po-Yu Chuang
2011-02-25  6:34                             ` Albert ARIBAUD
2011-02-25  7:15                               ` Wolfgang Denk
2011-02-25  8:00                                 ` Po-Yu Chuang
2011-03-01  8:31                                   ` Po-Yu Chuang
2011-03-01 12:22                                     ` Albert ARIBAUD
2011-03-02  2:56                                       ` Po-Yu Chuang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox