[U-Boot] [PATCH 1/1] armv7: start.S: Fix relocation address caculation
There will have issue if the _start not equal TEXT_BASE when enable relocation. The reason is as the followings,
The _dynsym_start_ofs and _rel_dyn_start_ofs is the offset from _start, so, need use _start address instead of _TEXT_BASE to caculate the rel dyn start and sym table address. This patch also correct the board_init_r function address caculation in relocation area. The addr of board_init_r after relocation is: _board_init_r_offs + relocation start address.
This patch also make code cleanup by removing some useless code
Signed-off-by: Jason Liu r64343@freescale.com --- arch/arm/cpu/armv7/start.S | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 684f2d2..08902b0 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -171,7 +171,6 @@ stack_setup: beq clear_bss /* skip relocation */ #endif mov r1, r6 /* r1 <- scratch for copy_loop */ - ldr r2, _TEXT_BASE ldr r3, _bss_start_ofs add r2, r0, r3 /* r2 <- source end address */
@@ -185,7 +184,7 @@ copy_loop: /* * fix .rel.dyn relocations */ - ldr r0, _TEXT_BASE /* r0 <- Text base */ + adr r0, _start sub r9, r6, r0 /* r9 <- relocation offset */ ldr r10, _dynsym_start_ofs /* r10 <- sym table ofs */ add r10, r10, r0 /* r10 <- sym table in FLASH */ @@ -224,7 +223,6 @@ fixnext: clear_bss: ldr r0, _bss_start_ofs ldr r1, _bss_end_ofs - ldr r3, _TEXT_BASE /* Text base */ mov r4, r6 /* reloc addr */ add r0, r0, r4 add r1, r1, r4 @@ -242,9 +240,7 @@ clbss_l:str r2, [r0] /* clear loop... */ */ jump_2_ram: ldr r0, _board_init_r_ofs - adr r1, _start - add lr, r0, r1 - add lr, lr, r9 + add lr, r0, r4 /* setup parameters for board_init_r */ mov r0, r5 /* gd_t */ mov r1, r6 /* dest_addr */
Hi Jason,
Le 15/12/2010 14:57, Jason Liu a écrit :
There will have issue if the _start not equal TEXT_BASE when enable relocation.
In what case does this happen?
Amicalement,
Hi, Albert,
2010/12/16 Albert ARIBAUD albert.aribaud@free.fr:
Hi Jason,
Le 15/12/2010 14:57, Jason Liu a écrit :
There will have issue if the _start not equal TEXT_BASE when enable relocation.
In what case does this happen?
Some ARM SOC ROM need run the plug-in code first in IRAM and the plugin-in code need appear at the beginning of the u-boot. ROM will check the plugin-in header to do security check and run the plug-in code to init the DDR etc. In this case the _start will be not the same as TEXT_BASE.
Thanks for your comments. Cheers,
Amicalement,
Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Le 16/12/2010 04:04, Jason Liu a écrit :
Hi, Albert,
2010/12/16 Albert ARIBAUDalbert.aribaud@free.fr:
Hi Jason,
Le 15/12/2010 14:57, Jason Liu a écrit :
There will have issue if the _start not equal TEXT_BASE when enable relocation.
In what case does this happen?
Some ARM SOC ROM need run the plug-in code first in IRAM and the plugin-in code need appear at the beginning of the u-boot. ROM will check the plugin-in header to do security check and run the plug-in code to init the DDR etc. In this case the _start will be not the same as TEXT_BASE.
I still don't see why u-boot would not end up where specified.
The fact that there is a "plug-in" (I assume it's what I would call an IPL) does not change the fact that its payload (u-boot) can and will be loaded where specified, i.e. at TEXT_BASE -- and if it is loaded elsewhere, it is at a fixed address, so TEXT_BASE can be adjusted) All IPLs that I know of put their payload where specified.
Thanks for your comments. Cheers,
Amicalement,
Hi, Albert,
2010/12/16 Albert ARIBAUD albert.aribaud@free.fr:
Le 16/12/2010 04:04, Jason Liu a écrit :
Hi, Albert,
2010/12/16 Albert ARIBAUDalbert.aribaud@free.fr:
Hi Jason,
Le 15/12/2010 14:57, Jason Liu a écrit :
There will have issue if the _start not equal TEXT_BASE when enable relocation.
In what case does this happen?
Some ARM SOC ROM need run the plug-in code first in IRAM and the plugin-in code need appear at the beginning of the u-boot. ROM will check the plugin-in header to do security check and run the plug-in code to init the DDR etc. In this case the _start will be not the same as TEXT_BASE.
I still don't see why u-boot would not end up where specified.
The fact that there is a "plug-in" (I assume it's what I would call an IPL) does not change the fact that its payload (u-boot) can and will be loaded where specified, i.e. at TEXT_BASE -- and if it is loaded elsewhere, it is at a fixed address, so TEXT_BASE can be adjusted) All IPLs that I know of put their payload where specified.
It's not an IPL. The layout is that as the following,
---- ----- TEXT_BASE plug-in ---------- _start
---------- _end
No matter what you adjusted the TEXT_BASE, the _star is not equal to it.
The fix doe not affect the original functionality but just make it more flexible.
Thanks for your comments. Cheers,
Amicalement,
Albert.
Le 16/12/2010 10:18, Jason Liu a écrit :
Hi, Albert,
2010/12/16 Albert ARIBAUDalbert.aribaud@free.fr:
Le 16/12/2010 04:04, Jason Liu a écrit :
Hi, Albert,
2010/12/16 Albert ARIBAUDalbert.aribaud@free.fr:
Hi Jason,
Le 15/12/2010 14:57, Jason Liu a écrit :
There will have issue if the _start not equal TEXT_BASE when enable relocation.
In what case does this happen?
Some ARM SOC ROM need run the plug-in code first in IRAM and the plugin-in code need appear at the beginning of the u-boot. ROM will check the plugin-in header to do security check and run the plug-in code to init the DDR etc. In this case the _start will be not the same as TEXT_BASE.
I still don't see why u-boot would not end up where specified.
The fact that there is a "plug-in" (I assume it's what I would call an IPL) does not change the fact that its payload (u-boot) can and will be loaded where specified, i.e. at TEXT_BASE -- and if it is loaded elsewhere, it is at a fixed address, so TEXT_BASE can be adjusted) All IPLs that I know of put their payload where specified.
It's not an IPL. The layout is that as the following,
---- ----- TEXT_BASE plug-in ---------- _start
---------- _end
No matter what you adjusted the TEXT_BASE, the _star is not equal to it.
The fix doe not affect the original functionality but just make it more flexible.
This layout is not that of u-boot for ARM; the fix thus corrects a fault not inherent to u-boot but introduced by inserting this "plug-in" where it should not be.
Why must you modify the original layout?
Also, what is this 'plug-in' if it is not an IPL?
Amicalement,
Dear Albert ARIBAUD,
In message 4D09E2B9.5030405@free.fr you wrote:
Why must you modify the original layout?
right!
Also, what is this 'plug-in' if it is not an IPL?
Yes, and why must it be linked at TEXT_BASE?
Let's stop this here.
Let's wait until any code is submitted or proposed that actually needs such a feature. Then we can discuss the design and try to find a solution that makes sense.
Best regards,
Wolfgang Denk
Hi, Wolfgang,
2010/12/16 Wolfgang Denk wd@denx.de:
Dear Albert ARIBAUD,
In message 4D09E2B9.5030405@free.fr you wrote:
Why must you modify the original layout?
right!
Also, what is this 'plug-in' if it is not an IPL?
Yes, and why must it be linked at TEXT_BASE?
Let's stop this here.
Let's wait until any code is submitted or proposed that actually needs such a feature. Then we can discuss the design and try to find a solution that makes sense.
In fact, this is indeed one bug in the code as I said in the comit log:
"the _dynsym_start_ofs and _rel_dyn_start_ofs is the offset from _start, so, need use _start address instead of _TEXT_BASE to caculate the rel dyn start and sym table address. This patch also correct the board_init_r function address caculation in relocation area. The addr of board_init_r after relocation is: _board_init_r_offs + relocation start address."
Is there any thing wrong with my patch? The case I met just make the bug show up.
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 History is only a confused heap of facts. -- Philip Earl of Chesterfield
Dear Jason Liu,
In message AANLkTik=BSaBbV-NOJYg2QXx0Q-YYK4=Ox3iF8dWR5re@mail.gmail.com you wrote:
In fact, this is indeed one bug in the code as I said in the comit log:
"the _dynsym_start_ofs and _rel_dyn_start_ofs is the offset from _start, so, need use _start address instead of _TEXT_BASE to caculate the rel dyn start and sym table address. This patch also correct the board_init_r function address caculation in relocation area. The addr of board_init_r after relocation is: _board_init_r_offs + relocation start address."
If there are really two different bugs, then please split your patch into to separate commits, and explain the respective problem in the commit messages.
Best regards,
Wolfgang Denk
Dear Jason Liu,
In message AANLkTimMct8TKe7k-0LA4Gob1mJZnCw0f9xbbp9VKMoz@mail.gmail.com you wrote:
In what case does this happen?
Some ARM SOC ROM need run the plug-in code first in IRAM and the plugin-in code need appear at the beginning of the u-boot. ROM will check the plugin-in header to do security check and run the plug-in code to init the DDR etc. In this case the _start will be not the same as TEXT_BASE.
So we are talking about aSoC that is not supported yet in U-Boot. Let's talk about this when such support gets added to U-Boot.
Best regards,
Wolfgang Denk
participants (4)
-
Albert ARIBAUD -
Jason Liu -
Jason Liu -
Wolfgang Denk