* [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE @ 2010-09-22 13:57 Albert Aribaud 2010-09-22 13:57 ` [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base Albert Aribaud 2010-09-22 18:05 ` [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Ben Gardiner 0 siblings, 2 replies; 31+ messages in thread From: Albert Aribaud @ 2010-09-22 13:57 UTC (permalink / raw) To: u-boot Replace GOT indirect addressing with more efficient pic-base relative addressing for initialized data (uninitialized data still use GOTi indirect addressing). This also reduces code size by 0.4% compared to -fPIC. Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> --- arch/arm/config.mk | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 6923f6d..138c43a 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -35,7 +35,7 @@ endif ifndef CONFIG_SYS_ARM_WITHOUT_RELOC # needed for relocation -PLATFORM_RELFLAGS += -fPIC +PLATFORM_RELFLAGS += -fPIE endif ifdef CONFIG_SYS_ARM_WITHOUT_RELOC -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 13:57 [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Albert Aribaud @ 2010-09-22 13:57 ` Albert Aribaud 2010-09-22 18:05 ` Ben Gardiner 2010-09-23 7:12 ` Heiko Schocher 2010-09-22 18:05 ` [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Ben Gardiner 1 sibling, 2 replies; 31+ messages in thread From: Albert Aribaud @ 2010-09-22 13:57 UTC (permalink / raw) To: u-boot Add -msingle-pic-base to the relocation flags, and compute the pic base in start.S twice and for all -- once before relocation to run board_init_f, and once after relocation to run board_init_r and the rest of u-boot. This further reduces code size by 2.5% compared to -fPIE alone. Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> --- arch/arm/cpu/arm926ejs/config.mk | 5 ++++ arch/arm/cpu/arm926ejs/start.S | 40 ++++++++++++++++++++++++++++++++++-- arch/arm/cpu/arm926ejs/u-boot.lds | 1 + 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk index f8ef90f..aa84706 100644 --- a/arch/arm/cpu/arm926ejs/config.mk +++ b/arch/arm/cpu/arm926ejs/config.mk @@ -23,6 +23,11 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +ifndef CONFIG_SYS_ARM_WITHOUT_RELOC +# needed for optimal relocation +PLATFORM_RELFLAGS += -msingle-pic-base +endif + PLATFORM_CPPFLAGS += -march=armv5te # ========================================================================= # diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 16ee972..739fa06 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -198,9 +198,23 @@ reset: bl cpu_init_crit #endif -/* Set stackpointer in internal RAM to call board_init_f */ -call_board_init_f: + /* + * Set stack pointer in internal RAM + */ ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) + + /* + * Set pic base register to the current GOT position. At the + * moment this is also our run-time position. + * Set both r10 and r9 because either could be used as pic base + * depending on whether stack checking is off or on. + */ + ldr r10, _got_base + mov r9, r10 + + /* + * Call board_init_f, passing it 0 for bootflag + */ ldr r0,=0x00000000 bl board_init_f @@ -220,7 +234,9 @@ relocate_code: mov r6, r2 /* save addr of destination */ mov r7, r2 /* save addr of destination */ - /* Set up the stack */ + /* + * Set up the stack + */ stack_setup: mov sp, r4 @@ -282,6 +298,18 @@ clbss_l:str r2, [r0] /* clear loop... */ bl red_LED_on #endif + /* + * Set pic base register to the current GOT position. Since we are + * now running from RAM, we need to offset it from its link-time to + * its run-time position. + */ + ldr r9, relocate_got_base_r + sub r9, pc, r9 + ldr r10, _got_base +relocate_got_base_r: + add r10, r9, r10 + mov r9, r10 + /* * We are done. Do not return, instead branch to second part of board * initialization, now running from RAM. @@ -303,6 +331,12 @@ _nand_boot: .word nand_boot mov pc, lr _board_init_r: .word board_init_r + +_got_base: + .word __got_base +_relocate_got_base_r: + .word relocate_got_base_r + #endif #else /* #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) */ diff --git a/arch/arm/cpu/arm926ejs/u-boot.lds b/arch/arm/cpu/arm926ejs/u-boot.lds index 02eb8ca..b6e21f2 100644 --- a/arch/arm/cpu/arm926ejs/u-boot.lds +++ b/arch/arm/cpu/arm926ejs/u-boot.lds @@ -53,6 +53,7 @@ SECTIONS __got_start = .; . = ALIGN(4); + __got_base = .; .got : { *(.got) } __got_end = .; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 13:57 ` [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base Albert Aribaud @ 2010-09-22 18:05 ` Ben Gardiner 2010-09-22 19:07 ` Albert ARIBAUD 2010-09-22 20:30 ` Wolfgang Denk 2010-09-23 7:12 ` Heiko Schocher 1 sibling, 2 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-22 18:05 UTC (permalink / raw) To: u-boot Bonjour Albert, On Wed, Sep 22, 2010 at 9:57 AM, Albert Aribaud <albert.aribaud@free.fr> wrote: > Add -msingle-pic-base to the relocation flags, and compute the pic base > in start.S twice and for all -- once before relocation to run board_init_f, > and once after relocation to run board_init_r and the rest of u-boot. > This further reduces code size by 2.5% compared to -fPIE alone. > > Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> I tried tested this on da850evm -- the da850evm.h has "#undef CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation support */". I never got a boot-prompt. I tried mucking with the patch a little and found that if I did: diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk index aa84706..f8ef90f 100644 --- a/arch/arm/cpu/arm926ejs/config.mk +++ b/arch/arm/cpu/arm926ejs/config.mk @@ -23,11 +23,6 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float -ifndef CONFIG_SYS_ARM_WITHOUT_RELOC -# needed for optimal relocation -PLATFORM_RELFLAGS += -msingle-pic-base -endif - PLATFORM_CPPFLAGS += -march=armv5te # ========================================================================= # Then I do get a prompt. The gcc version here is: $arm-none-linux-gnueabi-gcc --version arm-none-linux-gnueabi-gcc (Sourcery G++ Lite 2009q1-203) 4.3.3 Copyright (C) 2008 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I have a JTAG debugger for this board. Please let me know if there are register dumps or the like that I can get you. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 18:05 ` Ben Gardiner @ 2010-09-22 19:07 ` Albert ARIBAUD 2010-09-22 20:51 ` Ben Gardiner 2010-09-23 16:37 ` Ben Gardiner 2010-09-22 20:30 ` Wolfgang Denk 1 sibling, 2 replies; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-22 19:07 UTC (permalink / raw) To: u-boot Le 22/09/2010 20:05, Ben Gardiner a ?crit : > Bonjour Albert, :) Hi Ben, > On Wed, Sep 22, 2010 at 9:57 AM, Albert Aribaud<albert.aribaud@free.fr> wrote: >> Add -msingle-pic-base to the relocation flags, and compute the pic base >> in start.S twice and for all -- once before relocation to run board_init_f, >> and once after relocation to run board_init_r and the rest of u-boot. >> This further reduces code size by 2.5% compared to -fPIE alone. >> >> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr> > > I tried tested this on da850evm -- the da850evm.h has "#undef > CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation > support */". > > I never got a boot-prompt. I tried mucking with the patch a little and > found that if I did: > > diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk > index aa84706..f8ef90f 100644 > --- a/arch/arm/cpu/arm926ejs/config.mk > +++ b/arch/arm/cpu/arm926ejs/config.mk > @@ -23,11 +23,6 @@ > > PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float > > -ifndef CONFIG_SYS_ARM_WITHOUT_RELOC > -# needed for optimal relocation > -PLATFORM_RELFLAGS += -msingle-pic-base > -endif > - > PLATFORM_CPPFLAGS += -march=armv5te > # ========================================================================= > # > > Then I do get a prompt. > > The gcc version here is: > $arm-none-linux-gnueabi-gcc --version > arm-none-linux-gnueabi-gcc (Sourcery G++ Lite 2009q1-203) 4.3.3 > Copyright (C) 2008 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > I have a JTAG debugger for this board. Please let me know if there are > register dumps or the like that I can get you. > > Best Regards, > Ben Gardiner Basically your test seems to demonstrate show that the pic base value computed in start.S does not work for board_init_f. Did you execute the binary at the address specified in TEXT_BASE? If not, please adjust TEXT_BASE to the location where u-boot resides when you debug it. Otherwise, can you do the following? This will help me see if the pic base computed in start.S is the same as the one computed by each function without -msingle-pic-base. 1) build with your fix in; 2) debug (at the assembly instruction level) the start.S code and see what value ends up in r10 (aka sl) right before calling board_init_f; 3) proceed (still at the assembly instruction level) until you get within board_init_f. Among the first instructions will be the recomputation of 10/sl; see what value it is assigned; 4) compare values found in 2 and 3 with the value of __got_base in the .map file. 5) Report your findings here. :) Thanks for your help! Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 19:07 ` Albert ARIBAUD @ 2010-09-22 20:51 ` Ben Gardiner 2010-09-22 21:36 ` Albert ARIBAUD 2010-09-23 16:37 ` Ben Gardiner 1 sibling, 1 reply; 31+ messages in thread From: Ben Gardiner @ 2010-09-22 20:51 UTC (permalink / raw) To: u-boot On Wed, Sep 22, 2010 at 3:07 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > Basically your test seems to demonstrate show that the pic base value > computed in start.S does not work for board_init_f. > > Did you execute the binary at the address specified in TEXT_BASE? If not, > please adjust TEXT_BASE to the location where u-boot resides when you debug > it. On boot we see printed "Jumping to entry point at 0xC1080000." by UBL. The debugger agrees: (gdb) info registers r0 0x2710 10000 r1 0x0 0 r2 0x80003478 2147497080 r3 0xc1080000 3238526976 r4 0x80003484 2147497092 r5 0x80003480 2147497088 r6 0xc7fe9760 3355350880 r7 0xc7fbd000 3355168768 r8 0xc7ea8f8c 3354038156 r9 0xa1892306 2710119174 r10 0xa1892306 2710119174 r11 0xc7ea8f84 3354038148 r12 0x30 48 sp 0x800037d0 0x800037d0 lr 0x80000120 2147483936 pc 0xc1080000 0xc1080000 <_start> fps 0x0 0 cpsr 0x600000d3 1610612947 (gdb) l 50 */ 51 52 53 .globl _start 54 _start: 55 b reset 56 #ifdef CONFIG_PRELOADER 57 /* No exception handlers in preloader */ 58 ldr pc, _hang 59 ldr pc, _hang (gdb) It appears that TEXT_BASE Is 0xc1080040 in the binary though it is specified as 0xc1080000: $arm-none-linux-gnueabi-nm u-boot |grep TEXT c1080040 T _TEXT_BASE $cat board/davinci/da8xxevm/config.mk |grep TEXT TEXT_BASE = 0xC1080000 but I guess that's OK since _TEXT_BASE is a label at start.S:118 ... I'm not sure if it is relevant but note that the da850 is a board for which we have "#undef CONFIG_SKIP_RELOCATE_UBOOT" > Otherwise, can you do the following? This will help me see if the pic base > computed in start.S is the same as the one computed by each function without > -msingle-pic-base. > > 1) build with your fix in; > > 2) debug (at the assembly instruction level) the start.S code and see what > value ends up in r10 (aka sl) right before calling board_init_f; (gdb) info registers r0 0x0 0 r1 0x0 0 r2 0x80003478 2147497080 r3 0xc1080000 3238526976 r4 0x80003484 2147497092 r5 0x80003480 2147497088 r6 0xc7fe9ba0 3355351968 r7 0xc7fbb000 3355160576 r8 0xc7ea6f8c 3354029964 r9 0xc10ae600 3238716928 r10 0xc10ae600 3238716928 r11 0xc7ea6d3c 3354029372 r12 0x30 48 sp 0xc0000f80 0xc0000f80 lr 0x80000120 2147483936 pc 0xc1080088 0xc1080088 <reset+32> fps 0x0 0 cpsr 0x600000d3 1610612947 (gdb) > 3) proceed (still at the assembly instruction level) until you get within > board_init_f. Among the first instructions will be the recomputation of > 10/sl; see what value it is assigned; I couldn't see any modification of r10 in board_init_f; what follows is the assembly of the first instructions of that function: c1080730 <board_init_f>: c1080730: e92d4800 push {fp, lr} c1080734: e28db004 add fp, sp, #4 ; 0x4 c1080738: e24dd020 sub sp, sp, #32 ; 0x20 c108073c: e59f21e8 ldr r2, [pc, #488] ; c108092c <board_init_f+0x1fc> c1080740: e50b2024 str r2, [fp, #-36] c1080744: e51b3024 ldr r3, [fp, #-36] c1080748: e08f3003 add r3, pc, r3 c108074c: e50b3024 str r3, [fp, #-36] c1080750: e50b0020 str r0, [fp, #-32] c1080754: e59f81d4 ldr r8, [pc, #468] ; c1080930 <board_init_f+0x200> c1080758: e1a03008 mov r3, r8 c108075c: e1a00003 mov r0, r3 c1080760: e3a01000 mov r1, #0 ; 0x0 c1080764: e3a0205c mov r2, #92 ; 0x5c c1080768: eb007a1c bl c109efe0 <memset> c108076c: e1a01008 mov r1, r8 c1080770: e59f31bc ldr r3, [pc, #444] ; c1080934 <board_init_f+0x204> c1080774: e51b0024 ldr r0, [fp, #-36] c1080778: e7903003 ldr r3, [r0, r3] c108077c: e5932000 ldr r2, [r3] c1080780: e59f31b0 ldr r3, [pc, #432] ; c1080938 <board_init_f+0x208> c1080784: e51b0024 ldr r0, [fp, #-36] c1080788: e7903003 ldr r3, [r0, r3] c108078c: e5933000 ldr r3, [r3] c1080790: e0633002 rsb r3, r3, r2 c1080794: e5813024 str r3, [r1, #36] c1080798: e59f319c ldr r3, [pc, #412] ; c108093c <board_init_f+0x20c> c108079c: e51b2024 ldr r2, [fp, #-36] c10807a0: e0823003 add r3, r2, r3 c10807a4: e50b3014 str r3, [fp, #-20] c10807a8: ea000009 b c10807d4 <board_init_f+0xa4> c10807ac: e51b3014 ldr r3, [fp, #-20] c10807b0: e5933000 ldr r3, [r3] c10807b4: e12fff33 blx r3 c10807b8: e1a03000 mov r3, r0 c10807bc: e3530000 cmp r3, #0 ; 0x0 c10807c0: 0a000000 beq c10807c8 <board_init_f+0x98> c10807c4: eb0000d3 bl c1080b18 <hang> c10807c8: e51b3014 ldr r3, [fp, #-20] c10807cc: e2833004 add r3, r3, #4 ; 0x4 c10807d0: e50b3014 str r3, [fp, #-20] c10807d4: e51b3014 ldr r3, [fp, #-20] c10807d8: e5933000 ldr r3, [r3] c10807dc: e3530000 cmp r3, #0 ; 0x0 c10807e0: 1afffff1 bne c10807ac <board_init_f+0x7c> c10807e4: e1a03008 mov r3, r8 c10807e8: e5933020 ldr r3, [r3, #32] c10807ec: e2833103 add r3, r3, #-1073741824 ; 0xc0000000 c10807f0: e50b300c str r3, [fp, #-12] c10807f4: e51b300c ldr r3, [fp, #-12] c10807f8: e2433901 sub r3, r3, #16384 ; 0x4000 c10807fc: e50b300c str r3, [fp, #-12] c1080800: e51b300c ldr r3, [fp, #-12] c1080804: e1a03823 lsr r3, r3, #16 c1080808: e1a03803 lsl r3, r3, #16 c108080c: e50b300c str r3, [fp, #-12] c1080810: e1a02008 mov r2, r8 c1080814: e51b300c ldr r3, [fp, #-12] c1080818: e5823034 str r3, [r2, #52] c108081c: e51b300c ldr r3, [fp, #-12] c1080820: e3c33eff bic r3, r3, #4080 ; 0xff0 c1080824: e3c3300f bic r3, r3, #15 ; 0xf just before board_init_f calls relocate_code the registers are as follows: (gdb) info registers r0 0xc7ea6f8c 3354029964 r1 0xc0000f80 3221229440 r2 0x0 0 r3 0xc7ea6f8c 3354029964 r4 0x80003484 2147497092 r5 0x80003480 2147497088 r6 0xc7fe9ba0 3355351968 r7 0xc7fbb000 3355160576 r8 0xc0000f80 3221229440 r9 0xc10ae600 3238716928 r10 0xc10ae600 3238716928 r11 0xc0000f7c 3221229436 r12 0xc10ae600 3238716928 sp 0xc0000f58 0xc0000f58 lr 0xc108091c 3238529308 pc 0xc108091c 0xc108091c <board_init_f+492> fps 0x0 0 cpsr 0x600000d3 1610612947 > 4) compare values found in 2 and 3 with the value of __got_base in the .map > file. In 2 and 3 r10 was 0xc10ae600; the System.map shows: $cat System.map |grep got_base c1080150 t relocate_got_base_r c108017c t _got_base c1080180 t _relocate_got_base_r c10ae600 A __got_base > Thanks for your help! My pleasure. Just for the sake of details: without the removal of the -msingle-pic-base I have the following register contents just before 'bl board_init_f' (start.S:219): (gdb) info registers r0 0x0 0 r1 0x0 0 r2 0x80003478 2147497080 r3 0xc1080000 3238526976 r4 0x80003484 2147497092 r5 0x80003480 2147497088 r6 0xc7fe9760 3355350880 r7 0xc7fbd000 3355168768 r8 0xc7ea8f8c 3354038156 r9 0xc10ac1c0 3238707648 r10 0xc10ac1c0 3238707648 r11 0xc7ea8f84 3354038148 r12 0x30 48 sp 0xc0000f80 0xc0000f80 lr 0x80000120 2147483936 pc 0xc1080088 0xc1080088 <reset+32> fps 0x0 0 cpsr 0x600000d3 1610612947 (gdb) and the System.map shows: $cat System.map |grep got_base c1080150 t relocate_got_base_r c108017c t _got_base c1080180 t _relocate_got_base_r c10ac1c0 A __got_base Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 20:51 ` Ben Gardiner @ 2010-09-22 21:36 ` Albert ARIBAUD 2010-09-22 22:07 ` Albert ARIBAUD 2010-09-23 14:44 ` Ben Gardiner 0 siblings, 2 replies; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-22 21:36 UTC (permalink / raw) To: u-boot As for _TEXT_BASE vs TEXT_BASE, this is normal: TEXT_BASE is the image base, and _TEXT_BASE is the address of a variable holding the value of TEXT_BASE, so it's not surprising that they are not equal. As for your problem: your reports show that start.S sets r10 and r9 as expected, but that board_init_f does not contain the pic prolog code. I wonder if you may have built it without -fPIC or -fPIE. Normally with -fPIE/PIC you should have seen board_init_f (and any function) start with something like: c1080690: e59fa120 ldr sl, [pc, #288] ; c10807b8 <board_init_f+0x128> c1080694: e08fa00a add sl, pc, sl Can you check the build logs to see which options were applied to lib/arm/board.c? I'll try with your CodeSourcery toolchain (I'm using the ELDK 4.2 one), but there is no reason why it would not be EABI compliant. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 21:36 ` Albert ARIBAUD @ 2010-09-22 22:07 ` Albert ARIBAUD 2010-09-23 14:44 ` Ben Gardiner 1 sibling, 0 replies; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-22 22:07 UTC (permalink / raw) To: u-boot I've just recompiled with the CodeSourcery toolchain and I can see the board_init_f prolog going thus: c1080688: e59fa118 ldr sl, [pc, #280] ; c10807a8 <board_init_f+0x120> c108068c: e59f8118 ldr r8, [pc, #280] ; c10807ac <board_init_f+0x124> c1080690: e08fa00a add sl, pc, sl It is scattered, but it is there. Also, I see board_init_f starting at c108064c. Ben, that's a different address from what you see in your map file, but maybe we're not using the same exact source tree? I tested with the current 'next' branch plus my two patches. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 21:36 ` Albert ARIBAUD 2010-09-22 22:07 ` Albert ARIBAUD @ 2010-09-23 14:44 ` Ben Gardiner 2010-09-23 15:13 ` Albert ARIBAUD 2010-09-23 16:53 ` Ben Gardiner 1 sibling, 2 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-23 14:44 UTC (permalink / raw) To: u-boot On Wed, Sep 22, 2010 at 5:36 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > Can you check the build logs to see which options were applied to > lib/arm/board.c? make[1]: Entering directory `[...]/arch/arm/lib' arm-none-linux-gnueabi-gcc -g -Os -fPIE -fno-common -ffixed-r8 -msoft-float -msingle-pic-base -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DTEXT_BASE=0xC1080000 -I[...]/include -fno-builtin -ffreestanding -nostdinc -isystem /opt/codesourcery-arm-none-eabi-2009q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.3.3/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv5te -march=armv5te -Wall -Wstrict-prototypes -fno-stack-protector \ -o board.o board.c -c On Wed, Sep 22, 2010 at 6:07 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > I've just recompiled with the CodeSourcery toolchain and I can see the > board_init_f prolog going thus: > > c1080688: ? ? ? e59fa118 ? ? ? ?ldr ? ? sl, [pc, #280] ?; c10807a8 > <board_init_f+0x120> > > c108068c: ? ? ? e59f8118 ? ? ? ?ldr ? ? r8, [pc, #280] ?; c10807ac > <board_init_f+0x124> > > c1080690: ? ? ? e08fa00a ? ? ? ?add ? ? sl, pc, sl > > It is scattered, but it is there. Yes, I see them now that you point it out; but they are not pc-relative for here; c10806a4: e514a004 ldr sl, [r4, #-4] c10806a8: e35a0000 cmp sl, #0 ; 0x0 ... What version of that compiler did you try? I tried both the 2009q1 and 2009q3 compilers. > Also, I see board_init_f starting at c108064c. Ben, that's a different > address from what you see in your map file, but maybe we're not using the > same exact source tree? I tested with the current 'next' branch plus my two > patches. I had applied your patches to d70d8ccc200db8c16a6654cb726c3d74b6640b32. I rebased your patches onto e69e520f9d235bb7d96296081fdfc09b9fee8c46 this morning and I now have "c108064c <board_init_f>:" same as you. But I still do not get a boot prompt. I am downloading the arm-2008-11-24.iso for ELDK-4.2 to try to reproduce your results. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 14:44 ` Ben Gardiner @ 2010-09-23 15:13 ` Albert ARIBAUD 2010-09-23 15:35 ` Ben Gardiner 2010-09-23 16:53 ` Ben Gardiner 1 sibling, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-23 15:13 UTC (permalink / raw) To: u-boot 23/09/2010 16:44, Ben Gardiner a ?crit : > On Wed, Sep 22, 2010 at 5:36 PM, Albert ARIBAUD<albert.aribaud@free.fr> wrote: >> Can you check the build logs to see which options were applied to >> lib/arm/board.c? > > make[1]: Entering directory `[...]/arch/arm/lib' > arm-none-linux-gnueabi-gcc -g -Os -fPIE -fno-common -ffixed-r8 > -msoft-float -msingle-pic-base -fno-common -ffixed-r8 -msoft-float > -D__KERNEL__ -DTEXT_BASE=0xC1080000 -I[...]/include -fno-builtin > -ffreestanding -nostdinc -isystem > /opt/codesourcery-arm-none-eabi-2009q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.3.3/include > -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux > -mno-thumb-interwork -march=armv5te -march=armv5te -Wall > -Wstrict-prototypes -fno-stack-protector \ > -o board.o board.c -c This command line has -msingle-pic-base; that's why there is no pic recomputation in board_init_f. Looks like you compiled with my patches unaltered, whereas I thought you were compiling with your fix above m patches. Can you either apply your fix above my patches or just compile with the first of my patches? > What version of that compiler did you try? I tried both the 2009q1 and > 2009q3 compilers. For this case I downloaded and used the same 2009q1 you're using. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 15:13 ` Albert ARIBAUD @ 2010-09-23 15:35 ` Ben Gardiner 0 siblings, 0 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-23 15:35 UTC (permalink / raw) To: u-boot On Thu, Sep 23, 2010 at 11:13 AM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > This command line has -msingle-pic-base; that's why there is no pic > recomputation in board_init_f. Looks like you compiled with my patches > unaltered, whereas I thought you were compiling with your fix above m > patches. Can you either apply your fix above my patches or just compile with > the first of my patches? Right. I forgot about that part. I also now recognize the prologue to which you were referring. I will complete the testing originally requested. >> What version of that compiler did you try? I tried both the 2009q1 and >> 2009q3 compilers. > > For this case I downloaded and used the same 2009q1 you're using. Ok. Thank you for clarifying. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 14:44 ` Ben Gardiner 2010-09-23 15:13 ` Albert ARIBAUD @ 2010-09-23 16:53 ` Ben Gardiner 1 sibling, 0 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-23 16:53 UTC (permalink / raw) To: u-boot On Thu, Sep 23, 2010 at 10:44 AM, Ben Gardiner <bengardiner@nanometrics.ca> wrote: > I am downloading the arm-2008-11-24.iso for ELDK-4.2 to try to > reproduce your results. When I build u-boot/next plus your patches using the ELDK 4.2 arm toolchain I encounter the same problem as before; there is no boot prompt. Please let me know if there is any other debugging information I can get for you. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 19:07 ` Albert ARIBAUD 2010-09-22 20:51 ` Ben Gardiner @ 2010-09-23 16:37 ` Ben Gardiner 2010-09-23 17:04 ` Albert ARIBAUD 1 sibling, 1 reply; 31+ messages in thread From: Ben Gardiner @ 2010-09-23 16:37 UTC (permalink / raw) To: u-boot On Wed, Sep 22, 2010 at 3:07 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > 1) build with your fix in; $git log --format=oneline|head -n 3 f619c1537af105cd1471f9447ac9132feab79c3a arm926ejs: reduce code size with -msingle-pic-base 6285f63589a87dfd28c7d73ff68075c5d1ee7f35 arm: change relocation flag from -fPIC to -fPIE e69e520f9d235bb7d96296081fdfc09b9fee8c46 fsl: refactor MPC8610 and MPC5121 DIU code to use existing bitmap and logo features $git diff diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk index aa84706..f8ef90f 100644 --- a/arch/arm/cpu/arm926ejs/config.mk +++ b/arch/arm/cpu/arm926ejs/config.mk @@ -23,11 +23,6 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float -ifndef CONFIG_SYS_ARM_WITHOUT_RELOC -# needed for optimal relocation -PLATFORM_RELFLAGS += -msingle-pic-base -endif - PLATFORM_CPPFLAGS += -march=armv5te # ========================================================================= # $make mrproper; make da850evm_config; make -j9 all [...] make[1]: Entering directory `[...]arch/arm/lib' arm-none-linux-gnueabi-gcc -g -Os -fPIE -fno-common -ffixed-r8 -msoft-float -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ -DTEXT_BASE=0xC1080000 -I[...]/include -fno-builtin -ffreestanding -nostdinc -isystem /opt/codesourcery-arm-none-eabi-2009q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.3.3/include -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux -mno-thumb-interwork -march=armv5te -march=armv5te -Wall -Wstrict-prototypes -fno-stack-protector \ -o board.o board.c -c [...] > 2) debug (at the assembly instruction level) the start.S code and see what > value ends up in r10 (aka sl) right before calling board_init_f; (gdb) p /x $r10 $1 = 0xc1098564 (gdb) p /x $pc $2 = 0xc1080088 > 3) proceed (still at the assembly instruction level) until you get within > board_init_f. Among the first instructions will be the recomputation of > 10/sl; see what value it is assigned; (gdb) p /x $r10 $5 = 0xc1098564 (gdb) p /x $pc $6 = 0xc1080694 (gdb) > 4) compare values found in 2 and 3 with the value of __got_base in the .map > file. $cat System.map |grep __got_base c1098564 A __got_base I hope that helps. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 16:37 ` Ben Gardiner @ 2010-09-23 17:04 ` Albert ARIBAUD 2010-09-23 21:13 ` Ben Gardiner 0 siblings, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-23 17:04 UTC (permalink / raw) To: u-boot Le 23/09/2010 18:37, Ben Gardiner a ?crit : > On Wed, Sep 22, 2010 at 3:07 PM, Albert ARIBAUD<albert.aribaud@free.fr> wrote: >> 1) build with your fix in; > > $git log --format=oneline|head -n 3 > f619c1537af105cd1471f9447ac9132feab79c3a arm926ejs: reduce code size > with -msingle-pic-base > 6285f63589a87dfd28c7d73ff68075c5d1ee7f35 arm: change relocation flag > from -fPIC to -fPIE > e69e520f9d235bb7d96296081fdfc09b9fee8c46 fsl: refactor MPC8610 and > MPC5121 DIU code to use existing bitmap and logo features > > $git diff > diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk > index aa84706..f8ef90f 100644 > --- a/arch/arm/cpu/arm926ejs/config.mk > +++ b/arch/arm/cpu/arm926ejs/config.mk > @@ -23,11 +23,6 @@ > > PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float > > -ifndef CONFIG_SYS_ARM_WITHOUT_RELOC > -# needed for optimal relocation > -PLATFORM_RELFLAGS += -msingle-pic-base > -endif > - > PLATFORM_CPPFLAGS += -march=armv5te > # ========================================================================= > # > > $make mrproper; make da850evm_config; make -j9 all > [...] > make[1]: Entering directory `[...]arch/arm/lib' > arm-none-linux-gnueabi-gcc -g -Os -fPIE -fno-common -ffixed-r8 > -msoft-float -fno-common -ffixed-r8 -msoft-float -D__KERNEL__ > -DTEXT_BASE=0xC1080000 -I[...]/include -fno-builtin -ffreestanding > -nostdinc -isystem > /opt/codesourcery-arm-none-eabi-2009q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.3.3/include > -pipe -DCONFIG_ARM -D__ARM__ -marm -mabi=aapcs-linux > -mno-thumb-interwork -march=armv5te -march=armv5te -Wall > -Wstrict-prototypes -fno-stack-protector \ > -o board.o board.c -c > [...] > >> 2) debug (at the assembly instruction level) the start.S code and see what >> value ends up in r10 (aka sl) right before calling board_init_f; > > (gdb) p /x $r10 > $1 = 0xc1098564 > (gdb) p /x $pc > $2 = 0xc1080088 > >> 3) proceed (still at the assembly instruction level) until you get within >> board_init_f. Among the first instructions will be the recomputation of >> 10/sl; see what value it is assigned; > > (gdb) p /x $r10 > $5 = 0xc1098564 > (gdb) p /x $pc > $6 = 0xc1080694 > (gdb) > >> 4) compare values found in 2 and 3 with the value of __got_base in the .map >> file. > > $cat System.map |grep __got_base > c1098564 A __got_base > > I hope that helps. Well, it confirms that the start.S code computes a correct r10 / sl register, since it is the same value as board_init_f (and any other function) computes when -msingle-pic-base is not present, and this value is that of __got_base. Note that if I'm not mistaken, your build uses r9 as the pic base, not 10 -- this is a possibility, and my patches are written so as to allow either register use. Now, we need to find out why board_init_f fails to go through the init sequence and print out at least the banner and some diagnostics. Please build with my two patches alone (i.e., put -msingle-pic-base back in place, which will remove the recomputation of the pic base in every function), and run it until board_init_f() calls its first init function through (*init_fnc_ptr)(). This call should be materialized by a 'blx <reg>' instruction; please report the value of <reg> at that point as well as the addresses of arch_cpu_init, board_early_init_f, and timer_init; <reg> should be one of these depending on the board config. > Best Regards, > Ben Gardiner Thanks again for your help in testing my patches! Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 17:04 ` Albert ARIBAUD @ 2010-09-23 21:13 ` Ben Gardiner 2010-09-23 21:30 ` Albert ARIBAUD 0 siblings, 1 reply; 31+ messages in thread From: Ben Gardiner @ 2010-09-23 21:13 UTC (permalink / raw) To: u-boot On Thu, Sep 23, 2010 at 1:04 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > Note that if I'm not mistaken, your build uses r9 as the pic base, not 10 -- > this is a possibility, and my patches are written so as to allow either > register use. I don't know for sure so I did a little poking around: c1080688 <board_init_f>: c1080688: e59fa118 ldr sl, [pc, #280] ; c10807a8 <board_init_f+0x120> c108068c: e59f8118 ldr r8, [pc, #280] ; c10807ac <board_init_f+0x124> (gdb) info registers [...] r9 0xc1098564 3238626660 r10 0xc1098564 3238626660 [...] pc 0xc1080688 0xc1080688 <board_init_f> [...] (gdb) ni 514 gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR); (gdb) info registers [...] r9 0xc1098564 3238626660 r10 0x17ecc 97996 [...] pc 0xc108068c 0xc108068c <board_init_f+4> [...] (gdb) p /x $sl $1 = 0x17ecc (gdb) p /x $r10 $2 = 0x17ecc (gdb) p /x $r9 $3 = 0xc1098564 (gdb) p *0xc10807a8 $4 = 97996 (gdb) p /x *0xc10807a8 $5 = 0x17ecc I think that confirms that sl == r10 . > Now, we need to find out why board_init_f fails to go through the init > sequence and print out at least the banner and some diagnostics. > > Please build with my two patches alone (i.e., put -msingle-pic-base back in > place, which will remove the recomputation of the pic base in every > function), and run it until board_init_f() calls its first init function > through (*init_fnc_ptr)(). This call should be materialized by a 'blx <reg>' > instruction; please report the value of <reg> at that point as well as the > addresses of arch_cpu_init, board_early_init_f, and timer_init; <reg> should > be one of these depending on the board config. <reg> is r10/sl : for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { if ((*init_fnc_ptr)() != 0) { c1080690: e12fff3a blx sl c1080694: e2844004 add r4, r4, #4 ; 0x4 c1080698: e3500000 cmp r0, #0 ; 0x0 c108069c: 0a000000 beq c10806a4 <board_init_f+0x58> hang (); c10806a0: ebffff76 bl c1080480 <hang> Here's the addresses you requested for the very first time it hit that function call through (*init_fnc_prt)(): (gdb) p /x $pc $4 = 0xc1080690 (gdb) p /x $sl $5 = 0xc1091e6c (gdb) p /x $r10 $6 = 0xc1091e6c There is no arch_cpu_init or board_early_init_f; timer_init is at 0xc1091e6c. So it is timer_init which is being called above. The call to timer_init completes successfully; the next function pointer dereferenced and called is 0xc1087e04 == env_init. That call completes successfully. The next is 0xc10804f8 == init_baudrate; it completes successfully. The next call is 0xc108119c == serial_init; it completes successfully. The next is 0xc1086550 == console_init_f; it completes successfully. The next is 0xc10804d0 == display_banner; it completes successfully. They all did. Execution reaches the call to relocate_code and enters; the last instruction I am able to single-step is the 'ldr r10, _got_base' in the following lines from your patch: > + /* > + * Set pic base register to the current GOT position. Since we are > + * now running from RAM, we need to offset it from its link-time to > + * its run-time position. > + */ > + ldr r9, relocate_got_base_r > + sub r9, pc, r9 > + ldr r10, _got_base > +relocate_got_base_r: > + add r10, r9, r10 > + mov r9, r10 > + > Thanks again for your help in testing my patches! My pleasure. I hope we can get them working on the da850evm. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 21:13 ` Ben Gardiner @ 2010-09-23 21:30 ` Albert ARIBAUD 2010-09-24 13:38 ` Ben Gardiner 0 siblings, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-23 21:30 UTC (permalink / raw) To: u-boot Le 23/09/2010 23:13, Ben Gardiner a ?crit : > On Thu, Sep 23, 2010 at 1:04 PM, Albert ARIBAUD<albert.aribaud@free.fr> wrote: >> Note that if I'm not mistaken, your build uses r9 as the pic base, not 10 -- >> this is a possibility, and my patches are written so as to allow either >> register use. > > I don't know for sure so I did a little poking around: > > c1080688<board_init_f>: > c1080688: e59fa118 ldr sl, [pc, #280] ; c10807a8 > <board_init_f+0x120> > c108068c: e59f8118 ldr r8, [pc, #280] ; c10807ac > <board_init_f+0x124> > > (gdb) info registers > [...] > r9 0xc1098564 3238626660 > r10 0xc1098564 3238626660 > [...] > pc 0xc1080688 0xc1080688<board_init_f> > [...] > (gdb) ni > 514 gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR); > (gdb) info registers > [...] > r9 0xc1098564 3238626660 > r10 0x17ecc 97996 > [...] > pc 0xc108068c 0xc108068c<board_init_f+4> > [...] > (gdb) p /x $sl > $1 = 0x17ecc > (gdb) p /x $r10 > $2 = 0x17ecc > (gdb) p /x $r9 > $3 = 0xc1098564 > (gdb) p *0xc10807a8 > $4 = 97996 > (gdb) p /x *0xc10807a8 > $5 = 0x17ecc > > I think that confirms that sl == r10 . Watch out: 'sl' is always 'r10': those are alias names of the register; I specify both names because depending on the tool you use, you might see either name. But the pic base, i.e. the register which holds the address of the GOT, is not necessarily sl! It can can be r10 *or* r9, depending on the toolchain and options. Here your excepts show that r9 remains constant (and, I suspect, its value is the address of the symbol __got_base) while r10 varies; r9 is the pic base. Which is lucky because r10 is trashed by the loop that goes through init_sequence and runs each of the functions it points to. > There is no arch_cpu_init or board_early_init_f; timer_init is at > 0xc1091e6c. So it is timer_init which is being called above. > > The call to timer_init completes successfully; the next function > pointer dereferenced and called is 0xc1087e04 == env_init. That call > completes successfully. The next is 0xc10804f8 == init_baudrate; it > completes successfully. The next call is 0xc108119c == serial_init; it > completes successfully. The next is 0xc1086550 == console_init_f; it > completes successfully. The next is 0xc10804d0 == display_banner; it > completes successfully. They all did. Ok, so all this goes fine, yet you do not see the banner displayed? > Execution reaches the call to > relocate_code and enters; the last instruction I am able to > single-step is the 'ldr r10, _got_base' in the following lines > from your patch: What happens when you single-step through it? Any kind of error message, or does the target simply not respond any more? >> Thanks again for your help in testing my patches! > > My pleasure. I hope we can get them working on the da850evm. We certainly will. For me to reproduce your SW setting and build exactly the same binary as you do, can you confirm that you are again using the 2009q1 toolchain, and indicate the exact command line you use to build your target? Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 21:30 ` Albert ARIBAUD @ 2010-09-24 13:38 ` Ben Gardiner 2010-09-24 16:08 ` Albert ARIBAUD 0 siblings, 1 reply; 31+ messages in thread From: Ben Gardiner @ 2010-09-24 13:38 UTC (permalink / raw) To: u-boot On Thu, Sep 23, 2010 at 5:30 PM, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > Watch out: 'sl' is always 'r10': those are alias names of the register; I > specify both names because depending on the tool you use, you might see > either name. But the pic base, i.e. the register which holds the address of > the GOT, is not necessarily sl! It can can be r10 *or* r9, depending on the > toolchain and options. > > Here your excepts show that r9 remains constant (and, I suspect, its value > is the address of the symbol __got_base) while r10 varies; r9 is the pic > base. Which is lucky because r10 is trashed by the loop that goes through > init_sequence and runs each of the functions it points to. Ok :) So i showed that the alias is true. :) -- Thanks for the info, I'll need to do some research into what the pic base is with this toolchain. >> There is no arch_cpu_init or board_early_init_f; timer_init is at >> 0xc1091e6c. So it is timer_init which is being called above. >> >> The call to timer_init completes successfully; the next function >> pointer dereferenced and called is 0xc1087e04 == env_init. That call >> completes successfully. The next is 0xc10804f8 == init_baudrate; it >> completes successfully. The next call is 0xc108119c == serial_init; it >> completes successfully. The next is 0xc1086550 == console_init_f; it >> completes successfully. The next is 0xc10804d0 == display_banner; ?it >> completes successfully. They all did. > > Ok, so all this goes fine, yet you do not see the banner displayed? Correct. >> Execution reaches the call to >> relocate_code and enters; the last instruction I am able to >> single-step is the 'ldr ? ? r10, _got_base' in the following lines >> from your patch: > > What happens when you single-step through it? Any kind of error message, or > does the target simply not respond any more? No output on the serial console or from the debugger software. As you say, the target simply does not respond anymore. >>> Thanks again for your help in testing my patches! >> >> My pleasure. I hope we can get them working on the da850evm. > > We certainly will. For me to reproduce your SW setting and build exactly the > same binary as you do, can you confirm that you are again using the 2009q1 > toolchain, and indicate the exact command line you use to build your target? It was the 2009q1 toolchain. export CROSS_COMPILE=arm-none-linux-gnueabi- export PATH=/opt/codesourcery-arm-none-eabi-2009q1/bin/:${PATH} export ARCH=arm make mrproper ; make da850evm_config ; make -j9 all Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-24 13:38 ` Ben Gardiner @ 2010-09-24 16:08 ` Albert ARIBAUD 0 siblings, 0 replies; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-24 16:08 UTC (permalink / raw) To: u-boot I think I have found the root cause of your issue. I was actually lucky that it worked at all for me; and as Heiko's board skipped relocation, it would not trigger the bug either... My computing of r9/r10 in the relocate_code stage was plain wrong, based on the FLASH execution location, not the RAM one. :/ I'll post a V2 patch set within the next hour -- Heiko, this second patchset has your fixes except I mouved the r9/r10 recomputation inside the #ifndef CONFIG_SKIP_RELOCATE_UBOOT, the logic being that if we can skip relocation, then we are already running at the right location and we don't need fixing the pic base either. Ben and Heiko, please (re-)test the patch when it's out -- sorry for the extra workload. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 18:05 ` Ben Gardiner 2010-09-22 19:07 ` Albert ARIBAUD @ 2010-09-22 20:30 ` Wolfgang Denk 2010-09-22 20:55 ` Ben Gardiner 1 sibling, 1 reply; 31+ messages in thread From: Wolfgang Denk @ 2010-09-22 20:30 UTC (permalink / raw) To: u-boot Dear Ben Gardiner, In message <AANLkTinK1PWePkBaBHEnxcyV6eOwtjZb-7+kA-JhHM9Z@mail.gmail.com> you wrote: > > I tried tested this on da850evm -- the da850evm.h has "#undef > CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation > support */". This slipped through, sorry for that. That line must be removed. 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 How many Unix hacks does it take to change a light bulb? Let's see, can you use a shell script for that or does it need a C program? ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 20:30 ` Wolfgang Denk @ 2010-09-22 20:55 ` Ben Gardiner 2010-09-22 21:11 ` Wolfgang Denk 0 siblings, 1 reply; 31+ messages in thread From: Ben Gardiner @ 2010-09-22 20:55 UTC (permalink / raw) To: u-boot Hi Wolfgang, On Wed, Sep 22, 2010 at 4:30 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Ben Gardiner, > > In message <AANLkTinK1PWePkBaBHEnxcyV6eOwtjZb-7+kA-JhHM9Z@mail.gmail.com> you wrote: >> >> I tried tested this on da850evm -- the da850evm.h has "#undef >> CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation >> support */". > > This slipped through, sorry for that. > > That line must be removed. I don't understand; I thought that '#undef' _enabled_ relocation support. According to doc/README.arm-relocation on u-boot./next: To compile a board without relocation, define CONFIG_SYS_ARM_WITHOUT_RELOC This possibility will removed!! So please fix your board to compile without CONFIG_SYS_ARM_WITHOUT_RELOC defined!!! But I did test relocation on this board with Heiko and it is working as-is on d70d8ccc200db8c16a6654cb726c3d74b6640b32 of u-boot/next; we found that 97003756249bd790910417eb66f0039bbf06a02c made it work. I can confirm that -- as you suggest -- the following applied to u-boot/next diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index d02b196..e0a3bae 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -138,7 +138,6 @@ #endif /* additions for new relocation code, must added to all boards */ -#undef CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation support */ #define CONFIG_SYS_SDRAM_BASE 0xc0000000 #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x1000 - /* Fix this */ \ CONFIG_SYS_GBL_DATA_SIZE) also works on da850evm. But isn't this disabling the recently added relocation support? Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 20:55 ` Ben Gardiner @ 2010-09-22 21:11 ` Wolfgang Denk 2010-09-22 21:33 ` Ben Gardiner 0 siblings, 1 reply; 31+ messages in thread From: Wolfgang Denk @ 2010-09-22 21:11 UTC (permalink / raw) To: u-boot Dear Ben Gardiner, In message <AANLkTinLq9XStXA=6w1BLKwCrDuXAPYpK1X3XW5MruW7@mail.gmail.com> you wrote: > > >> I tried tested this on da850evm -- the da850evm.h has "#undef > >> CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation > >> support */". > > > > This slipped through, sorry for that. > > > > That line must be removed. > > I don't understand; I thought that '#undef' _enabled_ relocation > support. According to doc/README.arm-relocation on u-boot./next: We do not #undef what is not #defined in the first place. > To compile a board without relocation, define CONFIG_SYS_ARM_WITHOUT_RELOC > This possibility will removed!! So please fix your board to compile without > CONFIG_SYS_ARM_WITHOUT_RELOC defined!!! If your board is fixed, then there is no need to build it with CONFIG_SYS_ARM_WITHOUT_RELOC set. 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] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 21:11 ` Wolfgang Denk @ 2010-09-22 21:33 ` Ben Gardiner 0 siblings, 0 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-22 21:33 UTC (permalink / raw) To: u-boot On Wed, Sep 22, 2010 at 5:11 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Ben Gardiner, > > In message <AANLkTinLq9XStXA=6w1BLKwCrDuXAPYpK1X3XW5MruW7@mail.gmail.com> you wrote: >> >> >> I tried tested this on da850evm -- the da850evm.h has "#undef >> >> CONFIG_SYS_ARM_WITHOUT_RELOC /* This board is tested with relocation >> >> support */". >> > >> > This slipped through, sorry for that. >> > >> > That line must be removed. >> >> I don't understand; I thought that '#undef' _enabled_ relocation >> support. According to doc/README.arm-relocation on u-boot./next: > > We do not #undef what is not #defined in the first place. :) Good point. Patch is coming. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-22 13:57 ` [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base Albert Aribaud 2010-09-22 18:05 ` Ben Gardiner @ 2010-09-23 7:12 ` Heiko Schocher 2010-09-23 8:05 ` Albert ARIBAUD 1 sibling, 1 reply; 31+ messages in thread From: Heiko Schocher @ 2010-09-23 7:12 UTC (permalink / raw) To: u-boot Hello Albert, Albert Aribaud wrote: > Add -msingle-pic-base to the relocation flags, and compute the pic base > in start.S twice and for all -- once before relocation to run board_init_f, > and once after relocation to run board_init_r and the rest of u-boot. > This further reduces code size by 2.5% compared to -fPIE alone. > > Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> > --- > arch/arm/cpu/arm926ejs/config.mk | 5 ++++ > arch/arm/cpu/arm926ejs/start.S | 40 ++++++++++++++++++++++++++++++++++-- > arch/arm/cpu/arm926ejs/u-boot.lds | 1 + > 3 files changed, 43 insertions(+), 3 deletions(-) I get with your 2 patches for the tx25 board: [hs at pollux u-boot]$ ./MAKEALL tx25 Configuring for tx25 board... /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S: Assembler messages: /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:212: Error: internal_relocation (type: OFFSET_IMM) not fixed up /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:308: Error: internal_relocation (type: OFFSET_IMM) not fixed up make[1]: *** [start.o] Fehler 1 make: *** [nand_spl] Fehler 2 make: *** Warte auf noch nicht beendete Prozesse... make: INTERNAL: Exiting with 4 jobserver tokens available; should be 3! arm-linux-size: './u-boot': No such file --------------------- SUMMARY ---------------------------- Boards compiled: 1 Boards with warnings or errors: 1 ( tx25 ) ---------------------------------------------------------- [hs at pollux u-boot]$ below fix fixes this issue :-) I had to define _got_base and _relocate_got_base_r also for the NAND_SPL case, and don;t need to fix up the "pic base register", if we are at the right position. I think we(you ;-) should add my below fix to your [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base and send a second version of it, so we are bisect compatible. you can add my Signed-off-by: Heiko Schocher <hs@denx.de> to it. Can you test this also on your board? Thanks! actual next build for the tx25 board: [hs at pollux u-boot]$ ./MAKEALL tx25 Configuring for tx25 board... text data bss dec hex filename 149540 9368 36980 195888 2fd30 ./u-boot with your 2 patches, and my below fix: [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base [hs at pollux u-boot]$ ./MAKEALL tx25 Configuring for tx25 board... text data bss dec hex filename 146172 9172 36980 192324 2ef44 ./u-boot So, this saves for this board 0xdec bytes! ----------------------------------------------------------------------- here now the fix for the tx25 board: diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 739fa06..0f0a503 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -277,6 +277,18 @@ fixloop: #endif #endif /* #ifndef CONFIG_SKIP_RELOCATE_UBOOT */ + /* + * Set pic base register to the current GOT position. Since we are + * now running from RAM, we need to offset it from its link-time to + * its run-time position. + */ + ldr r9, relocate_got_base_r + sub r9, pc, r9 + ldr r10, _got_base +relocate_got_base_r: + add r10, r9, r10 + mov r9, r10 + clear_bss: #ifndef CONFIG_PRELOADER ldr r0, _bss_start @@ -298,18 +310,6 @@ clbss_l:str r2, [r0] /* clear loop... */ bl red_LED_on #endif - /* - * Set pic base register to the current GOT position. Since we are - * now running from RAM, we need to offset it from its link-time to - * its run-time position. - */ - ldr r9, relocate_got_base_r - sub r9, pc, r9 - ldr r10, _got_base -relocate_got_base_r: - add r10, r9, r10 - mov r9, r10 - /* * We are done. Do not return, instead branch to second part of board * initialization, now running from RAM. @@ -331,14 +331,13 @@ _nand_boot: .word nand_boot mov pc, lr _board_init_r: .word board_init_r +#endif _got_base: .word __got_base _relocate_got_base_r: .word relocate_got_base_r -#endif - #else /* #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) */ /* * the actual reset code diff --git a/board/karo/tx25/config.mk b/board/karo/tx25/config.mk index 51ca1ab..1a32c87 100644 --- a/board/karo/tx25/config.mk +++ b/board/karo/tx25/config.mk @@ -1,5 +1,5 @@ ifdef CONFIG_NAND_SPL TEXT_BASE = 0x810c0000 else -TEXT_BASE = 0x81fc0000 +TEXT_BASE = 0x81fc1000 endif diff --git a/include/configs/tx25.h b/include/configs/tx25.h index 7faa453..dfe8eab 100644 --- a/include/configs/tx25.h +++ b/include/configs/tx25.h @@ -41,7 +41,7 @@ #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x800 #define CONFIG_SYS_NAND_U_BOOT_SIZE 0x30000 -#define CONFIG_SYS_NAND_U_BOOT_DST (0x81fc0000) +#define CONFIG_SYS_NAND_U_BOOT_DST (0x81fc1000) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_NAND_U_BOOT_DST #define CONFIG_SYS_NAND_PAGE_SIZE 2048 diff --git a/nand_spl/board/karo/tx25/u-boot.lds b/nand_spl/board/karo/tx25/u-boot.lds index c572557..b60c0df 100644 --- a/nand_spl/board/karo/tx25/u-boot.lds +++ b/nand_spl/board/karo/tx25/u-boot.lds @@ -55,6 +55,7 @@ SECTIONS __got_start = .; . = ALIGN(4); + __got_base = .; .got : { *(.got) } __got_end = .; -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 7:12 ` Heiko Schocher @ 2010-09-23 8:05 ` Albert ARIBAUD 2010-09-23 10:08 ` Heiko Schocher 0 siblings, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-23 8:05 UTC (permalink / raw) To: u-boot Hello Heiko, Thanks for the in-depth analysis. IIUC: Le 23/09/2010 09:12, Heiko Schocher a ?crit : > /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:212: Error: internal_relocation (type: OFFSET_IMM) not fixed up > /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:308: Error: internal_relocation (type: OFFSET_IMM) not fixed up This is due to the fact you're using a specific .lds, and your fix below corrects this for your .lds indeed. However, there might be other boards in the arm926 subtree which have their own .lds and would need the same fix, so I'll find them all and extend your fix to others. > I had to define _got_base and _relocate_got_base_r also for > the NAND_SPL case, and don;t need to fix up the "pic base > register", if we are at the right position. Thanks for spotting and fixing these. :) > I think we(you ;-) should add my below fix to your > > [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base > > and send a second version of it, so we are bisect compatible. > > you can add my > > Signed-off-by: Heiko Schocher<hs@denx.de> > > to it. I will -- thanks! > Can you test this also on your board? Thanks! I will too. :) Just one thing: are these TEXT_BASE / CONFIG_SYS_NAND_U_BOOT_DST modifications in your patch really needed for the board to run, or are they a manual layout optimization decided based on the code size reduction? > diff --git a/board/karo/tx25/config.mk b/board/karo/tx25/config.mk > index 51ca1ab..1a32c87 100644 > --- a/board/karo/tx25/config.mk > +++ b/board/karo/tx25/config.mk > @@ -1,5 +1,5 @@ > ifdef CONFIG_NAND_SPL > TEXT_BASE = 0x810c0000 > else > -TEXT_BASE = 0x81fc0000 > +TEXT_BASE = 0x81fc1000 > endif > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > index 7faa453..dfe8eab 100644 > --- a/include/configs/tx25.h > +++ b/include/configs/tx25.h > @@ -41,7 +41,7 @@ > #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x800 > #define CONFIG_SYS_NAND_U_BOOT_SIZE 0x30000 > > -#define CONFIG_SYS_NAND_U_BOOT_DST (0x81fc0000) > +#define CONFIG_SYS_NAND_U_BOOT_DST (0x81fc1000) > #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_NAND_U_BOOT_DST > > #define CONFIG_SYS_NAND_PAGE_SIZE 2048 Thanks again for your work on my patch! Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 8:05 ` Albert ARIBAUD @ 2010-09-23 10:08 ` Heiko Schocher 2010-09-23 12:45 ` Albert ARIBAUD 0 siblings, 1 reply; 31+ messages in thread From: Heiko Schocher @ 2010-09-23 10:08 UTC (permalink / raw) To: u-boot Hello Albert, Albert ARIBAUD wrote: > Hello Heiko, > > Thanks for the in-depth analysis. > > IIUC: > > Le 23/09/2010 09:12, Heiko Schocher a ?crit : > >> /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:212: Error: internal_relocation (type: OFFSET_IMM) not fixed up >> /home/hs/celf/u-boot/arch/arm/cpu/arm926ejs/start.S:308: Error: internal_relocation (type: OFFSET_IMM) not fixed up > > This is due to the fact you're using a specific .lds, and your fix below > corrects this for your .lds indeed. However, there might be other boards Yep. > in the arm926 subtree which have their own .lds and would need the same > fix, so I'll find them all and extend your fix to others. Hmm.. I don;t know the other boards ... so, I decided for the relocation patches not to fix all arm boards, instead inroduce the CONFIG_SYS_ARM_WITHOUT_RELOC, and board maintainers (or others who have time for it) can adapt the boards to this new code. So we always have a chance to keep track which boards are converted *and* working and which boards are not working ... so, I could not say, if it is a good idea to fix other boards, you could not test (I know, usually we do this fixes, but the relocation code is such a deep change, I prefer to add only real tested patches ...) >> I had to define _got_base and _relocate_got_base_r also for >> the NAND_SPL case, and don;t need to fix up the "pic base >> register", if we are at the right position. > > Thanks for spotting and fixing these. :) > >> I think we(you ;-) should add my below fix to your >> >> [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base >> >> and send a second version of it, so we are bisect compatible. >> >> you can add my >> >> Signed-off-by: Heiko Schocher<hs@denx.de> >> >> to it. > > I will -- thanks! > >> Can you test this also on your board? Thanks! > > I will too. :) Thanks! > Just one thing: are these TEXT_BASE / CONFIG_SYS_NAND_U_BOOT_DST > modifications in your patch really needed for the board to run, or are > they a manual layout optimization decided based on the code size reduction? Yes. I decided for this board to set TEXT_BASE = CONFIG_SYS_NAND_U_BOOT_DST = relocation address, so I can prevent one copying of u-boot code. (The u-boot-nand code copy u-boot to CONFIG_SYS_NAND_U_BOOT_DST, which is on the later calculated relocation address -> no need to copy it again ... if there is more RAM on this board, u-boot of course copy itself to the higher address) 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] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 10:08 ` Heiko Schocher @ 2010-09-23 12:45 ` Albert ARIBAUD 2010-09-24 5:11 ` Heiko Schocher 0 siblings, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-23 12:45 UTC (permalink / raw) To: u-boot Le 23/09/2010 12:08, Heiko Schocher a ?crit : > Hmm.. I don;t know the other boards ... so, I decided for > the relocation patches not to fix all arm boards, instead > inroduce the CONFIG_SYS_ARM_WITHOUT_RELOC, and board > maintainers (or others who have time for it) can adapt > the boards to this new code. So we always have a chance > to keep track which boards are converted *and* working > and which boards are not working ... so, I could not say, > if it is a good idea to fix other boards, you could not > test (I know, usually we do this fixes, but the relocation > code is such a deep change, I prefer to add only real > tested patches ...) Understood. If only to be consistent with your patches, I will not fix any other board than the ones I can test directly. This leads me to another question: since I cannot test the tx25, I think I should take from your fix only the parts that modify my own patch set, and leave out the parts for tx25, which you would post above my patches. Is this ok? Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-23 12:45 ` Albert ARIBAUD @ 2010-09-24 5:11 ` Heiko Schocher 2010-09-24 5:47 ` Albert ARIBAUD 0 siblings, 1 reply; 31+ messages in thread From: Heiko Schocher @ 2010-09-24 5:11 UTC (permalink / raw) To: u-boot Hello Albert, Albert ARIBAUD wrote: > Le 23/09/2010 12:08, Heiko Schocher a ?crit : > >> Hmm.. I don;t know the other boards ... so, I decided for >> the relocation patches not to fix all arm boards, instead >> inroduce the CONFIG_SYS_ARM_WITHOUT_RELOC, and board >> maintainers (or others who have time for it) can adapt >> the boards to this new code. So we always have a chance >> to keep track which boards are converted *and* working >> and which boards are not working ... so, I could not say, >> if it is a good idea to fix other boards, you could not >> test (I know, usually we do this fixes, but the relocation >> code is such a deep change, I prefer to add only real >> tested patches ...) > > Understood. If only to be consistent with your patches, I will not fix > any other board than the ones I can test directly. > > This leads me to another question: since I cannot test the tx25, I think I test it ;-) > I should take from your fix only the parts that modify my own patch set, > and leave out the parts for tx25, which you would post above my patches. > Is this ok? I prefer to have this change in one patch, because if we do it so, we don;t break "git bisect" functionallity. 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] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-24 5:11 ` Heiko Schocher @ 2010-09-24 5:47 ` Albert ARIBAUD 2010-09-24 16:45 ` Rogan Dawes 0 siblings, 1 reply; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-24 5:47 UTC (permalink / raw) To: u-boot Le 24/09/2010 07:11, Heiko Schocher a ?crit : > I prefer to have this change in one patch, because if we do it so, we > don;t break "git bisect" functionallity. Not sure I follow you there, so let's dig a little bit into that point. How exactly would that break bisectability? The only difference I see is whether tx25 building succeeds or not; however if being able to build is the criterion for bisectability, this brings us back to all other boards in the tree which will also not build any more with my patch, and whether I should fix them or not. On a side note, I do not know of a clear general definition of 'bisectability', which means I could break it yet again unkonwingly. Can you (or anyone, actually :) ) point me to a, or even the, standard definition of 'bisectable' and, if that has a specific meaning, of 'fully bisectable'? > bye, > Heiko Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-24 5:47 ` Albert ARIBAUD @ 2010-09-24 16:45 ` Rogan Dawes 2010-09-24 16:58 ` Rogan Dawes 0 siblings, 1 reply; 31+ messages in thread From: Rogan Dawes @ 2010-09-24 16:45 UTC (permalink / raw) To: u-boot On 2010/09/24 7:47 AM, Albert ARIBAUD wrote: > On a side note, I do not know of a clear general definition of > 'bisectability', which means I could break it yet again unkonwingly. Can > you (or anyone, actually :) ) point me to a, or even the, standard > definition of 'bisectable' and, if that has a specific meaning, of > 'fully bisectable'? Not sure if there is an agreed definition, but the main idea behind bisection stems from the "git bisect" tool, which allows you to mark one commit as "known good", and another as "known bad", and then calculates a commit in the middle to be tested for "goodness" or "badness". As commits are marked "good" or "bad", the tool halves (bisects) the range of commits until the single commit that introduced the breakage can be identified. Of course, for the bisection process to be able to work, all points along the path must a) compile and b) be able to be tested for the breakage. So, a fully bisectable patch series would maintain the above properties, for all boards. This is one of the reasons that Linus likes to have code used as it is introduced, rather than building up a whole lot of unused infrastructure, only to activate it with a final commit. The bisection would then point to the final commit as the culprit, when the true failure may have been introduced by one of the preceding commits. That's "as I understand it", of course. Rogan ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-24 16:45 ` Rogan Dawes @ 2010-09-24 16:58 ` Rogan Dawes 2010-09-24 17:13 ` Albert ARIBAUD 0 siblings, 1 reply; 31+ messages in thread From: Rogan Dawes @ 2010-09-24 16:58 UTC (permalink / raw) To: u-boot On 2010/09/24 6:45 PM, Rogan Dawes wrote: > On 2010/09/24 7:47 AM, Albert ARIBAUD wrote: > >> On a side note, I do not know of a clear general definition of >> 'bisectability', which means I could break it yet again unkonwingly. Can >> you (or anyone, actually :) ) point me to a, or even the, standard >> definition of 'bisectable' and, if that has a specific meaning, of >> 'fully bisectable'? Here's a decent LWN.net article on the topic: http://lwn.net/Articles/317154/ Rogan ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base 2010-09-24 16:58 ` Rogan Dawes @ 2010-09-24 17:13 ` Albert ARIBAUD 0 siblings, 0 replies; 31+ messages in thread From: Albert ARIBAUD @ 2010-09-24 17:13 UTC (permalink / raw) To: u-boot Le 24/09/2010 18:58, Rogan Dawes a ?crit : > On 2010/09/24 6:45 PM, Rogan Dawes wrote: >> On 2010/09/24 7:47 AM, Albert ARIBAUD wrote: >> >>> On a side note, I do not know of a clear general definition of >>> 'bisectability', which means I could break it yet again unkonwingly. Can >>> you (or anyone, actually :) ) point me to a, or even the, standard >>> definition of 'bisectable' and, if that has a specific meaning, of >>> 'fully bisectable'? > > Here's a decent LWN.net article on the topic: > > http://lwn.net/Articles/317154/ > > Rogan Thanks Rogan. Actually I do know the git bisect command, having had to use it to track a boot issue in recent linux kernel versions; I should have made it clear that I was looking specifically for the meaning of 'bisectable' -- which you have clarified, thanks again. So in this specific case, I guess what I should do is look up Heiko's patch series, see which arm926 boards it makes relocation-capable, and apply the .lds fx to those. The other arm926 boards, which were left untouched by Heiko, would not build with his patches anyway, so there is no real point in fixing them yet. Or is there? So unless some strong disagreement is voiced, I'll prepare a V3 patch set which will be V2 + possible fixes stemming from Heiko's and Ben's test feedback + fixes to the .lds of boards which are made relocation capable in Heiko's patch series. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE 2010-09-22 13:57 [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Albert Aribaud 2010-09-22 13:57 ` [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base Albert Aribaud @ 2010-09-22 18:05 ` Ben Gardiner 1 sibling, 0 replies; 31+ messages in thread From: Ben Gardiner @ 2010-09-22 18:05 UTC (permalink / raw) To: u-boot On Wed, Sep 22, 2010 at 9:57 AM, Albert Aribaud <albert.aribaud@free.fr> wrote: > Replace GOT indirect addressing with more efficient pic-base > relative addressing for initialized data (uninitialized data > still use GOTi indirect addressing). ?This also reduces code > size by 0.4% compared to -fPIC. > > Signed-off-by: Albert Aribaud <albert.aribaud@free.fr> Applies clean to u-boot/next. Tested on da850evm; u-boot loads and gives the "DA850-evm >" prompt. Tested-by: Ben Gardiner <bengardiner@nanometrics.ca> Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2010-09-24 17:13 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-22 13:57 [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Albert Aribaud 2010-09-22 13:57 ` [U-Boot] [PATCH 2/2] [NEXT] arm926ejs: reduce code size with -msingle-pic-base Albert Aribaud 2010-09-22 18:05 ` Ben Gardiner 2010-09-22 19:07 ` Albert ARIBAUD 2010-09-22 20:51 ` Ben Gardiner 2010-09-22 21:36 ` Albert ARIBAUD 2010-09-22 22:07 ` Albert ARIBAUD 2010-09-23 14:44 ` Ben Gardiner 2010-09-23 15:13 ` Albert ARIBAUD 2010-09-23 15:35 ` Ben Gardiner 2010-09-23 16:53 ` Ben Gardiner 2010-09-23 16:37 ` Ben Gardiner 2010-09-23 17:04 ` Albert ARIBAUD 2010-09-23 21:13 ` Ben Gardiner 2010-09-23 21:30 ` Albert ARIBAUD 2010-09-24 13:38 ` Ben Gardiner 2010-09-24 16:08 ` Albert ARIBAUD 2010-09-22 20:30 ` Wolfgang Denk 2010-09-22 20:55 ` Ben Gardiner 2010-09-22 21:11 ` Wolfgang Denk 2010-09-22 21:33 ` Ben Gardiner 2010-09-23 7:12 ` Heiko Schocher 2010-09-23 8:05 ` Albert ARIBAUD 2010-09-23 10:08 ` Heiko Schocher 2010-09-23 12:45 ` Albert ARIBAUD 2010-09-24 5:11 ` Heiko Schocher 2010-09-24 5:47 ` Albert ARIBAUD 2010-09-24 16:45 ` Rogan Dawes 2010-09-24 16:58 ` Rogan Dawes 2010-09-24 17:13 ` Albert ARIBAUD 2010-09-22 18:05 ` [U-Boot] [PATCH 1/2] [NEXT] arm: change relocation flag from -fPIC to -fPIE Ben Gardiner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox