Optimize ARM relocation

PATCH New Albert ARIBAUD 4 participants 46 messages May 14, 2013 – Jun 21, 2013

Subsystems (4)

Arm (32-bit) Arm i.MX and Layerscape Kbuild Generic include/asm header files

lore

Share & tools

lore

Albert ARIBAUD

author

2013-05-14 20:02 UTC #0 lore raw

cover Optimize ARM relocation

*** NOTE: this series applies over the 'Factorize ARM relocate_code instances' series.

This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.

The only known case where relocation records other than R_ARM_RELATIVE are generated is when a reference is made to a symbol defined in the linker script, e.g. __image_copy_start, __image_copy_end, __rel_dyn_start, __rel_dyn_end, and __dynsym_start.

Moving the definition of these symbols from the linker scripts into a C module causes their references' types to become R_ARM_RELATIVE.

First, arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.

Second, __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c

Third, __rel_dyn_start, __rel_dyn_end and __synsym_start are moved from linker scripts into arch/arm/lib/sections.c

Fourth, a check is added to the build system to ensure that ELF U-Boot binaries only use R_ARM_RELATIVE records.

Last, relocate_code is optimized

Albert ARIBAUD (5):
arm: generalize lib/bss.c into lib/sections.c
arm: make __image_copy_{start,end} compiler-generated
arm: make relocation symbols compiler-generated
arm: ensure u-boot only uses relative relocations
arm: optimize relocate_code routine

 Makefile                               |    7 ++++
 arch/arm/config.mk                     |    5 +++
 arch/arm/cpu/arm920t/ep93xx/u-boot.lds |    6 ++-
 arch/arm/cpu/ixp/u-boot.lds            |   24 ++++++++++--
 arch/arm/cpu/u-boot.lds                |   25 ++++++++++---
 arch/arm/lib/Makefile                  |    2 +-
 arch/arm/lib/relocate.S                |   64 +++++++++-----------------------
 arch/arm/lib/{bss.c => sections.c}     |    9 ++++-
 board/actux1/u-boot.lds                |   24 ++++++++++--
 board/actux2/u-boot.lds                |   24 ++++++++++--
 board/actux3/u-boot.lds                |   24 ++++++++++--
 board/dvlhost/u-boot.lds               |   24 ++++++++++--
 board/freescale/mx31ads/u-boot.lds     |   24 ++++++++++--
 13 files changed, 183 insertions(+), 79 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (77%)

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-14 20:02 UTC #1 lore raw

PATCH 1/5 arm: generalize lib/bss.c into lib/sections.c

File arch/arm/lib/bss.c was initially defined for BSS only, but is now going to also contain definitions for other section-boundary-related symbols, so rename it for better accuracy.

Also, remove useless 'used' attributes.

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/lib/Makefile              |    2 +-
 arch/arm/lib/{bss.c => sections.c} |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (92%)

arch/arm/lib/Makefile +1 -1

index 30e3290..dbad5c1 100644
@@ -43,7 +43,7 @@ SOBJS-y += relocate.o
 ifndef CONFIG_SYS_GENERIC_BOARD
 COBJS-y	+= board.o
 endif
-COBJS-y += bss.o
+COBJS-y += sections.o
 
 COBJS-y	+= bootm.o
 COBJS-$(CONFIG_SYS_L2_PL310) += cache-pl310.o

arch/arm/lib/sections.c renamed from arch/arm/lib/bss.c +2 -2

similarity index 92%
rename from arch/arm/lib/bss.c
rename to arch/arm/lib/sections.c
index 99eda59..e52fec9 100644
@@ -35,5 +35,5 @@
  * aliasing warnings.
  */
 
-char __bss_start[0] __attribute__((used, section(".__bss_start")));
-char __bss_end[0] __attribute__((used, section(".__bss_end")));
+char __bss_start[0] __attribute__((section(".__bss_start")));
+char __bss_end[0] __attribute__((section(".__bss_end")));

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-14 20:02 UTC #2 lore raw

PATCH 2/5 arm: make __image_copy_{start, end} compiler-generated

This change is only done where needed: some linker scripts may contain __image_copy_{start,end} yet remain unchanged.

Also, __image_copy_end needs its own section; putting it in relocation sections changes the sections's nature which breaks relocation.

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/cpu/arm920t/ep93xx/u-boot.lds |    6 +++++-
 arch/arm/cpu/ixp/u-boot.lds            |    6 +++++-
 arch/arm/cpu/u-boot.lds                |    7 +++++--
 arch/arm/lib/relocate.S                |    7 ++-----
 arch/arm/lib/sections.c                |    2 ++
 board/actux1/u-boot.lds                |    6 +++++-
 board/actux2/u-boot.lds                |    6 +++++-
 board/actux3/u-boot.lds                |    6 +++++-
 board/dvlhost/u-boot.lds               |    6 +++++-
 board/freescale/mx31ads/u-boot.lds     |    6 +++++-
 10 files changed, 44 insertions(+), 14 deletions(-)

arch/arm/cpu/arm920t/ep93xx/u-boot.lds +5 -1

index cf55bf7..367c805 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text      :
 	{
+		*(.__image_copy_start)
 	  arch/arm/cpu/arm920t/start.o	(.text*)
 		/* the EP93xx expects to find the pattern 'CRUS' at 0x1000 */
 	  . = 0x1000;

@@ -56,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	__bss_start = .;
 	.bss : { *(.bss*) }

arch/arm/cpu/ixp/u-boot.lds +5 -1

index 553589c..514c6ec 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		*(.text*)
 	}

@@ -54,7 +55,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/cpu/u-boot.lds +5 -2

index d9bbee3..8950f5a 100644
@@ -33,7 +33,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
-		__image_copy_start = .;
+		*(.__image_copy_start)
 		CPUDIR/start.o (.text*)
 		*(.text*)
 	}

@@ -57,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/lib/relocate.S +2 -5

index 9e91fae..2f22c8c 100644
@@ -39,12 +39,11 @@
 ENTRY(relocate_code)
 	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =_start
+	ldr	r0, =__image_copy_start	/* r0 <- source start address */
 	subs	r9, r6, r0		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
 	mov	r1, r6			/* r1 <- scratch for copy loop */
-	ldr	r3, _image_copy_end_ofs
-	add	r2, r0, r3		/* r2 <- source end address	    */
+	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
 
 copy_loop:
 	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */

@@ -99,8 +98,6 @@ relocate_done:
         bx        lr
 #endif
 
-_image_copy_end_ofs:
-	.word __image_copy_end - relocate_code
 _rel_dyn_start_ofs:
 	.word __rel_dyn_start - relocate_code
 _rel_dyn_end_ofs:

arch/arm/lib/sections.c +2 -0

index e52fec9..03e846f 100644
@@ -37,3 +37,5 @@
 
 char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
+char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
+char __image_copy_end[0] __attribute__((section(".__image_copy_end")));

board/actux1/u-boot.lds +5 -1

index ef4a25b..80db9ff 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux1/libactux1.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux2/u-boot.lds +5 -1

index 00ad8b7..61f2e61 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux2/libactux2.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux3/u-boot.lds +5 -1

index 44b990e..c1ad8b5 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux3/libactux3.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/dvlhost/u-boot.lds +5 -1

index 6d4b187..71275c1 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/dvlhost/libdvlhost.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/freescale/mx31ads/u-boot.lds +5 -1

index 4969960..acb8244 100644
@@ -34,6 +34,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text	   :
 	{
+		*(.__image_copy_start)
 	  /* WARNING - the following is hand-optimized to fit within	*/
 	  /* the sector layout of our flash chips!	XXX FIXME XXX	*/

@@ -65,7 +66,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-14 20:02 UTC #3 lore raw

PATCH 3/5 arm: make relocation symbols compiler-generated

This change is only done where needed: some linker scripts may contain relocation symbols yet remain unchanged.

__rel_dyn_start, __rel_dyn_end and __dynsym_start each requires its own output section; putting them in relocation sections changes the sections' nature which breaks relocation.

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/cpu/ixp/u-boot.lds        |   18 +++++++++++++++---
 arch/arm/cpu/u-boot.lds            |   18 +++++++++++++++---
 arch/arm/lib/relocate.S            |   16 +++-------------
 arch/arm/lib/sections.c            |    3 +++
 board/actux1/u-boot.lds            |   18 +++++++++++++++---
 board/actux2/u-boot.lds            |   18 +++++++++++++++---
 board/actux3/u-boot.lds            |   18 +++++++++++++++---
 board/dvlhost/u-boot.lds           |   18 +++++++++++++++---
 board/freescale/mx31ads/u-boot.lds |   18 +++++++++++++++---
 9 files changed, 111 insertions(+), 34 deletions(-)

arch/arm/cpu/ixp/u-boot.lds +15 -3

index 514c6ec..933928a 100644
@@ -60,14 +60,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

arch/arm/cpu/u-boot.lds +15 -3

index 8950f5a..4ba2c38 100644
@@ -62,14 +62,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

arch/arm/lib/relocate.S +3 ~~-13~~

index 2f22c8c..818735c 100644
@@ -54,12 +54,9 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table ofs */
-	add	r10, r10, r9		/* r10 <- sym table in FLASH */
-	ldr	r2, _rel_dyn_start_ofs	/* r2 <- rel dyn start ofs */
-	add	r2, r2, r9		/* r2 <- rel dyn start in FLASH */
-	ldr	r3, _rel_dyn_end_ofs	/* r3 <- rel dyn end ofs */
-	add	r3, r3, r9		/* r3 <- rel dyn end in FLASH */
+	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
+	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
+	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
 fixloop:
 	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
 	add	r0, r0, r9		/* r0 <- DST location to fix up */

@@ -98,11 +95,4 @@ relocate_done:
         bx        lr
 #endif
 
-_rel_dyn_start_ofs:
-	.word __rel_dyn_start - relocate_code
-_rel_dyn_end_ofs:
-	.word __rel_dyn_end - relocate_code
-_dynsym_start_ofs:
-	.word __dynsym_start - relocate_code
-
 ENDPROC(relocate_code)

arch/arm/lib/sections.c +3 -0

index 03e846f..d065942 100644
@@ -39,3 +39,6 @@ char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
 char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
 char __image_copy_end[0] __attribute__((section(".__image_copy_end")));
+char __rel_dyn_start[0] __attribute__((section(".__rel_dyn_start")));
+char __rel_dyn_end[0] __attribute__((section(".__rel_dyn_end")));
+char __dynsym_start[0] __attribute__((section(".__dynsym_start")));

board/actux1/u-boot.lds +15 -3

index 80db9ff..f9b8c54 100644
@@ -68,14 +68,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

board/actux2/u-boot.lds +15 -3

index 61f2e61..3929694 100644
@@ -68,14 +68,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

board/actux3/u-boot.lds +15 -3

index c1ad8b5..5070354 100644
@@ -68,14 +68,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

board/dvlhost/u-boot.lds +15 -3

index 71275c1..bb5fa58 100644
@@ -68,14 +68,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

board/freescale/mx31ads/u-boot.lds +15 -3

index acb8244..63b5108 100644
@@ -71,14 +71,26 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
+	}
+
+	.dynsym_start :
+	{
+		*(.__dynsym_start)
 	}
 
 	.dynsym : {
-		__dynsym_start = .;
 		*(.dynsym)
 	}

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-14 20:02 UTC #4 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

