[U-Boot] [PATCH 0/6] RFC: True PIC on 83xx
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code: before: text data bss dec hex filename 222728 14336 24228 261292 3fcac u-boot after: text data bss dec hex filename 218556 6580 24228 249364 3ce14 u-boot
As can seen, I only have to add two LINK_OFF calls in early C code to pull this off.
Oh, this is based on the last u-boot release so it migth not apply without minor mods. It depends on 2 other patches which I have already sent.
Anyhow, if this is an acceptable approach I will clean it up once the ongoing release is out.
Joakim Tjernlund (6): mpc83xx: Add relocation support for -fpic mpc83xx: Make start.S true PIC mpc83xx: Add link vs. load address calculation mpc83xx: Add support form -msingle-pic-base mpc83xx: Add true PIC support. powerpc: Add LINK_OFF calls in early C-code.
arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- arch/powerpc/cpu/mpc83xx/start.S | 107 +++++++++++++++++++++-- arch/powerpc/cpu/mpc83xx/u-boot.lds | 5 +- arch/powerpc/lib/board.c | 2 +- include/common.h | 5 + nand_spl/board/freescale/mpc8313erdb/u-boot.lds | 4 +- nand_spl/board/freescale/mpc8315erdb/u-boot.lds | 4 +- 7 files changed, 114 insertions(+), 15 deletions(-)
By rearranging the linker script we get support for relocation of -fpic for free. --- arch/powerpc/cpu/mpc83xx/u-boot.lds | 5 +++-- nand_spl/board/freescale/mpc8313erdb/u-boot.lds | 4 +++- nand_spl/board/freescale/mpc8315erdb/u-boot.lds | 4 +++- 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds index 0b74a13..877f298 100644 --- a/arch/powerpc/cpu/mpc83xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds @@ -67,13 +67,14 @@ SECTIONS PROVIDE (erotext = .); .reloc : { - *(.got) _GOT2_TABLE_ = .; *(.got2) + *(.got) + PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); _FIXUP_TABLE_ = .; *(.fixup) } - __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; + __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
.data : diff --git a/nand_spl/board/freescale/mpc8313erdb/u-boot.lds b/nand_spl/board/freescale/mpc8313erdb/u-boot.lds index 853b2de..7c5a40f 100644 --- a/nand_spl/board/freescale/mpc8313erdb/u-boot.lds +++ b/nand_spl/board/freescale/mpc8313erdb/u-boot.lds @@ -40,8 +40,10 @@ SECTIONS *(.sdata*) _GOT2_TABLE_ = .; *(.got2) + *(.got) + PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); } - __got2_entries = (. - _GOT2_TABLE_) >> 2; + __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
. = ALIGN(8); __bss_start = .; diff --git a/nand_spl/board/freescale/mpc8315erdb/u-boot.lds b/nand_spl/board/freescale/mpc8315erdb/u-boot.lds index 853b2de..7c5a40f 100644 --- a/nand_spl/board/freescale/mpc8315erdb/u-boot.lds +++ b/nand_spl/board/freescale/mpc8315erdb/u-boot.lds @@ -40,8 +40,10 @@ SECTIONS *(.sdata*) _GOT2_TABLE_ = .; *(.got2) + *(.got) + PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); } - __got2_entries = (. - _GOT2_TABLE_) >> 2; + __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
. = ALIGN(8); __bss_start = .;
Remove dependencies on link address. Use GOT and add an new function to calculate the actual address. --- arch/powerpc/cpu/mpc83xx/start.S | 36 ++++++++++++++++++++++++++++-------- 1 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 09c5eac..3cac147 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -69,12 +69,12 @@ */ START_GOT GOT_ENTRY(_GOT2_TABLE_) + GOT_ENTRY(_start) GOT_ENTRY(__bss_start) GOT_ENTRY(_end)
#ifndef CONFIG_NAND_SPL GOT_ENTRY(_FIXUP_TABLE_) - GOT_ENTRY(_start) GOT_ENTRY(_start_of_vectors) GOT_ENTRY(_end_of_vectors) GOT_ENTRY(transfer_to_handler) @@ -240,10 +240,12 @@ boot_warm: /* time t 5 */ /* there and deflate the flash size back to minimal size */ /*------------------------------------------------------------*/ bl map_flash_by_law1 - lis r4, (CONFIG_SYS_MONITOR_BASE)@h - ori r4, r4, (CONFIG_SYS_MONITOR_BASE)@l - addi r5, r4, in_flash - _start + EXC_OFF_SYS_RESET - mtlr r5 + + bl 1f +1: mflr r3 /* get current address */ + addi r3, r3, in_flash - 1b + bl add_flash_base + mtlr r3 blr in_flash: #if 1 /* Remapping flash with LAW0. */ @@ -833,10 +835,12 @@ relocate_code: mr r10, r5 /* Save copy of Destination Address */
GET_GOT - mr r3, r5 /* Destination Address */ - lis r4, CONFIG_SYS_MONITOR_BASE@h /* Source Address */ - ori r4, r4, CONFIG_SYS_MONITOR_BASE@l + + lwz r4, GOT(_start) /* Source Address */ + addi r4, r4, -EXC_OFF_SYS_RESET lwz r5, GOT(__bss_start) + mr r3, r10 /* Destination Address */ + sub r5, r5, r4 li r6, CONFIG_SYS_CACHELINE_SIZE /* Cache Line Size */
@@ -1128,6 +1132,21 @@ unlock_ram_in_cache: #endif /* CONFIG_SYS_INIT_RAM_LOCK */
#ifdef CONFIG_SYS_FLASHBOOT + +add_flash_base: + /* Check if already inside flash address space. */ + /* if so, do not add CONFIG_SYS_FLASH_BASE */ + lis r4, (CONFIG_SYS_FLASH_BASE)@h + ori r4, r4, (CONFIG_SYS_FLASH_BASE)@l + cmplw cr0, r3, r4 + ble cr0, 2f /* r3 < r4 ? */ + lis r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@h + ori r6, r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@l + cmplw cr0, r3, r6 + blelr cr0 /* r3 < r6 ? */ +2: add r3,r3,r4 + blr + map_flash_by_law1: /* When booting from ROM (Flash or EPROM), clear the */ /* Address Mask in OR0 so ROM appears everywhere */ @@ -1179,6 +1198,7 @@ map_flash_by_law1: */ remap_flash_by_law0: /* Initialize the BR0 with the boot ROM starting address. */ + lis r3, (CONFIG_SYS_IMMR)@h /* r3 <= CONFIG_SYS_IMMR */ lwz r4, BR0(r3) li r5, 0x7FFF and r4, r4, r5
link_off calculates the difference between link address and actila load address. This is a must for true PIC u-boot. --- arch/powerpc/cpu/mpc83xx/start.S | 25 +++++++++++++++++++++++++ include/common.h | 5 +++++ 2 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 3cac147..ec65f40 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -427,6 +427,31 @@ _end_of_vectors:
. = 0x3000
+ .globl link_off /* const void * link_off(const void * ptr) */ + .type link_off, @function + /* + * Calculates the offset between link address and load address + * and subtracs the offset to from its argument(r3) + * This function must be where _GOT2_TABLE_ is defined + */ +link_off: + /* GOT hand coded as we cannot clobber r12 */ + mflr r4 + bl 1f + .text 2 +0: .long .LCTOC1-1f + .text +1: mflr r6 + lwz r0,0b-1b(r6) + add r6,r0,r6 + mtlr r4 + la r4,.L__GOT2_TABLE_(r6) /* addi r4,r6,.L__GOT2_TABLE_ */ + lwz r5,.L__GOT2_TABLE_(r6) + sub r4,r5,r4 /* r4 - r5 */ + sub r3,r3,r4 /* r4 - r3 */ + blr + .size link_off, .-link_off + /* * This code finishes saving the registers to the exception frame * and jumps to the appropriate handler for the exception. diff --git a/include/common.h b/include/common.h index 8bca04f..f257ea4 100644 --- a/include/common.h +++ b/include/common.h @@ -94,6 +94,9 @@ typedef volatile unsigned char vu_char; #ifdef CONFIG_MPC83xx #include <mpc83xx.h> #include <asm/immap_83xx.h> +const void * link_off(const void *); +#else +#define link_off(x) ((const void *)(x)) #endif #ifdef CONFIG_4xx #include <ppc4xx.h> @@ -111,6 +114,8 @@ typedef volatile unsigned char vu_char; #include <asm/arch/hardware.h> #endif
+#define LINK_OFF(x) ((__typeof__(x))link_off(x)) + #include <part.h> #include <flash.h> #include <image.h>
On Tue, 23 Nov 2010 19:48:48 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
diff --git a/include/common.h b/include/common.h index 8bca04f..f257ea4 100644 --- a/include/common.h +++ b/include/common.h @@ -94,6 +94,9 @@ typedef volatile unsigned char vu_char; #ifdef CONFIG_MPC83xx #include <mpc83xx.h> #include <asm/immap_83xx.h> +const void * link_off(const void *); +#else +#define link_off(x) ((const void *)(x)) #endif
What is special about 83xx?
If it's just that 83xx is the only one that this type of relocation has been enabled on so far, define a symbol for that.
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/23 20:32:32:
On Tue, 23 Nov 2010 19:48:48 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
diff --git a/include/common.h b/include/common.h index 8bca04f..f257ea4 100644 --- a/include/common.h +++ b/include/common.h @@ -94,6 +94,9 @@ typedef volatile unsigned char vu_char; #ifdef CONFIG_MPC83xx #include <mpc83xx.h> #include <asm/immap_83xx.h> +const void * link_off(const void *); +#else +#define link_off(x) ((const void *)(x)) #endif
What is special about 83xx?
Nothing, just it is the first one.
If it's just that 83xx is the only one that this type of relocation has been enabled on so far, define a symbol for that.
there is a #define LINK_OFF(x) ((__typeof__(x))link_off(x)) already but I am guessing you mean something else. Perhaps a dummy link_off for other targets? Don't think that is any better.
On Tue, 23 Nov 2010 21:08:37 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 20:32:32:
On Tue, 23 Nov 2010 19:48:48 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
diff --git a/include/common.h b/include/common.h index 8bca04f..f257ea4 100644 --- a/include/common.h +++ b/include/common.h @@ -94,6 +94,9 @@ typedef volatile unsigned char vu_char; #ifdef CONFIG_MPC83xx #include <mpc83xx.h> #include <asm/immap_83xx.h> +const void * link_off(const void *); +#else +#define link_off(x) ((const void *)(x)) #endif
What is special about 83xx?
Nothing, just it is the first one.
If it's just that 83xx is the only one that this type of relocation has been enabled on so far, define a symbol for that.
there is a #define LINK_OFF(x) ((__typeof__(x))link_off(x)) already but I am guessing you mean something else. Perhaps a dummy link_off for other targets? Don't think that is any better.
I mean instead of this:
#ifdef CONFIG_MPC83xx ...unrelated stuff... const void *link_off(const void *ptr); #else #define link_off(x) ((const void *)(x)); #endif
do something like this:
#ifdef CONFIG_RELOC_PIC const void *link_off(const void *ptr); #else #define link_off(x) ((const void *)(x)); #endif
...with CONFIG_RELOC_PIC defined in a board config file and also controlling whether this mechanism is enabled at all. Boards could enable it as they verify that they have the proper manual relocations (or just leave it off, if they don't think it's worth it) -- including non-83xx targets if they provide link_off() and do whatever else is required.
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/23 21:17:12:
On Tue, 23 Nov 2010 21:08:37 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 20:32:32:
On Tue, 23 Nov 2010 19:48:48 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
diff --git a/include/common.h b/include/common.h index 8bca04f..f257ea4 100644 --- a/include/common.h +++ b/include/common.h @@ -94,6 +94,9 @@ typedef volatile unsigned char vu_char; #ifdef CONFIG_MPC83xx #include <mpc83xx.h> #include <asm/immap_83xx.h> +const void * link_off(const void *); +#else +#define link_off(x) ((const void *)(x)) #endif
What is special about 83xx?
Nothing, just it is the first one.
If it's just that 83xx is the only one that this type of relocation has been enabled on so far, define a symbol for that.
there is a #define LINK_OFF(x) ((__typeof__(x))link_off(x)) already but I am guessing you mean something else. Perhaps a dummy link_off for other targets? Don't think that is any better.
I mean instead of this:
#ifdef CONFIG_MPC83xx ...unrelated stuff... const void *link_off(const void *ptr); #else #define link_off(x) ((const void *)(x)); #endif
do something like this:
#ifdef CONFIG_RELOC_PIC const void *link_off(const void *ptr); #else #define link_off(x) ((const void *)(x)); #endif
...with CONFIG_RELOC_PIC defined in a board config file and also controlling whether this mechanism is enabled at all. Boards could enable it as they verify that they have the proper manual relocations (or just leave it off, if they don't think it's worth it) -- including non-83xx targets if they provide link_off() and do whatever else is required.
Ah yes, will be in my cleanup later on.
singel-pic-base is pending inclusinon in gcc and is useful for reducing code size and impl. true PIC. --- arch/powerpc/cpu/mpc83xx/start.S | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index ec65f40..3d4e288 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -298,7 +298,11 @@ in_flash: /*------------------------------------------------------*/
GET_GOT /* initialize GOT access */ - +#if defined(__pic__) && __pic__ == 1 + /* Needed for upcoming -msingle-pic-base */ + bl _GLOBAL_OFFSET_TABLE_@local-4 + mflr r30 +#endif /* r3: IMMR */ lis r3, CONFIG_SYS_IMMR@h /* run low-level CPU init code (in Flash)*/ @@ -860,7 +864,11 @@ relocate_code: mr r10, r5 /* Save copy of Destination Address */
GET_GOT - +#if defined(__pic__) && __pic__ == 1 + /* Needed for upcoming -msingle-pic-base */ + bl _GLOBAL_OFFSET_TABLE_@local-4 + mflr r30 +#endif lwz r4, GOT(_start) /* Source Address */ addi r4, r4, -EXC_OFF_SYS_RESET lwz r5, GOT(__bss_start)
By copying the GOT to the end of the INIT_RAM(dcache) and relocating it there, it is much esier to support true PIC on u-boot. This cannot handle FIXUP so C code that depends on fixups before relocation to RAM must use LINK_OFF to calculate the difference.
This depends on the upcoming single-pic-base option to gcc. --- arch/powerpc/cpu/mpc83xx/start.S | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 3d4e288..b24a89a 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -302,6 +302,34 @@ in_flash: /* Needed for upcoming -msingle-pic-base */ bl _GLOBAL_OFFSET_TABLE_@local-4 mflr r30 +#ifdef CONFIG_SYS_TRUE_PIC + /* + * Copy GOT to cache and relocate it + * This assumes there is enough space at the end + * INIT_RAM to hold a copy of the GOT. + */ + li r3,0 + bl link_off /* r3 holds link offset at return */ + lis r9, (CONFIG_SYS_INIT_RAM_ADDR+CONFIG_SYS_INIT_RAM_END)@h + ori r9,r9,(CONFIG_SYS_INIT_RAM_ADDR+CONFIG_SYS_INIT_RAM_END)@l + lwz r11,GOT(_GOT2_TABLE_) + add r11,r11,r3 + + li r4, __got2_entries@sectoff@l + mtctr r4 + slwi r5,r4,2 /* r4 * 4 */ + subf r9,r5,r9 /* r9 - r5 */ + + subf r10,r9,r11 /* r11 - r9 */ + subf r30,r10,r30 /* r30 - r10, point PIC(r30) to new GOT */ + addi r11,r11,-4 + addi r9,r9,-4 +1: /* copy GOT and add link offset */ + lwzu r0,4(r11) + add r0,r0,r3 + stwu r0,4(r9) + bdnz 1b +#endif #endif /* r3: IMMR */ lis r3, CONFIG_SYS_IMMR@h @@ -869,9 +897,17 @@ relocate_code: bl _GLOBAL_OFFSET_TABLE_@local-4 mflr r30 #endif +#ifdef CONFIG_SYS_TRUE_PIC + li r3, 0 + bl link_off /* const void * link_off(const void *) */ +#endif lwz r4, GOT(_start) /* Source Address */ addi r4, r4, -EXC_OFF_SYS_RESET lwz r5, GOT(__bss_start) +#ifdef CONFIG_SYS_TRUE_PIC + add r4, r4, r3 /* Add link offset */ + add r5, r5, r3 /* Add link offset */ +#endif mr r3, r10 /* Destination Address */
sub r5, r5, r4
Only these 2 call sites depends on fixups for my mpc8321 based board. --- arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- arch/powerpc/lib/board.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c index f3b67ae..0437b49 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) sep = " "; for (i = 0; i < n; i++) if (rsr & bits[i].mask) { - printf("%s%s", sep, bits[i].desc); + printf("%s%s", sep, LINK_OFF(bits[i].desc)); sep = ", "; } puts("\n"); diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index 7b09fb5..9fa99dc 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) #endif
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { - if ((*init_fnc_ptr) () != 0) { + if ((LINK_OFF(*init_fnc_ptr)) () != 0) { hang (); } }
On Tue, 23 Nov 2010 19:48:51 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
Only these 2 call sites depends on fixups for my mpc8321 based board.
arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- arch/powerpc/lib/board.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c index f3b67ae..0437b49 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) sep = " "; for (i = 0; i < n; i++) if (rsr & bits[i].mask) {
printf("%s%s", sep, bits[i].desc);
printf("%s%s", sep, LINK_OFF(bits[i].desc)); sep = ", ";diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index 7b09fb5..9fa99dc 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) #endif
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
if ((*init_fnc_ptr) () != 0) {
if ((LINK_OFF(*init_fnc_ptr)) () != 0) { hang ();
"Only these" that you've found so far, for the board you've tried -- is it worth adding another pre-relocation landmine to shrink the image by 5%? I don't miss the manual fixups we had to do under the old relocation scheme.
Please document the specific circumstances in which one would need to use this (any data-segment pointer before relocation?).
Is a missing LINK_OFF likely to result in a crash, or silent bad behavior? Will LINK_OFF be a no-op after relocation?
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/23 21:46:51:
On Tue, 23 Nov 2010 19:48:51 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
Only these 2 call sites depends on fixups for my mpc8321 based board.
arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- arch/powerpc/lib/board.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c index f3b67ae..0437b49 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) sep = " "; for (i = 0; i < n; i++) if (rsr & bits[i].mask) {
printf("%s%s", sep, bits[i].desc);
printf("%s%s", sep, LINK_OFF(bits[i].desc)); sep = ", "; }diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index 7b09fb5..9fa99dc 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) #endif
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
if ((*init_fnc_ptr) () != 0) {
if ((LINK_OFF(*init_fnc_ptr)) () != 0) { hang (); }"Only these" that you've found so far, for the board you've tried --
Yes, that is what I wrote. The previous try had LINK_OFF calls all over the place just for my board.
is it worth adding another pre-relocation landmine to shrink the image by 5%? I don't miss the manual fixups we had to do under the old relocation scheme.
-msingle-pic-base does not require LINK_OFF per se. It will shrink the image significantly for free. It enables impl. of true PIC and if you want to have that too you need LINK_OFF.
Please document the specific circumstances in which one would need to use this (any data-segment pointer before relocation?).
Any ptr needing fixups: char *sptr = "hi"; Same for static initialized function ptrs too.
Is a missing LINK_OFF likely to result in a crash, or silent bad behavior? Will LINK_OFF be a no-op after relocation?
Either crash or garbage printed on the port. LINK_OFF is a NOP after relocation. It will be a NOP if link address == load address too.
Jocke
Scott Wood scottwood@freescale.com wrote on 2010/11/23 21:46:51:
On Tue, 23 Nov 2010 19:48:51 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote: Please document the specific circumstances in which one would need to use this (any data-segment pointer before relocation?).
Any ptr needing fixups: char *sptr = "hi"; Same for static initialized function ptrs too.
Clarification, the above is for ptrs used before relocation to RAM.
Jocke
On Tue, 23 Nov 2010 22:03:36 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 21:46:51:
"Only these" that you've found so far, for the board you've tried --
Yes, that is what I wrote.
Yes, I was just emphasizing that. :-)
is it worth adding another pre-relocation landmine to shrink the image by 5%? I don't miss the manual fixups we had to do under the old relocation scheme.
-msingle-pic-base does not require LINK_OFF per se. It will shrink the image significantly for free. It enables impl. of true PIC and if you want to have that too you need LINK_OFF.
What is "true PIC" versus the rest of what -msingle-pic-base does, and what are the advantages?
Is a missing LINK_OFF likely to result in a crash, or silent bad behavior? Will LINK_OFF be a no-op after relocation?
Either crash or garbage printed on the port.
Garbage possibly being empty strings that aren't noticed?
Or possibly bad values changing program flow in nonobvious ways?
LINK_OFF is a NOP after relocation. It will be a NOP if link address == load address too.
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Seems like there would be a high risk for a developer of board A to add new code that affects board B, and needs a manual relocation, but board A doesn't use this or loads at the link address, so it doesn't show up. Seems like a maintenance headache.
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
On Tue, 23 Nov 2010 22:03:36 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 21:46:51:
"Only these" that you've found so far, for the board you've tried --
Yes, that is what I wrote.
Yes, I was just emphasizing that. :-)
is it worth adding another pre-relocation landmine to shrink the image by 5%? I don't miss the manual fixups we had to do under the old relocation scheme.
-msingle-pic-base does not require LINK_OFF per se. It will shrink the image significantly for free. It enables impl. of true PIC and if you want to have that too you need LINK_OFF.
What is "true PIC" versus the rest of what -msingle-pic-base does, and what are the advantages?
Being able to load the same image to any address. msingle-pic-base simple skips the PIC prologue and assumes the R30 is already setup correctly. I a self contained env. like uboot this is true for -fpic. I use that property to copy the GOT to dcache, relocate it there and point R30 to the GOT in dcache.
Is a missing LINK_OFF likely to result in a crash, or silent bad behavior? Will LINK_OFF be a no-op after relocation?
Either crash or garbage printed on the port.
Garbage possibly being empty strings that aren't noticed?
Or possibly bad values changing program flow in nonobvious ways?
Impossible to say, I guess anything can happen.
LINK_OFF is a NOP after relocation. It will be a NOP if link address == load address too.
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
I think Wolfgang had some other uses in mind too.
Seems like there would be a high risk for a developer of board A to add new code that affects board B, and needs a manual relocation, but board A doesn't use this or loads at the link address, so it doesn't show up. Seems like a maintenance headache.
yes, it is a risk. One that can be minimized the more uses this function.
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
In fact, we could probably do something similar when copying to RAM as well, and not need any sort of link relocation.
-Scott
Dear Scott Wood,
In message 20101123163204.4f843e61@udp111988uds.am.freescale.net you wrote:
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
Not all systems have BATs.
Best regards,
Wolfgang Denk
On Wed, 24 Nov 2010 00:03:13 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Scott Wood,
In message 20101123163204.4f843e61@udp111988uds.am.freescale.net you wrote:
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
Not all systems have BATs.
I was speaking in the context of what he wanted to do with an 83xx board -- but the concept applies to any hardware with an MMU that isn't too painful to set up that early.
If someone wants to do this kind of thing on hardware that doesn't meet that description, that's another matter -- if the the hardware doesn't provide a nicer bank switching mechanism (e.g. p4080ds lets you rotate the flash banks' physical addresses, rather than change the reset vector), or an SRAM that U-Boot (or an SPL) could copy itself to before C code, etc.
-Scott
On Wed, Nov 24, 2010 at 10:24 AM, Scott Wood scottwood@freescale.com wrote:
On Wed, 24 Nov 2010 00:03:13 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Scott Wood,
In message 20101123163204.4f843e61@udp111988uds.am.freescale.net you wrote:
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
Not all systems have BATs.
I was speaking in the context of what he wanted to do with an 83xx board -- but the concept applies to any hardware with an MMU that isn't too painful to set up that early.
If someone wants to do this kind of thing on hardware that doesn't meet that description, that's another matter -- if the the hardware doesn't provide a nicer bank switching mechanism (e.g. p4080ds lets you rotate the flash banks' physical addresses, rather than change the reset vector), or an SRAM that U-Boot (or an SPL) could copy itself to before C code, etc.
It seems to me that we are applying at the architecture level a 'nice to have' which may belong at the board level. How many vendors are going to do a fancy 'two U-Boot images' trickery? Will it be (nearly) every 83xx board?
I agree with Scott - If you want to do something that fancy, provide support for it in your board hardware. I don't like the idea of diverging the core feature-set available at the architecture level if those features belong more at the SOC or Board level.
Regards,
Graeme
Scott Wood scottwood@freescale.com wrote on 2010/11/23 23:32:04:
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
Jocke
Scott Wood scottwood@freescale.com wrote on 2010/11/23 23:32:04:
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
Jocke
On Wed, 24 Nov 2010 12:04:15 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 23:32:04:
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash.
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/24 18:16:56:
On Wed, 24 Nov 2010 12:04:15 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 23:32:04:
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58:
"load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash.
Played a little with this but it seems like two BATs cannot overlap?
On Wed, 24 Nov 2010 22:36:27 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/24 18:16:56:
On Wed, 24 Nov 2010 12:04:15 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 23:32:04:
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/23 22:20:58: > "load address" being pre-relocation? Currently these must be equal > (which doesn't seem particularly burdensome).
Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images.
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash.
Played a little with this but it seems like two BATs cannot overlap?
They can't overlap in effective address space. They can have overlapping physical addresses (make sure the WIMG bits are the same).
-Scott
Scott Wood scottwood@freescale.com wrote on 2010/11/24 22:41:19:
How about playing with BATs before entering C code, so that the image always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash.
Played a little with this but it seems like two BATs cannot overlap?
They can't overlap in effective address space. They can have overlapping physical addresses (make sure the WIMG bits are the same).
hmm, now its starting to get a bit messy, I have flash over and under my boot area. In general it may be hard to match any random layout.
On Wed, 24 Nov 2010 22:50:21 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/24 22:41:19:
> How about playing with BATs before entering C code, so that the image > always appears at the same effective address?
hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either.
After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead.
You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash.
Played a little with this but it seems like two BATs cannot overlap?
They can't overlap in effective address space. They can have overlapping physical addresses (make sure the WIMG bits are the same).
hmm, now its starting to get a bit messy, I have flash over and under my boot area. In general it may be hard to match any random layout.
It shouldn't matter where in flash the boot area is.
E.g. if you have 64 MiB flash, with a 256 KiB boot image possibly residing at offset zero or 0x3f00000, and the flash physical address (after boot code is done with BR0/OR0 games) is 0xfc000000. You could have a 64 MiB mapping at, say, 0xe0000000 always poing to 0xfc000000, and a 256 KiB mapping at 0xfff00000 pointing at either 0xfc000000 or 0xfff00000 depending on which you booted from. If somehow you could also boot from offset 0x2480000, then have 0xfff00000 point to 0xfe480000. The mapping at 0xe0000000 is independent and always points to 64 MiB at 0xfc000000.
-Scott
Dear Joakim Tjernlund,
In message OF3286D177.B2B7747D-ONC12577E5.00769A1F-C12577E5.0076B18F@transmode.se you wrote:
Played a little with this but it seems like two BATs cannot overlap?
IIRC they can, but the first (lower register numbers) mapping wins.
Best regards,
Wolfgang Denk
Wolfgang Denk wd@denx.de wrote on 2010/11/24 22:45:11:
Dear Joakim Tjernlund,
In message OF3286D177.B2B7747D-ONC12577E5.00769A1F-C12577E5.0076B18F@transmode.se you wrote:
Played a little with this but it seems like two BATs cannot overlap?
IIRC they can, but the first (lower register numbers) mapping wins.
This text from PPC PEM suggests otherwise:
If a BAT entry is not valid for a given access, it does not participate in address translation for that access. Two BAT entries may not map an overlapping effective address range and be valid at the same time. Entries that have complementary settings of V[s] and V[p] may map overlapping effective address blocks. Complementary settings would be as follows: BAT entry A: Vs = 1, Vp = 0 BAT entry B: Vs = 0, Vp = 1
Best regards,
Wolfgang Denk
PS. Are you planning to apply some of my earlier patches(not this RFC series) or are you waiting for Freescale to Ack/pick up? So far nobody has said anything.
Jocke
Dear Joakim Tjernlund,
In message OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se you wrote:
Are you planning to apply some of my earlier patches(not this RFC series) or are you waiting for Freescale to Ack/pick up? So far nobody has said anything.
I think these are all on Freescale's (Kim Phillips') list.
Best regards,
Wolfgang Denk
On Wed, 24 Nov 2010 22:59:55 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Joakim Tjernlund,
In message OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se you wrote:
Are you planning to apply some of my earlier patches(not this RFC series) or are you waiting for Freescale to Ack/pick up? So far nobody has said anything.
I think these are all on Freescale's (Kim Phillips') list.
outside of the PIC ones, which I believe are still outstanding, I have:
Thu Nov 4 15:45:12 2010 +0100 "mpc83xx: Make it boot again"
-- which looks like it has comments that haven't been addressed
and:
Tue Nov 23 19:33:43 2010 +0100 "mpc83xx: remove some dead code, saving space"
-- which I've applied.
Please let me know what else is needed, if anything.
Thanks,
Kim
Kim Phillips kim.phillips@freescale.com wrote on 2010/11/28 16:12:21:
On Wed, 24 Nov 2010 22:59:55 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Joakim Tjernlund,
In message OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se you wrote:
Are you planning to apply some of my earlier patches(not this RFC series) or are you waiting for Freescale to Ack/pick up? So far nobody has said anything.
I think these are all on Freescale's (Kim Phillips') list.
outside of the PIC ones, which I believe are still outstanding, I have:
Thu Nov 4 15:45:12 2010 +0100 "mpc83xx: Make it boot again"
-- which looks like it has comments that haven't been addressed
What comments would that be? There are none.
Jocke
PS. The last version has this fix for the problem in case you are looking at an old one.
+ /* Wait for HW to catch up */ + lwz r4, LBLAWAR1(r3) + twi 0,r4,0 + isync
Dear Joakim Tjernlund,
In message 1290538131-12383-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code:
Is my inderstanding correct that this is a private, all new gcc extension? Do you have plans to submit it upstream?
I appreciate your work, and I, too, would like to be able to do what this patch series seems to acchieve, but I think we should not add this to mainline when neither mainline gcc nor any of the "common" tool chains (say, OE/Yocto/LF, Linaro, CodeSourcery, ...) supports it.
Best regards,
Wolfgang Denk
Wolfgang Denk wd@denx.de wrote on 2010/11/23 21:52:11:
Dear Joakim Tjernlund,
In message 1290538131-12383-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code:
Is my inderstanding correct that this is a private, all new gcc extension? Do you have plans to submit it upstream?
I appreciate your work, and I, too, would like to be able to do what this patch series seems to acchieve, but I think we should not add this to mainline when neither mainline gcc nor any of the "common" tool chains (say, OE/Yocto/LF, Linaro, CodeSourcery, ...) supports it.
-msingle-pic-base already exist for ARM. I impl. it for PowerPC too. I have submitted it upstream and I hope it will be accepted, it is ongoing but slowly as there are very few ppc maintainers.
Jocke
On Wed, Nov 24, 2010 at 8:08 AM, Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Wolfgang Denk wd@denx.de wrote on 2010/11/23 21:52:11:
Dear Joakim Tjernlund,
In message 1290538131-12383-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code:
Is my inderstanding correct that this is a private, all new gcc extension? Do you have plans to submit it upstream?
I appreciate your work, and I, too, would like to be able to do what this patch series seems to acchieve, but I think we should not add this to mainline when neither mainline gcc nor any of the "common" tool chains (say, OE/Yocto/LF, Linaro, CodeSourcery, ...) supports it.
-msingle-pic-base already exist for ARM. I impl. it for PowerPC too. I have submitted it upstream and I hope it will be accepted, it is ongoing but slowly as there are very few ppc maintainers.
I'm still trying to get my head around exactly what the end benifit of having this functionality is.
I implemented fully position independent image support for warm-boot (i.e. bootstraped from pre-initialised memory for build-load-test) for x86. But x86 is out of line with other arches in that it initialises memory and relocates into RAM _before_ exectuting any of the init sequence. It is the relocation prior to executing any other core U-Boot code which allows full PIC on x86 to work.
Now I'm in the process of aligning x86 with the other arches (create a temporary stack, run init sequence, init RAM, relocate) and I am going to lose my full PIC functionality. Now while not ideal, it is not a game breaker for me (especially now that I can easily specify a secondary board with a different TEXT_BASE trivially in boards.cfg)
Is there any real benifit to be gained from diverging the feature sets and implementation details at the architecture level when we have been spending so much time and effort aligning them (ELF Relocation etc)
Regards,
Graeme
Wolfgang Denk wd@denx.de wrote on 2010/11/23 21:52:11:
Dear Joakim Tjernlund,
In message 1290538131-12383-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code:
Is my inderstanding correct that this is a private, all new gcc extension? Do you have plans to submit it upstream?
Both patches has now been accepted and applied to gcc trunk.
Jocke
Wolfgang Denk wd@denx.de wrote on 2010/11/23 21:52:11:
Dear Joakim Tjernlund,
In message 1290538131-12383-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Here goes my attempt to revive true PIC on ppc, starting with 83xx. This uses a a new gcc option(which I have just impl.) to add msingle-pic-base also save a lot of code:
Is my inderstanding correct that this is a private, all new gcc extension? Do you have plans to submit it upstream?
Both patches has now been accepted and applied to gcc trunk.
I have uploaded patches for existing gcc's here: http://bugs.gentoo.org/show_bug.cgi?id=347281
participants (6)
-
Graeme Russ -
Joakim Tjernlund -
Joakim Tjernlund
-
Kim Phillips -
Scott Wood -
Wolfgang Denk