Add a Makefile target ('checkarmreloc') which fails of the ELF binary contains relocation records of types other than R_ARM_RELATIVE.

Signed-off-by Albert ARIBAUD <[email protected]>

---
 Makefile           |    7 +++++++
 arch/arm/config.mk |    5 +++++
 2 files changed, 12 insertions(+)

Makefile +7 -0

index c52f0f1..9aa5755 100644
@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
+		then echo "$(obj)u-boot contains relocations other than \
+		R_ARM_RELATIVE"; false; fi
+
 $(VERSION_FILE):
 		@mkdir -p $(dir $(VERSION_FILE))
 		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \

arch/arm/config.mk +5 -0

index 461899e..5b7a602 100644
@@ -106,3 +106,8 @@ ifeq ($(GAS_BUG_12532),y)
 PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
 endif
 endif
+
+# check that only R_ARM_RELATIVE relocations are generated
+ifneq ($(CONFIG_SPL_BUILD),y)
+ALL-y	+= checkarmreloc
+endif

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-14 20:03 UTC #5 lore raw

PATCH 5/5 arm: optimize relocate_code routine

Use section symbols directly
Drop support for R_ARM_ABS32 record types Eliminate unneeded intermediate registers Optimize relocation table iteration

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

arch/arm/lib/relocate.S +15 ~~-30~~

index 818735c..75ee3b4 100644
@@ -37,51 +37,36 @@
  * This function relocates the monitor code.
  */
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =__image_copy_start	/* r0 <- source start address */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- source start address */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
 	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
 	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
 	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff		/* r1 <- fixup type */
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
-	ldr	r1, [r0]
-	add	r1, r1, r9
+	add	r0, r0, r9		/* r0 <- DST location to fix up */
+	ldr	r1, [r0]		/* r1 <- content to fix up */
+	add	r1, r1, r9		/* fix up */
+	str	r1, [r0]		/* write back fixed-up content */
+
 fixnext:
-	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop

Signature · 1 line

1.7.10.4

Benoît Thébaudeau

2013-05-14 23:54 UTC #6 lore raw

PATCH 5/5 arm: optimize relocate_code routine

Hi Albert,

Quoted 70 lines · Albert ARIBAUD

Use section symbols directly
Drop support for R_ARM_ABS32 record types Eliminate unneeded intermediate registers Optimize relocation table iteration

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

arch/arm/lib/relocate.S +10 ~~-28~~

index 818735c..75ee3b4 100644
@@ -37,51 +37,36 @@
  * This function relocates the monitor code.
  */
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =__image_copy_start	/* r0 <- source start address */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- source start address */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
 	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
 	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
 	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff		/* r1 <- fixup type */
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
-	ldr	r1, [r0]
-	add	r1, r1, r9

arch/arm/lib/relocate.S line 71

Quoted 10 lines

arch/arm/lib/relocate.S +5 -2

+	add	r0, r0, r9		/* r0 <- DST location to fix up */
+	ldr	r1, [r0]		/* r1 <- content to fix up */
+	add	r1, r1, r9		/* fix up */
+	str	r1, [r0]		/* write back fixed-up content */
+
 fixnext:
-	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop

The final state of relocate.S is correct in this series, but it is not correct at the end of "[PATCH v2 4/4] arm: factorize relocate_code routine" as I pointed out in the part of my review that you cut in your first reply.

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-15 07:32 UTC #7 lore raw

PATCH 5/5 arm: optimize relocate_code routine

Hi Beno?t,

Quoted 79 lines · Beno?t Th?baudeau

Hi Albert,

Quoted 76 lines · Albert ARIBAUD

Use section symbols directly
Drop support for R_ARM_ABS32 record types Eliminate unneeded intermediate registers Optimize relocation table iteration

Signed-off-by Albert ARIBAUD <[email protected]>

---
 arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

arch/arm/lib/relocate.S +15 ~~-28~~

index 818735c..75ee3b4 100644
@@ -37,51 +37,36 @@
  * This function relocates the monitor code.
  */
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =__image_copy_start	/* r0 <- source start address */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- source start address */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
 	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
 	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
 	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff		/* r1 <- fixup type */
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
-	ldr	r1, [r0]
-	add	r1, r1, r9
+	add	r0, r0, r9		/* r0 <- DST location to fix up */
+	ldr	r1, [r0]		/* r1 <- content to fix up */
+	add	r1, r1, r9		/* fix up */
+	str	r1, [r0]		/* write back fixed-up content */
+
 fixnext:

Quoted 8 lines

arch/arm/lib/relocate.S line 71
Quoted 4 lines
arch/arm/lib/relocate.S +0 -2
-	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop
The final state of relocate.S is correct in this series, but it is not correct at the end of "[PATCH v2 4/4] arm: factorize relocate_code routine" as I pointed out in the part of my review that you cut in your first reply.

Error is relocate patch series to be fixed in V2; I'll rebase V2 of this series accordingly.

Thanks again.

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

2013-05-14 22:12 UTC #8 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

In reply to Albert ARIBAUD

Hi Albert,

Quoted 15 lines · Albert ARIBAUD

Add a Makefile target ('checkarmreloc') which fails of the ELF binary contains relocation records of types other than R_ARM_RELATIVE.
Signed-off-by Albert ARIBAUD <[email protected]>
---
 Makefile           |    7 +++++++
 arch/arm/config.mk |    5 +++++
 2 files changed, 12 insertions(+)
Makefile +0 -0
index c52f0f1..9aa5755 100644

Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 752

Quoted 7 lines

Makefile +4 -0

@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \

^
or $$< to avoid a duplicate?

Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 753

Quoted 1 line

Makefile +1 -0

+		then echo "$(obj)u-boot contains relocations other than \

                           ^
                           or $$< too, or no $(obj) prefix@all for this message?

Quoted 19 lines

Makefile +2 -0

+		R_ARM_RELATIVE"; false; fi
+
 $(VERSION_FILE):
 		@mkdir -p $(dir $(VERSION_FILE))
 		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \

arch/arm/config.mk +5 -1

index 461899e..5b7a602 100644
@@ -106,3 +106,8 @@ ifeq ($(GAS_BUG_12532),y)
 PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
 endif
 endif
+
+# check that only R_ARM_RELATIVE relocations are generated
+ifneq ($(CONFIG_SPL_BUILD),y)
+ALL-y	+= checkarmreloc
+endif
--

Quoted 1 line

1.7.10.4

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-15 07:46 UTC #9 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

Hi Beno?t,

Quoted 8 lines · Beno?t Th?baudeau

Hi Albert,

Quoted 6 lines

Makefile +0 -0

@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk

Quoted 6 lines

Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 752
Quoted 4 lines
Makefile +4 -0
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
^
or $$< to avoid a duplicate?

Will fix as suggested.

Quoted 3 lines

Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 753

Quoted 1 line

Makefile +1 -0

+		then echo "$(obj)u-boot contains relocations other than \

                           ^
                           or $$< too, or no $(obj) prefix@all for this message?

I prefer leaving the prefix so that failures during out-of-tree builds or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.

Quoted 2 lines

Best regards,
Beno?t

Thanks!

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-05-15 09:38 UTC #10 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

Hi again Beno?t,

Quoted 20 lines · Albert ARIBAUD

Hi Beno?t,
Quoted 8 lines · Beno?t Th?baudeau
Hi Albert,
Quoted 6 lines
Makefile +0 -0
@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
Quoted 6 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 752
Quoted 4 lines
Makefile +4 -0
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
^
or $$< to avoid a duplicate?

Quoted 8 lines

Will fix as suggested.
Quoted 3 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 753
Quoted 1 line
Makefile +1 -0
+		then echo "$(obj)u-boot contains relocations other than \
                           ^
                           or $$< too, or no $(obj) prefix@all for this message?
I prefer leaving the prefix so that failures during out-of-tree builds or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.

Actually $$< does not work within backquotes unless escaped as a less legible \$\$<, and does not work properly@all within double quotes, whether escaped or not.

Do you prefer that I change only the first $(obj)u-boot into \$\$< and leave the second one untouched, or that I leave both $(obj)u-boot instances as-is for the sake of homogeneity?

Quoted 2 lines

Quoted 2 lines
Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

2013-05-15 13:49 UTC #11 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

Hi Albert,

Quoted 36 lines · Albert ARIBAUD

Hi again Beno?t,
Quoted 23 lines · Albert ARIBAUD
Hi Beno?t,
Quoted 8 lines · Beno?t Th?baudeau
Hi Albert,
Quoted 6 lines
Makefile +0 -0
@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
Quoted 7 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
Quoted 5 lines
Makefile +4 -1
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort
-u`"; \
^
or $$< to avoid a duplicate?
Will fix as suggested.
Quoted 7 lines
Quoted 4 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 753
Quoted 1 line
Makefile +1 -0
+		then echo "$(obj)u-boot contains relocations other than \
                           ^
                           or $$< too, or no $(obj) prefix@all for
                           this message?
I prefer leaving the prefix so that failures during out-of-tree builds or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.

Quoted 7 lines

Actually $$< does not work within backquotes unless escaped as a less legible \$\$<, and does not work properly@all within double quotes, whether escaped or not.
Do you prefer that I change only the first $(obj)u-boot into \$\$< and leave the second one untouched, or that I leave both $(obj)u-boot instances as-is for the sake of homogeneity?

Actually, a single dollar sign (i.e. "$<") would be needed since it must have been expanded by make before reaching the shell, and no shell backslash escape sequences should be required.

If this still does not pass smoothly, then I prefer simplicity and homogeneity.

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-15 15:01 UTC #12 lore raw

PATCH 4/5 arm: ensure u-boot only uses relative relocations

Hi Beno?t,

Quoted 47 lines · Beno?t Th?baudeau

Hi Albert,
Quoted 36 lines · Albert ARIBAUD
Hi again Beno?t,
Quoted 23 lines · Albert ARIBAUD
Hi Beno?t,
Quoted 8 lines · Beno?t Th?baudeau
Hi Albert,
Quoted 6 lines
Makefile +0 -0
@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
Quoted 7 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
Quoted 5 lines
Makefile +4 -1
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort
-u`"; \
^
or $$< to avoid a duplicate?
Will fix as suggested.
Quoted 7 lines
Quoted 4 lines
Makefile tools: $(VERSION_FILE) $(TIMESTAMP_FILE) · line 753
Quoted 1 line
Makefile +1 -0
+		then echo "$(obj)u-boot contains relocations other than \
                           ^
                           or $$< too, or no $(obj) prefix@all for
                           this message?
I prefer leaving the prefix so that failures during out-of-tree builds or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.
Quoted 7 lines
Actually $$< does not work within backquotes unless escaped as a less legible \$\$<, and does not work properly@all within double quotes, whether escaped or not.
Do you prefer that I change only the first $(obj)u-boot into \$\$< and leave the second one untouched, or that I leave both $(obj)u-boot instances as-is for the sake of homogeneity?

Quoted 5 lines

Actually, a single dollar sign (i.e. "$<") would be needed since it must have been expanded by make before reaching the shell, and no shell backslash escape sequences should be required.
If this still does not pass smoothly, then I prefer simplicity and homogeneity.

Single unescaped $< works like a charm within backward as well as double quotes, thanks!

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

2013-05-14 22:09 UTC #13 lore raw

PATCH 3/5 arm: make relocation symbols compiler-generated

In reply to Albert ARIBAUD

Hi Albert,

Quoted 10 lines · Albert ARIBAUD

This change is only done where needed: some linker scripts may contain relocation symbols yet remain unchanged.
__rel_dyn_start, __rel_dyn_end and __dynsym_start each requires its own output section; putting them in relocation sections changes the sections' nature which breaks relocation.
Signed-off-by Albert ARIBAUD <[email protected]>

[...]

Quoted 7 lines

arch/arm/lib/relocate.S +0 -0

index 2f22c8c..818735c 100644
@@ -54,12 +54,9 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations

arch/arm/lib/relocate.S copy_loop: · line 59

Quoted 10 lines

arch/arm/lib/relocate.S +3 -6

 	 */
-	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table ofs */
-	add	r10, r10, r9		/* r10 <- sym table in FLASH */
-	ldr	r2, _rel_dyn_start_ofs	/* r2 <- rel dyn start ofs */
-	add	r2, r2, r9		/* r2 <- rel dyn start in FLASH */
-	ldr	r3, _rel_dyn_end_ofs	/* r3 <- rel dyn end ofs */
-	add	r3, r3, r9		/* r3 <- rel dyn end in FLASH */
+	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
+	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
+	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */

'ofs' -> 'in FLASH' in the comments of the 3 lines above.

Quoted 5 lines

arch/arm/lib/relocate.S +0 -0

 fixloop:
 	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
 	add	r0, r0, r9		/* r0 <- DST location to fix up */

@@ -98,11 +95,4 @@ relocate_done:
         bx        lr

arch/arm/lib/relocate.S relocate_done: · line 98

Quoted 10 lines

arch/arm/lib/relocate.S +0 -7

 #endif
 
-_rel_dyn_start_ofs:
-	.word __rel_dyn_start - relocate_code
-_rel_dyn_end_ofs:
-	.word __rel_dyn_end - relocate_code
-_dynsym_start_ofs:
-	.word __dynsym_start - relocate_code
-
 ENDPROC(relocate_code)

[...]

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-15 06:39 UTC #14 lore raw

PATCH 3/5 arm: make relocation symbols compiler-generated

Hi Beno?t,

Quoted 27 lines · Beno?t Th?baudeau

Hi Albert,
Quoted 10 lines · Albert ARIBAUD
This change is only done where needed: some linker scripts may contain relocation symbols yet remain unchanged.
__rel_dyn_start, __rel_dyn_end and __dynsym_start each requires its own output section; putting them in relocation sections changes the sections' nature which breaks relocation.
Signed-off-by Albert ARIBAUD <[email protected]>
[...]
Quoted 11 lines
arch/arm/lib/relocate.S +0 -3
index 2f22c8c..818735c 100644
@@ -54,12 +54,9 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table ofs */
-	add	r10, r10, r9		/* r10 <- sym table in FLASH */
-	ldr	r2, _rel_dyn_start_ofs	/* r2 <- rel dyn start ofs */

Quoted 8 lines

arch/arm/lib/relocate.S copy_loop: · line 59

Quoted 6 lines

arch/arm/lib/relocate.S +3 -3

-	add	r2, r2, r9		/* r2 <- rel dyn start in FLASH */
-	ldr	r3, _rel_dyn_end_ofs	/* r3 <- rel dyn end ofs */
-	add	r3, r3, r9		/* r3 <- rel dyn end in FLASH */
+	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
+	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
+	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */

'ofs' -> 'in FLASH' in the comments of the 3 lines above.

You are right that 'ofs' should have disappeared from the comments; but 'in FLASH' is right only for some U-Boots and wrong for others, as some U-Boots indeed run their board_init_f phase in FLASH, but others run in DDR and thus, copy themselves to DDR from DDR, not FLASH, notably those with an SPL.

So the most correct commenting is using 'SRC' and 'DST'; will fix in V3 with 'sym table SRC addr' etc.

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-05-15 06:41 UTC #15 lore raw

PATCH 3/5 arm: make relocation symbols compiler-generated

Quoted 1 line · Albert ARIBAUD

[...] will fix in V3

Make that V2.

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-05-14 20:16 UTC #16 lore raw

cover Optimize ARM relocation

In reply to Albert ARIBAUD

Quoted 25 lines · Albert ARIBAUD

*** NOTE: this series applies over the 'Factorize ARM relocate_code instances' series.
This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.
The only known case where relocation records other than R_ARM_RELATIVE are generated is when a reference is made to a symbol defined in the linker script, e.g. __image_copy_start, __image_copy_end, __rel_dyn_start, __rel_dyn_end, and __dynsym_start.
Moving the definition of these symbols from the linker scripts into a C module causes their references' types to become R_ARM_RELATIVE.
First, arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.
Second, __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c

Quoted 7 lines

Third, __rel_dyn_start, __rel_dyn_end and __synsym_start are moved from linker scripts into arch/arm/lib/sections.c
Fourth, a check is added to the build system to ensure that ELF U-Boot binaries only use R_ARM_RELATIVE records.
Last, relocate_code is optimized

Hmm, it seems I need to see my work posted on the list to discover how I could have done it better... I just noticed that if ARM binaries only have R_ARM_RELATIVE record types, then all the .dynsym sections can actually be dropped from the binaries. So, if I reorder the series and put patch 5/5 first, then I can eliminate patch 3/5, or more to the point, replace it with one which eliminates all dynsym stuff from linker scripts, further reducing code size. That'll go in V2...

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

2013-05-14 23:58 UTC #17 lore raw

cover Optimize ARM relocation

Hi Albert,

Quoted 36 lines · Albert ARIBAUD

Quoted 25 lines · Albert ARIBAUD
*** NOTE: this series applies over the 'Factorize ARM relocate_code instances' series.
This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.
The only known case where relocation records other than R_ARM_RELATIVE are generated is when a reference is made to a symbol defined in the linker script, e.g. __image_copy_start, __image_copy_end, __rel_dyn_start, __rel_dyn_end, and __dynsym_start.
Moving the definition of these symbols from the linker scripts into a C module causes their references' types to become R_ARM_RELATIVE.
First, arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.
Second, __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c
Quoted 7 lines
Third, __rel_dyn_start, __rel_dyn_end and __synsym_start are moved from linker scripts into arch/arm/lib/sections.c
Fourth, a check is added to the build system to ensure that ELF U-Boot binaries only use R_ARM_RELATIVE records.
Last, relocate_code is optimized

Quoted 7 lines

Hmm, it seems I need to see my work posted on the list to discover how I could have done it better... I just noticed that if ARM binaries only have R_ARM_RELATIVE record types, then all the .dynsym sections can actually be dropped from the binaries. So, if I reorder the series and put patch 5/5 first, then I can eliminate patch 3/5, or more to the point, replace it with one which eliminates all dynsym stuff from linker scripts, further reducing code size. That'll go in V2...

Indeed.

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-15 07:49 UTC #18 lore raw

cover Optimize ARM relocation

Hi Beno?t,

Quoted 47 lines · Beno?t Th?baudeau

Hi Albert,
Quoted 36 lines · Albert ARIBAUD
Quoted 25 lines · Albert ARIBAUD
*** NOTE: this series applies over the 'Factorize ARM relocate_code instances' series.
This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.
The only known case where relocation records other than R_ARM_RELATIVE are generated is when a reference is made to a symbol defined in the linker script, e.g. __image_copy_start, __image_copy_end, __rel_dyn_start, __rel_dyn_end, and __dynsym_start.
Moving the definition of these symbols from the linker scripts into a C module causes their references' types to become R_ARM_RELATIVE.
First, arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.
Second, __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c
Quoted 7 lines
Third, __rel_dyn_start, __rel_dyn_end and __synsym_start are moved from linker scripts into arch/arm/lib/sections.c
Fourth, a check is added to the build system to ensure that ELF U-Boot binaries only use R_ARM_RELATIVE records.
Last, relocate_code is optimized
Quoted 7 lines
Hmm, it seems I need to see my work posted on the list to discover how I could have done it better... I just noticed that if ARM binaries only have R_ARM_RELATIVE record types, then all the .dynsym sections can actually be dropped from the binaries. So, if I reorder the series and put patch 5/5 first, then I can eliminate patch 3/5, or more to the point, replace it with one which eliminates all dynsym stuff from linker scripts, further reducing code size. That'll go in V2...

Quoted 1 line

Indeed.

Correction: if I out patch 4/5, not 5/5 -- 4/5 is the patch that ensures only relative relocations are present, and 5/5 depends on previous patches in the series (at least 1/5 and 2/5).

So the new order will be 4/5, 1/5, 2/5, amended 3/5, 5/5.

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-05-28 07:01 UTC #19 lore raw

v2 cover Optimize ARM relocation

In reply to Albert ARIBAUD

*** NOTE: this series applies over the 'Factorize ARM relocate_code instances' series.

This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.

1. A Makefile rule is added that checks that no other relocation record types are generated except R_ARM_RELATIVE; build fails if this is the case.

2. All references to dymsym are removed, as this table is not used for R_ARM_RELATIVE relocations.

3. arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.

4. __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c

5. __rel_dyn_start and __rel_dyn_end are moved from linker scripts into arch/arm/lib/sections.c

6. relocate_code is optimized based on the fact that symbol references are now always valid even before relcation, and that only R_ARM_RELATIVE relocations will be met.

Changes in v2:
- use $< instead of $(obj)u-boot
- new in V2: remove all dynsym references
- various comment fixes

Albert ARIBAUD (6):
arm: ensure u-boot only uses relative relocations remove all references to .dynsym
arm: generalize lib/bss.c into lib/sections.c
arm: make __image_copy_{start,end} compiler-generated
arm: make __rel_dyn_{start,end} compiler-generated
arm: optimize relocate_code routine

 Makefile                                       |    7 +++
 arch/arm/config.mk                             |    5 ++
 arch/arm/cpu/arm920t/ep93xx/u-boot.lds         |    6 ++-
 arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds      |    5 --
 arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds    |    5 --
 arch/arm/cpu/ixp/u-boot.lds                    |   20 +++++---
 arch/arm/cpu/u-boot-spl.lds                    |    6 +--
 arch/arm/cpu/u-boot.lds                        |   21 +++++---
 arch/arm/lib/Makefile                          |    2 +-
 arch/arm/lib/relocate.S                        |   61 ++++++------------------
 arch/arm/lib/{bss.c => sections.c}             |    8 +++-
 board/actux1/u-boot.lds                        |   20 +++++---
 board/actux2/u-boot.lds                        |   20 +++++---
 board/actux3/u-boot.lds                        |   20 +++++---
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    5 --
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    5 --
 board/davinci/da8xxevm/u-boot-spl-hawk.lds     |    1 -
 board/dvlhost/u-boot.lds                       |   20 +++++---
 board/freescale/mx31ads/u-boot.lds             |   20 +++++---
 board/vpac270/u-boot-spl.lds                   |    6 +--
 include/asm-generic/sections.h                 |    3 --
 21 files changed, 139 insertions(+), 127 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (79%)

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #20 lore raw

PATCH v2 1/6 arm: ensure u-boot only uses relative relocations

Add a Makefile target ('checkarmreloc') which fails of the ELF binary contains relocation records of types other than R_ARM_RELATIVE.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2:
- use $< instead of $(obj)u-boot

 Makefile           |    7 +++++++
 arch/arm/config.mk |    5 +++++
 2 files changed, 12 insertions(+)

Makefile +7 -0

index c52f0f1..70b5183 100644
@@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $< | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
+		then echo "$< contains relocations other than \
+		R_ARM_RELATIVE"; false; fi
+
 $(VERSION_FILE):
 		@mkdir -p $(dir $(VERSION_FILE))
 		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \

arch/arm/config.mk +5 -0

index dc64160..e80e1ed 100644
@@ -109,3 +109,8 @@ ifeq ($(GAS_BUG_12532),y)
 PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
 endif
 endif
+
+# check that only R_ARM_RELATIVE relocations are generated
+ifneq ($(CONFIG_SPL_BUILD),y)
+ALL-y	+= checkarmreloc
+endif

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #21 lore raw

PATCH v2 2/6 remove all references to .dynsym

Discard all .dynsym sections from linker scripts Remove all __dynsym_start definitions from linker scripts Remove all __dynsym_start references from the codebase

Note: this touches include/asm-generic/sections.h, which is not ARM-specific, but actual uses of __dynsym_start are only in ARM, so this patch can safely go through the ARM repository.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2:
- new in V2: remove all dynsym references

 arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds      |    5 -----
 arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds    |    5 -----
 arch/arm/cpu/ixp/u-boot.lds                    |    6 +-----
 arch/arm/cpu/u-boot-spl.lds                    |    6 +-----
 arch/arm/cpu/u-boot.lds                        |    6 +-----
 arch/arm/lib/relocate.S                        |   13 -------------
 board/actux1/u-boot.lds                        |    6 +-----
 board/actux2/u-boot.lds                        |    6 +-----
 board/actux3/u-boot.lds                        |    6 +-----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    5 -----
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    5 -----
 board/davinci/da8xxevm/u-boot-spl-hawk.lds     |    1 -
 board/dvlhost/u-boot.lds                       |    6 +-----
 board/freescale/mx31ads/u-boot.lds             |    6 +-----
 board/vpac270/u-boot-spl.lds                   |    6 +-----
 include/asm-generic/sections.h                 |    3 ---
 16 files changed, 9 insertions(+), 82 deletions(-)

arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds +0 -5

index 673c725..f4e7525 100644
@@ -57,11 +57,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	.bss : {
 		. = ALIGN(4);
 		__bss_start = .;

arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds +0 -5

index 967a135..446d095 100644
@@ -57,11 +57,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	.bss : {
 		. = ALIGN(4);
 		__bss_start = .;

arch/arm/cpu/ixp/u-boot.lds +1 -5

index 553589c..5cfff68 100644
@@ -62,11 +62,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -88,6 +83,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/cpu/u-boot-spl.lds +1 -5

index 1408f03..b6ed25f 100644
@@ -58,11 +58,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 	.bss __rel_dyn_start (OVERLAY) : {

@@ -72,6 +67,7 @@ SECTIONS
 		__bss_end = .;
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/cpu/u-boot.lds +1 -5

index d9bbee3..fe2ca98 100644
@@ -65,11 +65,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 	/*

@@ -101,6 +96,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/lib/relocate.S +0 ~~-13~~

index 4446da9..7a7c4c0 100644
@@ -56,8 +56,6 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, _dynsym_start_ofs	/* r10 <- __dynsym_start local ofs */
-	add	r10, r10, r7		/* r10 <- SRC &__dynsym_start */
 	ldr	r2, _rel_dyn_start_ofs	/* r2 <- __rel_dyn_start local ofs */
 	add	r2, r2, r7		/* r2 <- SRC &__rel_dyn_start */
 	ldr	r3, _rel_dyn_end_ofs	/* r3 <- __rel_dyn_end local ofs */

@@ -69,17 +67,8 @@ fixloop:
 	and	r7, r1, #0xff
 	cmp	r7, #23			/* relative fixup? */
 	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
 	/* ignore unknown type of fixup */
 	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
 fixrel:
 	/* relative fix: increase location by offset */
 	ldr	r1, [r0]

@@ -106,7 +95,5 @@ _rel_dyn_start_ofs:
 	.word __rel_dyn_start - relocate_code
 _rel_dyn_end_ofs:
 	.word __rel_dyn_end - relocate_code
-_dynsym_start_ofs:
-	.word __dynsym_start - relocate_code
 
 ENDPROC(relocate_code)

board/actux1/u-boot.lds +1 -5

index ef4a25b..989ad71 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/actux2/u-boot.lds +1 -5

index 00ad8b7..0e20670 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/actux3/u-boot.lds +1 -5

index 44b990e..b7d29b4 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/ait/cam_enc_4xx/u-boot-spl.lds +0 -5

index 1daa1b3..3972685 100644
@@ -54,11 +54,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	} >.sram
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	} >.sram
-
 	.bss :
 	{
 		. = ALIGN(4);

board/davinci/da8xxevm/u-boot-spl-da850evm.lds +0 -5

index b1b8701..6fa4509 100644
@@ -55,11 +55,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	} >.sram
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	} >.sram
-
 	.bss :
 	{
 		. = ALIGN(4);

board/davinci/da8xxevm/u-boot-spl-hawk.lds +0 -1

index 596a9e0..b452f20 100644
@@ -61,7 +61,6 @@ SECTIONS
 	__image_copy_end = .;
 	__rel_dyn_start = .;
 	__rel_dyn_end = .;
-	__dynsym_start = .;
 
 	__got_start = .;
 	. = ALIGN(4);

board/dvlhost/u-boot.lds +1 -5

index 6d4b187..ecd9efe 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/freescale/mx31ads/u-boot.lds +1 -5

index 4969960..2197883 100644
@@ -73,11 +73,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -100,6 +95,7 @@ SECTIONS
 	}
 
 	/DISCARD/ : { *(.bss*) }
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynsym*) }
 	/DISCARD/ : { *(.dynamic*) }

board/vpac270/u-boot-spl.lds +1 -5

index 61d1154..1a3ef92 100644
@@ -67,11 +67,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	. = ALIGN(0x800);
 
 	_end = .;

@@ -84,6 +79,7 @@ SECTIONS
 	}
 
 	/DISCARD/ : { *(.bss*) }
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynsym*) }
 	/DISCARD/ : { *(.dynamic*) }

include/asm-generic/sections.h +0 -3

index 4b39844..3e32eee 100644
@@ -90,9 +90,6 @@ extern void _start(void);
 extern ulong _rel_dyn_start_ofs;
 extern ulong _rel_dyn_end_ofs;
 
-/* Start/end of the relocation symbol table, as an offset from _start */
-extern ulong _dynsym_start_ofs;
-
 /* End of the region to be relocated, as an offset form _start */
 extern ulong _image_copy_end_ofs;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #22 lore raw

PATCH v2 3/6 arm: generalize lib/bss.c into lib/sections.c

File arch/arm/lib/bss.c was initially defined for BSS only, but is now going to also contain definitions for other section-boundary-related symbols, so rename it for better accuracy.

Also, remove useless 'used' attributes.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2: None

 arch/arm/lib/Makefile              |    2 +-
 arch/arm/lib/{bss.c => sections.c} |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (92%)

arch/arm/lib/Makefile +1 -1

index 30e3290..dbad5c1 100644
@@ -43,7 +43,7 @@ SOBJS-y += relocate.o
 ifndef CONFIG_SYS_GENERIC_BOARD
 COBJS-y	+= board.o
 endif
-COBJS-y += bss.o
+COBJS-y += sections.o
 
 COBJS-y	+= bootm.o
 COBJS-$(CONFIG_SYS_L2_PL310) += cache-pl310.o

arch/arm/lib/sections.c renamed from arch/arm/lib/bss.c +2 -2

similarity index 92%
rename from arch/arm/lib/bss.c
rename to arch/arm/lib/sections.c
index 99eda59..e52fec9 100644
@@ -35,5 +35,5 @@
  * aliasing warnings.
  */
 
-char __bss_start[0] __attribute__((used, section(".__bss_start")));
-char __bss_end[0] __attribute__((used, section(".__bss_end")));
+char __bss_start[0] __attribute__((section(".__bss_start")));
+char __bss_end[0] __attribute__((section(".__bss_end")));

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #23 lore raw

PATCH v2 4/6 arm: make __image_copy_{start, end} compiler-generated

This change is only done where needed: some linker scripts may contain __image_copy_{start,end} yet remain unchanged.

Also, __image_copy_end needs its own section; putting it in relocation sections changes their flags and makes relocation breaks.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2: None

 arch/arm/cpu/arm920t/ep93xx/u-boot.lds |    6 +++++-
 arch/arm/cpu/ixp/u-boot.lds            |    6 +++++-
 arch/arm/cpu/u-boot.lds                |    7 +++++--
 arch/arm/lib/relocate.S                |    7 ++-----
 arch/arm/lib/sections.c                |    2 ++
 board/actux1/u-boot.lds                |    6 +++++-
 board/actux2/u-boot.lds                |    6 +++++-
 board/actux3/u-boot.lds                |    6 +++++-
 board/dvlhost/u-boot.lds               |    6 +++++-
 board/freescale/mx31ads/u-boot.lds     |    6 +++++-
 10 files changed, 44 insertions(+), 14 deletions(-)

arch/arm/cpu/arm920t/ep93xx/u-boot.lds +5 -1

index cf55bf7..367c805 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text      :
 	{
+		*(.__image_copy_start)
 	  arch/arm/cpu/arm920t/start.o	(.text*)
 		/* the EP93xx expects to find the pattern 'CRUS' at 0x1000 */
 	  . = 0x1000;

@@ -56,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	__bss_start = .;
 	.bss : { *(.bss*) }

arch/arm/cpu/ixp/u-boot.lds +5 -1

index 5cfff68..9141199 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		*(.text*)
 	}

@@ -54,7 +55,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/cpu/u-boot.lds +5 -2

index fe2ca98..d7adf90 100644
@@ -33,7 +33,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
-		__image_copy_start = .;
+		*(.__image_copy_start)
 		CPUDIR/start.o (.text*)
 		*(.text*)
 	}

@@ -57,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/lib/relocate.S +2 -5

index 7a7c4c0..3767a95 100644
@@ -39,13 +39,12 @@
 ENTRY(relocate_code)
 	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =_start		/* r0 <- SRC &_start */
+	ldr	r0, =__image_copy_start	/* r0 <- SRC &__image_copy_start */
 	subs	r9, r6, r0		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
 	mov	r1, r6			/* r1 <- scratch for copy loop */
 	adr	r7, relocate_code	/* r7 <- SRC &relocate_code */
-	ldr	r3, _image_copy_end_ofs	/* r3 <- __image_copy_end local ofs */
-	add	r2, r7, r3		/* r2 <- SRC &__image_copy_end */
+	ldr	r2, =__image_copy_end	/* r2 <- SRC &__image_copy_end */
 
 copy_loop:
 	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */

@@ -89,8 +88,6 @@ relocate_done:
         bx        lr
 #endif
 
-_image_copy_end_ofs:
-	.word __image_copy_end - relocate_code
 _rel_dyn_start_ofs:
 	.word __rel_dyn_start - relocate_code
 _rel_dyn_end_ofs:

arch/arm/lib/sections.c +2 -0

index e52fec9..03e846f 100644
@@ -37,3 +37,5 @@
 
 char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
+char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
+char __image_copy_end[0] __attribute__((section(".__image_copy_end")));

board/actux1/u-boot.lds +5 -1

index 989ad71..531e598 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux1/libactux1.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux2/u-boot.lds +5 -1

index 0e20670..aff773c 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux2/libactux2.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux3/u-boot.lds +5 -1

index b7d29b4..9d43e95 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux3/libactux3.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/dvlhost/u-boot.lds +5 -1

index ecd9efe..ee7219f 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/dvlhost/libdvlhost.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/freescale/mx31ads/u-boot.lds +5 -1

index 2197883..f8ef00c 100644
@@ -34,6 +34,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text	   :
 	{
+		*(.__image_copy_start)
 	  /* WARNING - the following is hand-optimized to fit within	*/
 	  /* the sector layout of our flash chips!	XXX FIXME XXX	*/

@@ -65,7 +66,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #24 lore raw

PATCH v2 5/6 arm: make __rel_dyn_{start, end} compiler-generated

This change is only done where needed: some linker scripts may contain relocation symbols yet remain unchanged.

__rel_dyn_start and __rel_dyn_end each requires its own output section; putting them in relocation sections changes their flags and breaks relocation.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2:
- various comment fixes

 arch/arm/cpu/ixp/u-boot.lds        |   12 ++++++++++--
 arch/arm/cpu/u-boot.lds            |   12 ++++++++++--
 arch/arm/lib/relocate.S            |   11 ++---------
 arch/arm/lib/sections.c            |    2 ++
 board/actux1/u-boot.lds            |   12 ++++++++++--
 board/actux2/u-boot.lds            |   12 ++++++++++--
 board/actux3/u-boot.lds            |   12 ++++++++++--
 board/dvlhost/u-boot.lds           |   12 ++++++++++--
 board/freescale/mx31ads/u-boot.lds |   12 ++++++++++--
 9 files changed, 74 insertions(+), 23 deletions(-)

arch/arm/cpu/ixp/u-boot.lds +10 -2

index 9141199..54bafda 100644
@@ -60,10 +60,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

arch/arm/cpu/u-boot.lds +10 -2

index d7adf90..3037885 100644
@@ -62,10 +62,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

arch/arm/lib/relocate.S +2 -9

index 3767a95..3f444c1 100644
@@ -55,10 +55,8 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r2, _rel_dyn_start_ofs	/* r2 <- __rel_dyn_start local ofs */
-	add	r2, r2, r7		/* r2 <- SRC &__rel_dyn_start */
-	ldr	r3, _rel_dyn_end_ofs	/* r3 <- __rel_dyn_end local ofs */
-	add	r3, r3, r7		/* r3 <- SRC &__rel_dyn_end */
+	ldr	r2, =__rel_dyn_start	/* r2 <- SRC &__rel_dyn_start */
+	ldr	r3, =__rel_dyn_end	/* r3 <- SRC &__rel_dyn_end */
 fixloop:
 	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
 	add	r0, r0, r9		/* r0 <- DST location to fix up */

@@ -88,9 +86,4 @@ relocate_done:
         bx        lr
 #endif
 
-_rel_dyn_start_ofs:
-	.word __rel_dyn_start - relocate_code
-_rel_dyn_end_ofs:
-	.word __rel_dyn_end - relocate_code
-
 ENDPROC(relocate_code)

arch/arm/lib/sections.c +2 -0

index 03e846f..5921dd8 100644
@@ -39,3 +39,5 @@ char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
 char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
 char __image_copy_end[0] __attribute__((section(".__image_copy_end")));
+char __rel_dyn_start[0] __attribute__((section(".__rel_dyn_start")));
+char __rel_dyn_end[0] __attribute__((section(".__rel_dyn_end")));

board/actux1/u-boot.lds +10 -2

index 531e598..74aec5f 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/actux2/u-boot.lds +10 -2

index aff773c..c276501 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/actux3/u-boot.lds +10 -2

index 9d43e95..5610644 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/dvlhost/u-boot.lds +10 -2

index ee7219f..f359112 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/freescale/mx31ads/u-boot.lds +10 -2

index f8ef00c..963d29f 100644
@@ -71,10 +71,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-05-28 07:01 UTC #25 lore raw

PATCH v2 6/6 arm: optimize relocate_code routine

Use section symbols directly
Drop support for R_ARM_ABS32 record types Eliminate unneeded intermediate registers Optimize relocation table iteration

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v2: None

 arch/arm/lib/relocate.S |   32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

arch/arm/lib/relocate.S +12 ~~-20~~

index 3f444c1..949b9e8 100644
@@ -37,19 +37,15 @@
  */
 
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
-
-	ldr	r0, =__image_copy_start	/* r0 <- SRC &__image_copy_start */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- SRC &__image_copy_start */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
-	adr	r7, relocate_code	/* r7 <- SRC &relocate_code */
 	ldr	r2, =__image_copy_end	/* r2 <- SRC &__image_copy_end */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*

@@ -58,21 +54,17 @@ copy_loop:
 	ldr	r2, =__rel_dyn_start	/* r2 <- SRC &__rel_dyn_start */
 	ldr	r3, =__rel_dyn_end	/* r3 <- SRC &__rel_dyn_end */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
+	add	r0, r0, r9
 	ldr	r1, [r0]
 	add	r1, r1, r9
-fixnext:
 	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
+fixnext:
 	cmp	r2, r3
 	blo	fixloop

Signature · 1 line

1.7.10.4

Benoît Thébaudeau

2013-05-28 17:11 UTC #26 lore raw

PATCH v2 4/6 arm: make __image_copy_{start, end} compiler-generated

In reply to Albert ARIBAUD

Hi Albert,

Quoted 7 lines · Albert ARIBAUD

This change is only done where needed: some linker scripts may contain __image_copy_{start,end} yet remain unchanged.
Also, __image_copy_end needs its own section; putting it in relocation sections changes their flags and makes relocation breaks.

             ^
             break

The rest of the patch is OK.

Best regards,
Beno?t

Benoît Thébaudeau

2013-05-28 17:04 UTC #27 lore raw

PATCH v2 1/6 arm: ensure u-boot only uses relative relocations

In reply to Albert ARIBAUD

Hi Albert,

Quoted 2 lines · Albert ARIBAUD

Add a Makefile target ('checkarmreloc') which fails of the ELF binary contains relocation records

        ^
        if

Sorry to have missed that in my review of v1.

Quoted 1 line

of types other than R_ARM_RELATIVE.

The rest of the patch is OK.

Best regards,
Beno?t

Albert ARIBAUD

author

2013-05-28 17:31 UTC #28 lore raw

PATCH v2 1/6 arm: ensure u-boot only uses relative relocations

Hi Beno?t,

Quoted 7 lines · Beno?t Th?baudeau

Hi Albert,
Quoted 2 lines · Albert ARIBAUD
Add a Makefile target ('checkarmreloc') which fails of the ELF binary contains relocation records
        ^
        if

(and)

Quoted 7 lines

Quoted 3 lines
Also, __image_copy_end needs its own section; putting it in relocation sections changes their flags and makes relocation breaks.
             ^
             break
Sorry to have missed that in my review of v1.

Never mind: I'd missed them too. :)

I'll wait a bit for other comments if any, then send out a fixed V3.

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-06-11 12:17 UTC #29 lore raw

v3 cover Optimize ARM relocation

In reply to Albert ARIBAUD

This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.

1. A Makefile rule is added that checks that no other relocation record types are generated except R_ARM_RELATIVE; build fails if this is the case.

2. All references to dymsym are removed, as this table is not used for R_ARM_RELATIVE relocations.

3. arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.

4. __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c

5. __rel_dyn_start and __rel_dyn_end are moved from linker scripts into arch/arm/lib/sections.c

6. relocate_code is optimized based on the fact that symbol references are now always valid even before relcation, and that only R_ARM_RELATIVE relocations will be met.

Changes in v3:
- fix commit message typo (of -> if)
- fix commit message typo (breaks -> break)

Changes in v2:
- use $< instead of $(obj)u-boot
- new in V2: remove all dynsym references
- various comment fixes

 Makefile                                       |    7 +++
 arch/arm/config.mk                             |    5 ++
 arch/arm/cpu/arm920t/ep93xx/u-boot.lds         |    6 ++-
 arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds      |    5 --
 arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds    |    5 --
 arch/arm/cpu/ixp/u-boot.lds                    |   20 +++++---
 arch/arm/cpu/u-boot-spl.lds                    |    6 +--
 arch/arm/cpu/u-boot.lds                        |   21 +++++---
 arch/arm/lib/Makefile                          |    2 +-
 arch/arm/lib/relocate.S                        |   61 ++++++------------------
 arch/arm/lib/{bss.c => sections.c}             |    8 +++-
 board/actux1/u-boot.lds                        |   20 +++++---
 board/actux2/u-boot.lds                        |   20 +++++---
 board/actux3/u-boot.lds                        |   20 +++++---
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    5 --
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    5 --
 board/davinci/da8xxevm/u-boot-spl-hawk.lds     |    1 -
 board/dvlhost/u-boot.lds                       |   20 +++++---
 board/freescale/mx31ads/u-boot.lds             |   20 +++++---
 board/vpac270/u-boot-spl.lds                   |    6 +--
 include/asm-generic/sections.h                 |    3 --
 21 files changed, 139 insertions(+), 127 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (79%)

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #30 lore raw

PATCH v3 1/6 arm: ensure u-boot only uses relative relocations

Add a Makefile target ('checkarmreloc') which fails if the ELF binary contains relocation records of types other than R_ARM_RELATIVE.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3:
- fix commit message typo (of -> if)

Changes in v2:
- use $< instead of $(obj)u-boot

 Makefile           |    7 +++++++
 arch/arm/config.mk |    5 +++++
 2 files changed, 12 insertions(+)

Makefile +7 -0

index af4c3c0..b1e5d5f 100644
@@ -743,6 +743,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $< | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
+		then echo "$< contains relocations other than \
+		R_ARM_RELATIVE"; false; fi
+
 $(VERSION_FILE):
 		@mkdir -p $(dir $(VERSION_FILE))
 		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \

arch/arm/config.mk +5 -0

index dc64160..e80e1ed 100644
@@ -109,3 +109,8 @@ ifeq ($(GAS_BUG_12532),y)
 PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
 endif
 endif
+
+# check that only R_ARM_RELATIVE relocations are generated
+ifneq ($(CONFIG_SPL_BUILD),y)
+ALL-y	+= checkarmreloc
+endif

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #31 lore raw

PATCH v3 2/6 remove all references to .dynsym

Discard all .dynsym sections from linker scripts Remove all __dynsym_start definitions from linker scripts Remove all __dynsym_start references from the codebase

Note: this touches include/asm-generic/sections.h, which is not ARM-specific, but actual uses of __dynsym_start are only in ARM, so this patch can safely go through the ARM repository.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3: None
Changes in v2:
- new in V2: remove all dynsym references

 arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds      |    5 -----
 arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds    |    5 -----
 arch/arm/cpu/ixp/u-boot.lds                    |    6 +-----
 arch/arm/cpu/u-boot-spl.lds                    |    6 +-----
 arch/arm/cpu/u-boot.lds                        |    6 +-----
 arch/arm/lib/relocate.S                        |   13 -------------
 board/actux1/u-boot.lds                        |    6 +-----
 board/actux2/u-boot.lds                        |    6 +-----
 board/actux3/u-boot.lds                        |    6 +-----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    5 -----
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    5 -----
 board/davinci/da8xxevm/u-boot-spl-hawk.lds     |    1 -
 board/dvlhost/u-boot.lds                       |    6 +-----
 board/freescale/mx31ads/u-boot.lds             |    6 +-----
 board/vpac270/u-boot-spl.lds                   |    6 +-----
 include/asm-generic/sections.h                 |    3 ---
 16 files changed, 9 insertions(+), 82 deletions(-)

arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds +0 -5

index 673c725..f4e7525 100644
@@ -57,11 +57,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	.bss : {
 		. = ALIGN(4);
 		__bss_start = .;

arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds +0 -5

index 967a135..446d095 100644
@@ -57,11 +57,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	.bss : {
 		. = ALIGN(4);
 		__bss_start = .;

arch/arm/cpu/ixp/u-boot.lds +1 -5

index 553589c..5cfff68 100644
@@ -62,11 +62,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -88,6 +83,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/cpu/u-boot-spl.lds +1 -5

index 1408f03..b6ed25f 100644
@@ -58,11 +58,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 	.bss __rel_dyn_start (OVERLAY) : {

@@ -72,6 +67,7 @@ SECTIONS
 		__bss_end = .;
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/cpu/u-boot.lds +1 -5

index d9bbee3..fe2ca98 100644
@@ -65,11 +65,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 	/*

@@ -101,6 +96,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

arch/arm/lib/relocate.S +0 ~~-13~~

index 4446da9..7a7c4c0 100644
@@ -56,8 +56,6 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, _dynsym_start_ofs	/* r10 <- __dynsym_start local ofs */
-	add	r10, r10, r7		/* r10 <- SRC &__dynsym_start */
 	ldr	r2, _rel_dyn_start_ofs	/* r2 <- __rel_dyn_start local ofs */
 	add	r2, r2, r7		/* r2 <- SRC &__rel_dyn_start */
 	ldr	r3, _rel_dyn_end_ofs	/* r3 <- __rel_dyn_end local ofs */

@@ -69,17 +67,8 @@ fixloop:
 	and	r7, r1, #0xff
 	cmp	r7, #23			/* relative fixup? */
 	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
 	/* ignore unknown type of fixup */
 	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
 fixrel:
 	/* relative fix: increase location by offset */
 	ldr	r1, [r0]

@@ -106,7 +95,5 @@ _rel_dyn_start_ofs:
 	.word __rel_dyn_start - relocate_code
 _rel_dyn_end_ofs:
 	.word __rel_dyn_end - relocate_code
-_dynsym_start_ofs:
-	.word __dynsym_start - relocate_code
 
 ENDPROC(relocate_code)

board/actux1/u-boot.lds +1 -5

index ef4a25b..989ad71 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/actux2/u-boot.lds +1 -5

index 00ad8b7..0e20670 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/actux3/u-boot.lds +1 -5

index 44b990e..b7d29b4 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/ait/cam_enc_4xx/u-boot-spl.lds +0 -5

index 1daa1b3..3972685 100644
@@ -54,11 +54,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	} >.sram
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	} >.sram
-
 	.bss :
 	{
 		. = ALIGN(4);

board/davinci/da8xxevm/u-boot-spl-da850evm.lds +0 -5

index b1b8701..6fa4509 100644
@@ -55,11 +55,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	} >.sram
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	} >.sram
-
 	.bss :
 	{
 		. = ALIGN(4);

board/davinci/da8xxevm/u-boot-spl-hawk.lds +0 -1

index 596a9e0..b452f20 100644
@@ -61,7 +61,6 @@ SECTIONS
 	__image_copy_end = .;
 	__rel_dyn_start = .;
 	__rel_dyn_end = .;
-	__dynsym_start = .;
 
 	__got_start = .;
 	. = ALIGN(4);

board/dvlhost/u-boot.lds +1 -5

index 6d4b187..ecd9efe 100644
@@ -70,11 +70,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -96,6 +91,7 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynamic*) }
 	/DISCARD/ : { *(.plt*) }

board/freescale/mx31ads/u-boot.lds +1 -5

index 4969960..2197883 100644
@@ -73,11 +73,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	_end = .;
 
 /*

@@ -100,6 +95,7 @@ SECTIONS
 	}
 
 	/DISCARD/ : { *(.bss*) }
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynsym*) }
 	/DISCARD/ : { *(.dynamic*) }

board/vpac270/u-boot-spl.lds +1 -5

index 61d1154..1a3ef92 100644
@@ -67,11 +67,6 @@ SECTIONS
 		__rel_dyn_end = .;
 	}
 
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
 	. = ALIGN(0x800);
 
 	_end = .;

@@ -84,6 +79,7 @@ SECTIONS
 	}
 
 	/DISCARD/ : { *(.bss*) }
+	/DISCARD/ : { *(.dynsym) }
 	/DISCARD/ : { *(.dynstr*) }
 	/DISCARD/ : { *(.dynsym*) }
 	/DISCARD/ : { *(.dynamic*) }

include/asm-generic/sections.h +0 -3

index 4b39844..3e32eee 100644
@@ -90,9 +90,6 @@ extern void _start(void);
 extern ulong _rel_dyn_start_ofs;
 extern ulong _rel_dyn_end_ofs;
 
-/* Start/end of the relocation symbol table, as an offset from _start */
-extern ulong _dynsym_start_ofs;
-
 /* End of the region to be relocated, as an offset form _start */
 extern ulong _image_copy_end_ofs;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #32 lore raw

PATCH v3 3/6 arm: generalize lib/bss.c into lib/sections.c

File arch/arm/lib/bss.c was initially defined for BSS only, but is now going to also contain definitions for other section-boundary-related symbols, so rename it for better accuracy.

Also, remove useless 'used' attributes.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3: None
Changes in v2: None

 arch/arm/lib/Makefile              |    2 +-
 arch/arm/lib/{bss.c => sections.c} |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (92%)

arch/arm/lib/Makefile +1 -1

index 8ad9f66..9ecafb2 100644
@@ -43,7 +43,7 @@ SOBJS-y += relocate.o
 ifndef CONFIG_SYS_GENERIC_BOARD
 COBJS-y	+= board.o
 endif
-COBJS-y += bss.o
+COBJS-y += sections.o
 
 COBJS-y	+= bootm.o
 COBJS-$(CONFIG_OF_LIBFDT) += bootm-fdt.o

arch/arm/lib/sections.c renamed from arch/arm/lib/bss.c +2 -2

similarity index 92%
rename from arch/arm/lib/bss.c
rename to arch/arm/lib/sections.c
index 99eda59..e52fec9 100644
@@ -35,5 +35,5 @@
  * aliasing warnings.
  */
 
-char __bss_start[0] __attribute__((used, section(".__bss_start")));
-char __bss_end[0] __attribute__((used, section(".__bss_end")));
+char __bss_start[0] __attribute__((section(".__bss_start")));
+char __bss_end[0] __attribute__((section(".__bss_end")));

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #33 lore raw

PATCH v3 4/6 arm: make __image_copy_{start, end} compiler-generated

This change is only done where needed: some linker scripts may contain __image_copy_{start,end} yet remain unchanged.

Also, __image_copy_end needs its own section; putting it in relocation sections changes their flags and makes relocation break.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3:
- fix commit message typo (breaks -> break)

Changes in v2: None

 arch/arm/cpu/arm920t/ep93xx/u-boot.lds |    6 +++++-
 arch/arm/cpu/ixp/u-boot.lds            |    6 +++++-
 arch/arm/cpu/u-boot.lds                |    7 +++++--
 arch/arm/lib/relocate.S                |    7 ++-----
 arch/arm/lib/sections.c                |    2 ++
 board/actux1/u-boot.lds                |    6 +++++-
 board/actux2/u-boot.lds                |    6 +++++-
 board/actux3/u-boot.lds                |    6 +++++-
 board/dvlhost/u-boot.lds               |    6 +++++-
 board/freescale/mx31ads/u-boot.lds     |    6 +++++-
 10 files changed, 44 insertions(+), 14 deletions(-)

arch/arm/cpu/arm920t/ep93xx/u-boot.lds +5 -1

index cf55bf7..367c805 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text      :
 	{
+		*(.__image_copy_start)
 	  arch/arm/cpu/arm920t/start.o	(.text*)
 		/* the EP93xx expects to find the pattern 'CRUS' at 0x1000 */
 	  . = 0x1000;

@@ -56,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	__bss_start = .;
 	.bss : { *(.bss*) }

arch/arm/cpu/ixp/u-boot.lds +5 -1

index 5cfff68..9141199 100644
@@ -31,6 +31,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		*(.text*)
 	}

@@ -54,7 +55,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/cpu/u-boot.lds +5 -2

index fe2ca98..d7adf90 100644
@@ -33,7 +33,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
-		__image_copy_start = .;
+		*(.__image_copy_start)
 		CPUDIR/start.o (.text*)
 		*(.text*)
 	}

@@ -57,7 +57,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

arch/arm/lib/relocate.S +2 -5

index 7a7c4c0..3767a95 100644
@@ -39,13 +39,12 @@
 ENTRY(relocate_code)
 	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =_start		/* r0 <- SRC &_start */
+	ldr	r0, =__image_copy_start	/* r0 <- SRC &__image_copy_start */
 	subs	r9, r6, r0		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
 	mov	r1, r6			/* r1 <- scratch for copy loop */
 	adr	r7, relocate_code	/* r7 <- SRC &relocate_code */
-	ldr	r3, _image_copy_end_ofs	/* r3 <- __image_copy_end local ofs */
-	add	r2, r7, r3		/* r2 <- SRC &__image_copy_end */
+	ldr	r2, =__image_copy_end	/* r2 <- SRC &__image_copy_end */
 
 copy_loop:
 	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */

@@ -89,8 +88,6 @@ relocate_done:
         bx        lr
 #endif
 
-_image_copy_end_ofs:
-	.word __image_copy_end - relocate_code
 _rel_dyn_start_ofs:
 	.word __rel_dyn_start - relocate_code
 _rel_dyn_end_ofs:

arch/arm/lib/sections.c +2 -0

index e52fec9..03e846f 100644
@@ -37,3 +37,5 @@
 
 char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
+char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
+char __image_copy_end[0] __attribute__((section(".__image_copy_end")));

board/actux1/u-boot.lds +5 -1

index 989ad71..531e598 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux1/libactux1.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux2/u-boot.lds +5 -1

index 0e20670..aff773c 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux2/libactux2.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/actux3/u-boot.lds +5 -1

index b7d29b4..9d43e95 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/actux3/libactux3.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/dvlhost/u-boot.lds +5 -1

index ecd9efe..ee7219f 100644
@@ -30,6 +30,7 @@ SECTIONS
 
 	. = ALIGN (4);
 	.text : {
+		*(.__image_copy_start)
 		arch/arm/cpu/ixp/start.o(.text*)
 		net/libnet.o(.text*)
 		board/dvlhost/libdvlhost.o(.text*)

@@ -62,7 +63,10 @@ SECTIONS
 
 	. = ALIGN (4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

board/freescale/mx31ads/u-boot.lds +5 -1

index 2197883..f8ef00c 100644
@@ -34,6 +34,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text	   :
 	{
+		*(.__image_copy_start)
 	  /* WARNING - the following is hand-optimized to fit within	*/
 	  /* the sector layout of our flash chips!	XXX FIXME XXX	*/

@@ -65,7 +66,10 @@ SECTIONS
 
 	. = ALIGN(4);
 
-	__image_copy_end = .;
+	.image_copy_end :
+	{
+		*(.__image_copy_end)
+	}
 
 	.rel.dyn : {
 		__rel_dyn_start = .;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #34 lore raw

PATCH v3 5/6 arm: make __rel_dyn_{start, end} compiler-generated

This change is only done where needed: some linker scripts may contain relocation symbols yet remain unchanged.

__rel_dyn_start and __rel_dyn_end each requires its own output section; putting them in relocation sections changes their flags and breaks relocation.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3: None
Changes in v2:
- various comment fixes

 arch/arm/cpu/ixp/u-boot.lds        |   12 ++++++++++--
 arch/arm/cpu/u-boot.lds            |   12 ++++++++++--
 arch/arm/lib/relocate.S            |   11 ++---------
 arch/arm/lib/sections.c            |    2 ++
 board/actux1/u-boot.lds            |   12 ++++++++++--
 board/actux2/u-boot.lds            |   12 ++++++++++--
 board/actux3/u-boot.lds            |   12 ++++++++++--
 board/dvlhost/u-boot.lds           |   12 ++++++++++--
 board/freescale/mx31ads/u-boot.lds |   12 ++++++++++--
 9 files changed, 74 insertions(+), 23 deletions(-)

arch/arm/cpu/ixp/u-boot.lds +10 -2

index 9141199..54bafda 100644
@@ -60,10 +60,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

arch/arm/cpu/u-boot.lds +10 -2

index d7adf90..3037885 100644
@@ -62,10 +62,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

arch/arm/lib/relocate.S +2 -9

index 3767a95..3f444c1 100644
@@ -55,10 +55,8 @@ copy_loop:
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r2, _rel_dyn_start_ofs	/* r2 <- __rel_dyn_start local ofs */
-	add	r2, r2, r7		/* r2 <- SRC &__rel_dyn_start */
-	ldr	r3, _rel_dyn_end_ofs	/* r3 <- __rel_dyn_end local ofs */
-	add	r3, r3, r7		/* r3 <- SRC &__rel_dyn_end */
+	ldr	r2, =__rel_dyn_start	/* r2 <- SRC &__rel_dyn_start */
+	ldr	r3, =__rel_dyn_end	/* r3 <- SRC &__rel_dyn_end */
 fixloop:
 	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
 	add	r0, r0, r9		/* r0 <- DST location to fix up */

@@ -88,9 +86,4 @@ relocate_done:
         bx        lr
 #endif
 
-_rel_dyn_start_ofs:
-	.word __rel_dyn_start - relocate_code
-_rel_dyn_end_ofs:
-	.word __rel_dyn_end - relocate_code
-
 ENDPROC(relocate_code)

arch/arm/lib/sections.c +2 -0

index 03e846f..5921dd8 100644
@@ -39,3 +39,5 @@ char __bss_start[0] __attribute__((section(".__bss_start")));
 char __bss_end[0] __attribute__((section(".__bss_end")));
 char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
 char __image_copy_end[0] __attribute__((section(".__image_copy_end")));
+char __rel_dyn_start[0] __attribute__((section(".__rel_dyn_start")));
+char __rel_dyn_end[0] __attribute__((section(".__rel_dyn_end")));

board/actux1/u-boot.lds +10 -2

index 531e598..74aec5f 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/actux2/u-boot.lds +10 -2

index aff773c..c276501 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/actux3/u-boot.lds +10 -2

index 9d43e95..5610644 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/dvlhost/u-boot.lds +10 -2

index ee7219f..f359112 100644
@@ -68,10 +68,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

board/freescale/mx31ads/u-boot.lds +10 -2

index f8ef00c..963d29f 100644
@@ -71,10 +71,18 @@ SECTIONS
 		*(.__image_copy_end)
 	}
 
+	.rel_dyn_start :
+	{
+		*(.__rel_dyn_start)
+	}
+
 	.rel.dyn : {
-		__rel_dyn_start = .;
 		*(.rel*)
-		__rel_dyn_end = .;
+	}
+
+	.rel_dyn_end :
+	{
+		*(.__rel_dyn_end)
 	}
 
 	_end = .;

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:17 UTC #35 lore raw

PATCH v3 6/6 arm: optimize relocate_code routine

Use section symbols directly
Drop support for R_ARM_ABS32 record types
Eliminate unneeded intermediate registers
Optimize relocation table iteration

Signed-off-by Albert ARIBAUD <[email protected]>

---
Changes in v3: None
Changes in v2: None

 arch/arm/lib/relocate.S |   32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

arch/arm/lib/relocate.S +12 ~~-20~~

index 3f444c1..949b9e8 100644
@@ -37,19 +37,15 @@
  */
 
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
-
-	ldr	r0, =__image_copy_start	/* r0 <- SRC &__image_copy_start */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- SRC &__image_copy_start */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
-	adr	r7, relocate_code	/* r7 <- SRC &relocate_code */
 	ldr	r2, =__image_copy_end	/* r2 <- SRC &__image_copy_end */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*

@@ -58,21 +54,17 @@ copy_loop:
 	ldr	r2, =__rel_dyn_start	/* r2 <- SRC &__rel_dyn_start */
 	ldr	r3, =__rel_dyn_end	/* r3 <- SRC &__rel_dyn_end */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
+	add	r0, r0, r9
 	ldr	r1, [r0]
 	add	r1, r1, r9
-fixnext:
 	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
+fixnext:
 	cmp	r2, r3
 	blo	fixloop

Signature · 1 line

1.7.10.4

Albert ARIBAUD

author

2013-06-11 12:47 UTC #36 lore raw

v3 cover Optimize ARM relocation

In reply to Albert ARIBAUD

Hi Albert,

Quoted 4 lines · Albert ARIBAUD

This series optimizes relocation by ensuring ARM
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.

This is a cosmetic version only; if no one complains loudly, this will be the final version too.

So people, as this touches ARM relocation, I would appreciate it if a few people could test it on their own ARM HW target and see if they still get a prompt, if command completion still works, etc.

Amicalement,

Signature · 1 line

Albert.

Lubomir Popov

2013-06-11 14:22 UTC #37 lore raw

v3 cover Optimize ARM relocation

Hi Albert,

Quoted 10 lines · Albert ARIBAUD

Hi Albert,
Quoted 4 lines · Albert ARIBAUD
This series optimizes relocation by ensuring ARM
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.

Quoted 6 lines

This is a cosmetic version only; if no one complains loudly, this will be the final version too.
So people, as this touches ARM relocation, I would appreciate it if a few people could test it on their own ARM HW target and see if they still get a prompt, if command completion still works, etc.

Applied the series to a relatively fresh (yesterday's) u-boot-ti/master and tested on a custom OMAP5430 board. Boots normally. USB storage and Ethernet (dhcp & ping), SD/MMC, I2C are all working.

Tested-by Lubomir Popov <[email protected]>

Regards,
Lubo

Albert ARIBAUD

author

2013-06-11 15:29 UTC #38 lore raw

v3 cover Optimize ARM relocation

Hi Lubomir,

Quoted 7 lines · Lubomir Popov

Hi Albert,
Applied the series to a relatively fresh (yesterday's) u-boot-ti/master and tested on a custom OMAP5430 board. Boots normally. USB storage and Ethernet (dhcp & ping), SD/MMC, I2C are all working.
Tested-by Lubomir Popov <[email protected]>

Thanks a lot!

Quoted 2 lines

Regards,
Lubo

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-06-13 09:05 UTC #39 lore raw

v3 cover Optimize ARM relocation

In reply to Albert ARIBAUD

Hi Albert,

Quoted 10 lines · Albert ARIBAUD

Hi Albert,
Quoted 4 lines · Albert ARIBAUD
This series optimizes relocation by ensuring ARM
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.

Quoted 6 lines

This is a cosmetic version only; if no one complains loudly, this will be the final version too.
So people, as this touches ARM relocation, I would appreciate it if a few people could test it on their own ARM HW target and see if they still get a prompt, if command completion still works, etc.

Lubo has tested on a custom ARM board, my thanks to him.

Anyone else would be so kind as to do a quick test on their ARM boards?

Amicalement,

Signature · 1 line

Albert.

Jeroen Hofstee

2013-06-13 18:54 UTC #40 lore raw

v3 cover Optimize ARM relocation

Quoted 17 lines · Albert ARIBAUD

Hi Albert,
Quoted 12 lines · Albert ARIBAUD
Hi Albert,
Quoted 4 lines · Albert ARIBAUD
This series optimizes relocation by ensuring ARM
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.
This is a cosmetic version only; if no one complains loudly, this will be the final version too.

Quoted 8 lines

Quoted 3 lines
So people, as this touches ARM relocation, I would appreciate it if a few people could test it on their own ARM HW target and see if they still get a prompt, if command completion still works, etc.
Lubo has tested on a custom ARM board, my thanks to him.
Anyone else would be so kind as to do a quick test on their ARM boards?
Amicalement,

Tested-by Jeroen Hofstee <[email protected]>

arm-master + these patches on a twister (am3517) with gcc / Linaro 4.7.2. Will have a look at clang.

Regards,
Jeroen

am35

Jeroen Hofstee

2013-06-16 13:33 UTC #41 lore raw

v3 cover Optimize ARM relocation

Hello Albert,

Quoted 6 lines · Jeroen Hofstee

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.

fyi, I had a look at the clang build and it doesn't boot after these patches...

When constant pointers are used without fpie, e.g. the arguments to printf, gcc will load the address with ldr / R_ARM_ABS32, clang sets the address with a movw / movt pair.

ld -r will make the former relative, but the movw / movt cannot be fixed. So I set fpie for clang, which generates a couple of R_ARM_ABS32:

Most notably a reference to do_bootd. Since it is no longer valid after this patch and used in command.c this crashes the board (unless there happens to be a valid address in it). gcc seems to always recalculate it using pc.

Another symbol is _start, but that looks easily fixable.

The last one looks similar like the do_bootd and I haven't bothered to check the details.

Regards,
Jeroen

Albert ARIBAUD

author

2013-06-18 14:54 UTC #42 lore raw

v3 cover Optimize ARM relocation

Hi Jeroen,

Quoted 10 lines · Jeroen Hofstee

Hello Albert,
Quoted 6 lines · Jeroen Hofstee
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.

Quoted 7 lines

fyi, I had a look at the clang build and it doesn't boot after these patches...
When constant pointers are used without fpie, e.g. the arguments to printf, gcc will load the address with ldr / R_ARM_ABS32, clang sets the address with a movw / movt pair.

Hmm... Why do you remove -fpie from the gcc build?

Quoted 9 lines

ld -r will make the former relative, but the movw / movt cannot be fixed. So I set fpie for clang, which generates a couple of R_ARM_ABS32:
Most notably a reference to do_bootd. Since it is no longer valid after this patch and used in command.c this crashes the board (unless there happens to be a valid address in it). gcc seems to always recalculate it using pc.

Quoted 4 lines

Another symbol is _start, but that looks easily fixable.
The last one looks similar like the do_bootd and I haven't bothered to check the details.

Can you give more precise info on the issue? Such as the U-Boot codebase (even if not in shape for submitting) and clang compiler version you are using, so that I can try the build on my side and have a look at how/why r_arm_abs32 relocation recodes are generated and how to avoid it?

Quoted 2 lines

Regards,
Jeroen

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

author

2013-06-19 07:31 UTC #43 lore raw

Quoted 36 lines · Albert ARIBAUD

Hi Jeroen,
Quoted 10 lines · Jeroen Hofstee
Hello Albert,
Quoted 6 lines · Jeroen Hofstee
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
binaries only use one type of relocation record,
R_ARM_RELATIVE., then optimizing relocation code
accordingly.
Quoted 7 lines
fyi, I had a look at the clang build and it doesn't boot after these patches...
When constant pointers are used without fpie, e.g. the arguments to printf, gcc will load the address with ldr / R_ARM_ABS32, clang sets the address with a movw / movt pair.
Hmm... Why do you remove -fpie from the gcc build?
Quoted 11 lines
ld -r will make the former relative, but the movw / movt cannot be fixed. So I set fpie for clang, which generates a couple of R_ARM_ABS32:
Most notably a reference to do_bootd. Since it is no longer valid after this patch and used in command.c this crashes the board (unless there happens to be a valid address in it). gcc seems to always recalculate it using pc.
Another symbol is _start, but that looks easily fixable.

Quoted 8 lines

Quoted 2 lines
The last one looks similar like the do_bootd and I haven't bothered to check the details.
Can you give more precise info on the issue? Such as the U-Boot codebase (even if not in shape for submitting) and clang compiler version you are using, so that I can try the build on my side and have a look at how/why r_arm_abs32 relocation recodes are generated and how to avoid it?

Update:

After some IRC discussion with Jeroen, we've been able to pinpoint an issue when building armv7 (and possibly others) with optimization level -O2 instead of -Os (OPTFLAGS in ./config.mk). NOTE: -O2 is not the optimization level defined by default, and default builds do not exhibit any issue.

Specifically, building the twister target with -O2 results in the build failing with this error message: "arm-linux-gnueabi-ld.bfd:
arch/arm/cpu/armv7/omap3/libomap3.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can not be used when making a shared object; recompile with -fPIC".

Compiler option -fPIC was replaced with linker option -pie when switching from GOT to ELF relocation tables in commit 92d5ecba.

Adding -fPIE (rather than -fPIC as we are linking an executable) results in the build succeeding again, but with a few R_ARM_ABS32 relocations creeping back in.

Analysis of the absolute relocations show that their causes are:

1. use of 'ldr r0, =_start' instead of 'adr r0,_start' in arch/arm/cpu/armv7/start.S (twice) -- actually, ldr refers to the link-time _start, which may be right or wrong, whereas adr refers to the run-time _start, which is always correct.

2. use of 'ldr r0, =_start' instead of 'adr r0,_start' in arch/arm/lib/relocate.S. Same remark as above.

3. Declaration EXPORT_FUNC(do_reset) in include/_exports.h. That is the only 'do_xxx' exported function, and I suspect there is no actual need for exporting it.
Incidentally, the U_BOOT_CMD() statement is in common/cmd_boot.c while the do_reset function is in arch/arm/lib/reset -- that's surprising, although I understand why we might not want a common/do_reset.c file with only the U_BOOT_CMD() statement.

4. There is an explicit comparision between a function pointer and do_bootd for recursion detection reasons. While I am not sure why there is a recursion issue, I am sure however that we could test for recursion by turning the 'repeatable' field in cmd_tbl_s into a 'flags' field with bit 0 set for 'repeatable' and bit 1 set when command is 'do_bootd', thereby removing the test for the do_bootd address.

Even though -O2 (plus -fPIE) is not currently used in U-boot, and even though it reduces the relocation table size, I don't intend to switch to it because this increases the code and data size -- for twister, global increase is 4.5% for u-boot and 15.5% for SPL.

Nevertheless, I think we should fix the issues above, if only because they are indicative of potential trouble, so I'll send out a patch for this; but it'll be based over the relocation optimization patches to avoid merge conflicts.

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

2013-06-12 22:38 UTC #44 lore raw

v3 cover Optimize ARM relocation

In reply to Albert ARIBAUD

Hi Albert,

Quoted 22 lines · Albert ARIBAUD

This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.
1. A Makefile rule is added that checks that no other relocation record types are generated except R_ARM_RELATIVE; build fails if this is the case.
2. All references to dymsym are removed, as this table is not used for R_ARM_RELATIVE relocations.
3. arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.
4. __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c
5. __rel_dyn_start and __rel_dyn_end are moved from linker scripts into arch/arm/lib/sections.c

Quoted 8 lines

6. relocate_code is optimized based on the fact that symbol references are now always valid even before relcation, and that only R_ARM_RELATIVE relocations will be met.
Changes in v3:
- fix commit message typo (of -> if)
- fix commit message typo (breaks -> break)

For this v3 series:

Reviewed-by Beno?t Th?baudeau <[email protected]>

Best regards,
Beno?t

Albert ARIBAUD

author

2013-06-21 21:07 UTC #45 lore raw

v3 cover Optimize ARM relocation

In reply to Albert ARIBAUD

Quoted 59 lines · Albert ARIBAUD

This series optimizes relocation by ensuring ARM binaries only use one type of relocation record, R_ARM_RELATIVE., then optimizing relocation code accordingly.
1. A Makefile rule is added that checks that no other relocation record types are generated except R_ARM_RELATIVE; build fails if this is the case.
2. All references to dymsym are removed, as this table is not used for R_ARM_RELATIVE relocations.
3. arch/arm/lib/bss.c is replaced by a more generic arch/arm/lib/sections.c where all section symbols will be defined.
4. __image_copy_start and __image_copy_end symbols are moved from linker scripts to arch/arm/lib/sections.c
5. __rel_dyn_start and __rel_dyn_end are moved from linker scripts into arch/arm/lib/sections.c
6. relocate_code is optimized based on the fact that symbol references are now always valid even before relcation, and that only R_ARM_RELATIVE relocations will be met.
Changes in v3:
- fix commit message typo (of -> if)
- fix commit message typo (breaks -> break)
Changes in v2:
- use $< instead of $(obj)u-boot
- new in V2: remove all dynsym references
- various comment fixes
Albert ARIBAUD (6):
arm: ensure u-boot only uses relative relocations remove all references to .dynsym
arm: generalize lib/bss.c into lib/sections.c
arm: make __image_copy_{start,end} compiler-generated
arm: make __rel_dyn_{start,end} compiler-generated
arm: optimize relocate_code routine
 Makefile                                       |    7 +++
 arch/arm/config.mk                             |    5 ++
 arch/arm/cpu/arm920t/ep93xx/u-boot.lds         |    6 ++-
 arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds      |    5 --
 arch/arm/cpu/arm926ejs/spear/u-boot-spl.lds    |    5 --
 arch/arm/cpu/ixp/u-boot.lds                    |   20 +++++---
 arch/arm/cpu/u-boot-spl.lds                    |    6 +--
 arch/arm/cpu/u-boot.lds                        |   21 +++++---
 arch/arm/lib/Makefile                          |    2 +-
 arch/arm/lib/relocate.S                        |   61 ++++++------------------
 arch/arm/lib/{bss.c => sections.c}             |    8 +++-
 board/actux1/u-boot.lds                        |   20 +++++---
 board/actux2/u-boot.lds                        |   20 +++++---
 board/actux3/u-boot.lds                        |   20 +++++---
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    5 --

Quoted 9 lines

 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    5 --
 board/davinci/da8xxevm/u-boot-spl-hawk.lds     |    1 -
 board/dvlhost/u-boot.lds                       |   20 +++++---
 board/freescale/mx31ads/u-boot.lds             |   20 +++++---
 board/vpac270/u-boot-spl.lds                   |    6 +--
 include/asm-generic/sections.h                 |    3 --
 21 files changed, 139 insertions(+), 127 deletions(-)
 rename arch/arm/lib/{bss.c => sections.c} (79%)

Applied to u-boot-arm/master.

Amicalement,

Signature · 1 line

Albert.