[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

** This patch is only meant to allow others to test relocation, it
should not be applied!! **

This patch is a quick hack to enable proper relocation on powerpc
boards.  I tested on some mpc85xx-based boards.

I updated the common ppc config.mk and u-boot.lds in cpu/ as needed, but
didn't bother to update board-specific ones.  CONFIG_RELOC_FIXUP_WORKS
has also been hacked into common.h unconditionally.

So if you want to try out this patch, make sure that you
1. Remove the *(.fixup) entry from the text section in your board's
linker script.

2. Make sure your board's config.mk file includes:
PLATFORM_RELFLAGS += -mrelocatable

I'm hoping that relocation will work for all powerpc boards assuming you
use a semi-recent version.  I think at least gcc >= 3.4.6 (or maybe even
3.4.5) should work.

It'd be great if people could give feedback if this patch works for
them, and if not, how their board breaks.

Thanks,
Peter
---
 cpu/74xx_7xx/config.mk              |    2 +-
 cpu/mpc512x/config.mk               |    2 +-
 cpu/mpc512x/u-boot.lds              |    1 -
 cpu/mpc5xx/config.mk                |    2 +-
 cpu/mpc5xx/u-boot.lds               |    1 -
 cpu/mpc5xxx/config.mk               |    2 +-
 cpu/mpc5xxx/u-boot-customlayout.lds |    1 -
 cpu/mpc5xxx/u-boot.lds              |    1 -
 cpu/mpc8220/config.mk               |    2 +-
 cpu/mpc8220/u-boot.lds              |    1 -
 cpu/mpc824x/config.mk               |    2 +-
 cpu/mpc824x/u-boot.lds              |    1 -
 cpu/mpc8260/config.mk               |    2 +-
 cpu/mpc8260/u-boot.lds              |    1 -
 cpu/mpc83xx/config.mk               |    2 +-
 cpu/mpc83xx/u-boot.lds              |    1 -
 cpu/mpc85xx/config.mk               |    2 +-
 cpu/mpc85xx/u-boot.lds              |    1 -
 cpu/mpc86xx/config.mk               |    2 +-
 cpu/mpc8xx/config.mk                |    2 +-
 cpu/ppc4xx/config.mk                |    2 +-
 include/common.h                    |    1 +
 22 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/cpu/74xx_7xx/config.mk b/cpu/74xx_7xx/config.mk
index d589210..68d27fe 100644
--- a/cpu/74xx_7xx/config.mk
+++ b/cpu/74xx_7xx/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_74xx_7xx -ffixed-r2 -mstring
diff --git a/cpu/mpc512x/config.mk b/cpu/mpc512x/config.mk
index 6ab34b1..07e3e55 100644
--- a/cpu/mpc512x/config.mk
+++ b/cpu/mpc512x/config.mk
@@ -19,7 +19,7 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston,
 # MA 02111-1307 USA
 #
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC512X -DCONFIG_E300 \
 			-ffixed-r2 -msoft-float -mcpu=603e
diff --git a/cpu/mpc512x/u-boot.lds b/cpu/mpc512x/u-boot.lds
index dae3269..2e260eb 100644
--- a/cpu/mpc512x/u-boot.lds
+++ b/cpu/mpc512x/u-boot.lds
@@ -51,7 +51,6 @@ SECTIONS
   {
     cpu/mpc512x/start.o	(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc5xx/config.mk b/cpu/mpc5xx/config.mk
index 157ddc5..60320e6 100644
--- a/cpu/mpc5xx/config.mk
+++ b/cpu/mpc5xx/config.mk
@@ -28,7 +28,7 @@
 #
 
 
-PLATFORM_RELFLAGS +=	-fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS +=	-fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS +=	-DCONFIG_5xx -ffixed-r2 -mpowerpc -msoft-float
 
diff --git a/cpu/mpc5xx/u-boot.lds b/cpu/mpc5xx/u-boot.lds
index cb17ca5..deeb06a 100644
--- a/cpu/mpc5xx/u-boot.lds
+++ b/cpu/mpc5xx/u-boot.lds
@@ -58,7 +58,6 @@ SECTIONS
     cpu/mpc5xx/start.o	(.text)
 
     *(.text)
-    *(.fixup)
     *(.got1)
   }
   _etext = .;
diff --git a/cpu/mpc5xxx/config.mk b/cpu/mpc5xxx/config.mk
index b0ce2ee..3fd4fca 100644
--- a/cpu/mpc5xxx/config.mk
+++ b/cpu/mpc5xxx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC5xxx -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/cpu/mpc5xxx/u-boot-customlayout.lds b/cpu/mpc5xxx/u-boot-customlayout.lds
index 9563690..c340086 100644
--- a/cpu/mpc5xxx/u-boot-customlayout.lds
+++ b/cpu/mpc5xxx/u-boot-customlayout.lds
@@ -65,7 +65,6 @@ SECTIONS
     common/env_embedded.o        (.ppcenv)
 
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc5xxx/u-boot.lds b/cpu/mpc5xxx/u-boot.lds
index a6d4ff3..7fe1e95 100644
--- a/cpu/mpc5xxx/u-boot.lds
+++ b/cpu/mpc5xxx/u-boot.lds
@@ -54,7 +54,6 @@ SECTIONS
   {
     cpu/mpc5xxx/start.o	(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc8220/config.mk b/cpu/mpc8220/config.mk
index 5819048..b08ac11 100644
--- a/cpu/mpc8220/config.mk
+++ b/cpu/mpc8220/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC8220 -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/cpu/mpc8220/u-boot.lds b/cpu/mpc8220/u-boot.lds
index 436423c..4400e60 100644
--- a/cpu/mpc8220/u-boot.lds
+++ b/cpu/mpc8220/u-boot.lds
@@ -54,7 +54,6 @@ SECTIONS
   {
     cpu/mpc8220/start.o	(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc824x/config.mk b/cpu/mpc824x/config.mk
index b607fee..4a41239 100644
--- a/cpu/mpc824x/config.mk
+++ b/cpu/mpc824x/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC824X -ffixed-r2 -mstring -mcpu=603e -msoft-float
 
diff --git a/cpu/mpc824x/u-boot.lds b/cpu/mpc824x/u-boot.lds
index 46f7087..0eac48f 100644
--- a/cpu/mpc824x/u-boot.lds
+++ b/cpu/mpc824x/u-boot.lds
@@ -54,7 +54,6 @@ SECTIONS
   {
     cpu/mpc824x/start.o		(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc8260/config.mk b/cpu/mpc8260/config.mk
index 2cb0270..156c627 100644
--- a/cpu/mpc8260/config.mk
+++ b/cpu/mpc8260/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_8260 -DCONFIG_CPM2 -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/cpu/mpc8260/u-boot.lds b/cpu/mpc8260/u-boot.lds
index b3a103d..c777cf9 100644
--- a/cpu/mpc8260/u-boot.lds
+++ b/cpu/mpc8260/u-boot.lds
@@ -54,7 +54,6 @@ SECTIONS
   {
     cpu/mpc8260/start.o		(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc83xx/config.mk b/cpu/mpc83xx/config.mk
index d619426..0301cce 100644
--- a/cpu/mpc83xx/config.mk
+++ b/cpu/mpc83xx/config.mk
@@ -20,7 +20,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC83xx -DCONFIG_E300 \
 			-ffixed-r2 -msoft-float
diff --git a/cpu/mpc83xx/u-boot.lds b/cpu/mpc83xx/u-boot.lds
index 7d57ee4..c84d4b0 100644
--- a/cpu/mpc83xx/u-boot.lds
+++ b/cpu/mpc83xx/u-boot.lds
@@ -52,7 +52,6 @@ SECTIONS
   {
     cpu/mpc83xx/start.o	(.text)
     *(.text)
-    *(.fixup)
     *(.got1)
     . = ALIGN(16);
     *(.eh_frame)
diff --git a/cpu/mpc85xx/config.mk b/cpu/mpc85xx/config.mk
index beb3514..dd0b7fa 100644
--- a/cpu/mpc85xx/config.mk
+++ b/cpu/mpc85xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -ffixed-r2 -Wa,-me500 -msoft-float -mno-string
 PLATFORM_CPPFLAGS +=$(call cc-option,-mno-spe)
diff --git a/cpu/mpc85xx/u-boot.lds b/cpu/mpc85xx/u-boot.lds
index ec47871..a347cd1 100644
--- a/cpu/mpc85xx/u-boot.lds
+++ b/cpu/mpc85xx/u-boot.lds
@@ -62,7 +62,6 @@ SECTIONS
   .text      :
   {
     *(.text)
-    *(.fixup)
     *(.got1)
    } :text
     _etext = .;
diff --git a/cpu/mpc86xx/config.mk b/cpu/mpc86xx/config.mk
index 13da2cf..86ac904 100644
--- a/cpu/mpc86xx/config.mk
+++ b/cpu/mpc86xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -ffixed-r2 -mstring
 PLATFORM_CPPFLAGS += -maltivec -mabi=altivec -msoft-float
diff --git a/cpu/mpc8xx/config.mk b/cpu/mpc8xx/config.mk
index 2b3d545..4ad773a 100644
--- a/cpu/mpc8xx/config.mk
+++ b/cpu/mpc8xx/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 
 PLATFORM_CPPFLAGS += -DCONFIG_8xx -ffixed-r2 -mstring -mcpu=860 -msoft-float
diff --git a/cpu/ppc4xx/config.mk b/cpu/ppc4xx/config.mk
index 00ad39b..e75f783 100644
--- a/cpu/ppc4xx/config.mk
+++ b/cpu/ppc4xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
+PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi -mrelocatable
 PLATFORM_CPPFLAGS += -DCONFIG_4xx -ffixed-r2 -mstring -msoft-float
 
 cfg=$(shell grep configs $(OBJTREE)/include/config.h | sed 's/.*<\(configs.*\)>/\1/')
diff --git a/include/common.h b/include/common.h
index f7c93bf..394bfa4 100644
--- a/include/common.h
+++ b/include/common.h
@@ -35,6 +35,7 @@ typedef volatile unsigned short vu_short;
 typedef volatile unsigned char	vu_char;
 
 #include <config.h>
+#define CONFIG_RELOC_FIXUP_WORKS
 #include <linux/bitops.h>
 #include <linux/types.h>
 #include <linux/string.h>
-- 
1.6.2.1

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Wolfgang Denk]

Dear Peter Tyser,

In message <1252709159-22326-1-git-send-email-ptyser@xes-inc.com> you wrote:
> ** This patch is only meant to allow others to test relocation, it
> should not be applied!! **
> 
> This patch is a quick hack to enable proper relocation on powerpc
> boards.  I tested on some mpc85xx-based boards.
> 
> I updated the common ppc config.mk and u-boot.lds in cpu/ as needed, but
> didn't bother to update board-specific ones.  CONFIG_RELOC_FIXUP_WORKS
> has also been hacked into common.h unconditionally.
> 
> So if you want to try out this patch, make sure that you
> 1. Remove the *(.fixup) entry from the text section in your board's
> linker script.
> 
> 2. Make sure your board's config.mk file includes:
> PLATFORM_RELFLAGS += -mrelocatable
> 
> I'm hoping that relocation will work for all powerpc boards assuming you
> use a semi-recent version.  I think at least gcc >= 3.4.6 (or maybe even
> 3.4.5) should work.
> 
> It'd be great if people could give feedback if this patch works for
> them, and if not, how their board breaks.

I have tested this patch on the following boards / tool chains:


Tool Chain:	ELDK 3.1.1	ELDK 4.0	ELDK 4.2
		gcc-3.3.3	gcc-4.0.0	gcc-4.2.2
Board:		binutils-2.14	binutils-2.16.1	binutils-2.17.50.0
TQM834x		OK		OK		OK
Canyonlands	NOK1		OK		OK
MPC5121ADS	OK		OK		OK
Haleakala	OK		OK		OK
Ocotea		NOK1		OK		OK

NOK1: build error because old compiler does not accept "-m440" option


So I would say this looks pretty good :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
That said, there may be good reasons for what you did beyond obsequi-
ous sycophantic parody. Perhaps you might be so kind as to elucidate.
         -- Tom Christiansen in <5ldjbm$jtk$1@csnews.cs.colorado.edu>

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

On Mon, 2009-09-14 at 23:26 +0200, Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <1252709159-22326-1-git-send-email-ptyser@xes-inc.com> you wrote:
> > ** This patch is only meant to allow others to test relocation, it
> > should not be applied!! **
> > 
> > This patch is a quick hack to enable proper relocation on powerpc
> > boards.  I tested on some mpc85xx-based boards.
> > 
> > I updated the common ppc config.mk and u-boot.lds in cpu/ as needed, but
> > didn't bother to update board-specific ones.  CONFIG_RELOC_FIXUP_WORKS
> > has also been hacked into common.h unconditionally.
> > 
> > So if you want to try out this patch, make sure that you
> > 1. Remove the *(.fixup) entry from the text section in your board's
> > linker script.
> > 
> > 2. Make sure your board's config.mk file includes:
> > PLATFORM_RELFLAGS += -mrelocatable
> > 
> > I'm hoping that relocation will work for all powerpc boards assuming you
> > use a semi-recent version.  I think at least gcc >= 3.4.6 (or maybe even
> > 3.4.5) should work.
> > 
> > It'd be great if people could give feedback if this patch works for
> > them, and if not, how their board breaks.
> 
> I have tested this patch on the following boards / tool chains:
> 
> 
> Tool Chain:	ELDK 3.1.1	ELDK 4.0	ELDK 4.2
> 		gcc-3.3.3	gcc-4.0.0	gcc-4.2.2
> Board:		binutils-2.14	binutils-2.16.1	binutils-2.17.50.0
> TQM834x		OK		OK		OK
> Canyonlands	NOK1		OK		OK
> MPC5121ADS	OK		OK		OK
> Haleakala	OK		OK		OK
> Ocotea		NOK1		OK		OK
> 
> NOK1: build error because old compiler does not accept "-m440" option
> 
> 
> So I would say this looks pretty good :-)

Great!  Thanks for the testing.  I'll submit a proper patch in this
window.  I'll also add a check for the .fixup section to catch those
compilers which don't generate it as Mike suggested in the original
relocation thread.

Best,
Peter

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

On Mon, 2009-09-14 at 23:26 +0200, Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <1252709159-22326-1-git-send-email-ptyser@xes-inc.com> you wrote:
> > ** This patch is only meant to allow others to test relocation, it
> > should not be applied!! **
> > 
> > This patch is a quick hack to enable proper relocation on powerpc
> > boards.  I tested on some mpc85xx-based boards.
> > 
> > I updated the common ppc config.mk and u-boot.lds in cpu/ as needed, but
> > didn't bother to update board-specific ones.  CONFIG_RELOC_FIXUP_WORKS
> > has also been hacked into common.h unconditionally.
> > 
> > So if you want to try out this patch, make sure that you
> > 1. Remove the *(.fixup) entry from the text section in your board's
> > linker script.
> > 
> > 2. Make sure your board's config.mk file includes:
> > PLATFORM_RELFLAGS += -mrelocatable
> > 
> > I'm hoping that relocation will work for all powerpc boards assuming you
> > use a semi-recent version.  I think at least gcc >= 3.4.6 (or maybe even
> > 3.4.5) should work.
> > 
> > It'd be great if people could give feedback if this patch works for
> > them, and if not, how their board breaks.
> 
> I have tested this patch on the following boards / tool chains:
> 
> 
> Tool Chain:	ELDK 3.1.1	ELDK 4.0	ELDK 4.2
> 		gcc-3.3.3	gcc-4.0.0	gcc-4.2.2
> Board:		binutils-2.14	binutils-2.16.1	binutils-2.17.50.0
> TQM834x		OK		OK		OK
> Canyonlands	NOK1		OK		OK
> MPC5121ADS	OK		OK		OK
> Haleakala	OK		OK		OK
> Ocotea		NOK1		OK		OK
> 
> NOK1: build error because old compiler does not accept "-m440" option

When preparing the ppc relocation patches I noticed that the gcc
-mrelocatable compiler flag increases the .reloc section by 3 or 4
Kbytes.  I did a compile test, and this increase pushes the ALPR board
back over 256K (it recently had the same size issue, see "ppc4xx: Remove
some features from ALPR to fit into 256k again").  No other boards
appear to break size-wise.

So I guess I had 2 questions:
1. Is enabling proper relocation worth the 3/4KB that will be added to
every ppc binary?  I personally think so as the manual relocation fixups
that currently litter the code can be removed and true relocation is
much less hokey in the long run.  X-ES's U-Boot binaries also are
generally much smaller than their allocated 512KB, so this increase
doesn't affect me much:)

2. Assuming we do want proper relocation, what should be done to
decrease the size of the ALPR binary?  Pieter had mentioned getting rid
of CONFIG_CMD_PCI was OK in a previous email thread.  Making this change
puts the ALPR binary at around 253KB.  I can roll this change in the
relocation fixup changeset if he is OK with it.

Thanks,
Peter

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

>
> When preparing the ppc relocation patches I noticed that the gcc
> -mrelocatable compiler flag increases the .reloc section by 3 or 4
> Kbytes.  I did a compile test, and this increase pushes the ALPR board
> back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> some features from ALPR to fit into 256k again").  No other boards
> appear to break size-wise.
>
> So I guess I had 2 questions:
> 1. Is enabling proper relocation worth the 3/4KB that will be added to
> every ppc binary?  I personally think so as the manual relocation fixups
> that currently litter the code can be removed and true relocation is
> much less hokey in the long run.  X-ES's U-Boot binaries also are
> generally much smaller than their allocated 512KB, so this increase
> doesn't affect me much:)

You can get some of this space back by just #ifdef:ing out the manual relocation
code. Removing it completely is OK by me though.

The size can be further decreased by looking over the use of global data:
- Some tables can be replaced by code.
- Combine several global variables into one struct variable.
- reducing string literals

One day we can fit the whole relocation table into built-in CPU memory, hopefully
that will make it possible to make u-boot a true PIC exe

 Jocke

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFF85C09CB.2A4A1999-ONC1257634.0025DF59-C1257634.00270B15@transmode.se> you wrote:
>
> One day we can fit the whole relocation table into built-in CPU memory, hopefully
> that will make it possible to make u-boot a true PIC exe

Why is the former the prerequisite for the latter?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
This is now.  Later is later.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 17/09/2009 09:50:51:
>
> Dear Joakim Tjernlund,
>
> In message <OFF85C09CB.2A4A1999-ONC1257634.0025DF59-C1257634.
> 00270B15 at transmode.se> you wrote:
> >
> > One day we can fit the whole relocation table into built-in CPU memory, hopefully
> > that will make it possible to make u-boot a true PIC exe
>
> Why is the former the prerequisite for the latter?

Some time ago I looked into making u-boot PIC and I came to the conclusion
that one must be able to modify the GOT table before we have relocated to RAM.
Perhaps there is another way but didn't see it at the time, maybe there is some
"hidden" option to gcc that I missed.
The main problem are all those string literals, these are in the fixup table
and one wants to use printf and friends before relocation to RAM.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFA0E17029.E568D101-ONC1257634.002EFBFD-C1257634.002F97F1@transmode.se> you wrote:
>
> > > One day we can fit the whole relocation table into built-in CPU memory, hopefully
> > > that will make it possible to make u-boot a true PIC exe
> >
> > Why is the former the prerequisite for the latter?
> 
> Some time ago I looked into making u-boot PIC and I came to the conclusion
> that one must be able to modify the GOT table before we have relocated to RAM.
> Perhaps there is another way but didn't see it at the time, maybe there is some
> "hidden" option to gcc that I missed.

But why has the GOT table to fit "into built-in CPU memory"? When we
are about to relocate U-Boot to RAM, we already have RAM working. So
we should also be able to copy the GOT table to RAM and modify it
there as needed before we use it?

> The main problem are all those string literals, these are in the fixup table
> and one wants to use printf and friends before relocation to RAM.

Maybe a similar approach is usable here?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Unsichtbar macht sich die Dummheit, indem sie immer  gr??ere  Ausma?e
annimmt.                             -- Bertold Brecht: Der Tui-Roman

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 17/09/2009 12:15:42:
>
> Dear Joakim Tjernlund,
>
> In message <OFA0E17029.E568D101-ONC1257634.002EFBFD-C1257634.
> 002F97F1 at transmode.se> you wrote:
> >
> > > > One day we can fit the whole relocation table into built-in CPU memory, hopefully
> > > > that will make it possible to make u-boot a true PIC exe
> > >
> > > Why is the former the prerequisite for the latter?
> >
> > Some time ago I looked into making u-boot PIC and I came to the conclusion
> > that one must be able to modify the GOT table before we have relocated to RAM.
> > Perhaps there is another way but didn't see it at the time, maybe there is some
> > "hidden" option to gcc that I missed.
>
> But why has the GOT table to fit "into built-in CPU memory"? When we
> are about to relocate U-Boot to RAM, we already have RAM working. So
> we should also be able to copy the GOT table to RAM and modify it
> there as needed before we use it?

Before you get RAM working there are lots of references to global data and
string literals. These point to their link address so if you have loaded
u-boot at a different address than you link address you have to adjust for
the offset. Hard to do that when GOT is in flash :(

>
> > The main problem are all those string literals, these are in the fixup table
> > and one wants to use printf and friends before relocation to RAM.
>
> Maybe a similar approach is usable here?

Yes, fixups and GOT are the same in this regard.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF680476D5.A9D9D259-ONC1257634.00449AC4-C1257634.0045177E@transmode.se> you wrote:
>
> > But why has the GOT table to fit "into built-in CPU memory"? When we
> > are about to relocate U-Boot to RAM, we already have RAM working. So
> > we should also be able to copy the GOT table to RAM and modify it
> > there as needed before we use it?
> 
> Before you get RAM working there are lots of references to global data and
> string literals. These point to their link address so if you have loaded
> u-boot at a different address than you link address you have to adjust for
> the offset. Hard to do that when GOT is in flash :(

Ah, I finally GOT you :-)

Hm... but being able to load (and start) U-Boot from an arbitrary RAM
location would be a big benefit, too, even if we still have to copy it
to the correct (linked) address when storing it in flash.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 17/09/2009 14:53:59:
>
> Dear Joakim Tjernlund,

Dear me :)

>
> In message <OF680476D5.A9D9D259-ONC1257634.00449AC4-C1257634.
> 0045177E at transmode.se> you wrote:
> >
> > > But why has the GOT table to fit "into built-in CPU memory"? When we
> > > are about to relocate U-Boot to RAM, we already have RAM working. So
> > > we should also be able to copy the GOT table to RAM and modify it
> > > there as needed before we use it?
> >
> > Before you get RAM working there are lots of references to global data and
> > string literals. These point to their link address so if you have loaded
> > u-boot at a different address than you link address you have to adjust for
> > the offset. Hard to do that when GOT is in flash :(
>
> Ah, I finally GOT you :-)

*LOL*

>
> Hm... but being able to load (and start) U-Boot from an arbitrary RAM
> location would be a big benefit, too, even if we still have to copy it
> to the correct (linked) address when storing it in flash.

Naa, true PIC is what I want.
What is really missing is an option to gcc telling it to use a register
as GOT base. Something like *assume* GOT is r30 instead of calculating it
from __GLOBAL_OFFSET_TABLE__ in every function.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

On Thu, 2009-09-17 at 09:06 +0200, Joakim Tjernlund wrote:
> >
> > When preparing the ppc relocation patches I noticed that the gcc
> > -mrelocatable compiler flag increases the .reloc section by 3 or 4
> > Kbytes.  I did a compile test, and this increase pushes the ALPR board
> > back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> > some features from ALPR to fit into 256k again").  No other boards
> > appear to break size-wise.
> >
> > So I guess I had 2 questions:
> > 1. Is enabling proper relocation worth the 3/4KB that will be added to
> > every ppc binary?  I personally think so as the manual relocation fixups
> > that currently litter the code can be removed and true relocation is
> > much less hokey in the long run.  X-ES's U-Boot binaries also are
> > generally much smaller than their allocated 512KB, so this increase
> > doesn't affect me much:)
> 
> You can get some of this space back by just #ifdef:ing out the manual relocation
> code. Removing it completely is OK by me though.

The original patchset I had planned on submitting completely removed all
PPC-specific manual relocation fixups, but didn't do anything with the
references to gd->reloc_off in common files.  The thought was that we
could get other architectures to properly relocate, then get rid of
gd->reloc_off globally.  Otherwise there's going to be a fair number of
#ifdef CONFIG_RELOC_FIXUP_WORKS littering the code until all arches
support proper relocation which is a bit ugly.

With all PPC-specific relocation manual fixups removed, the ALPR still
didn't fit.  However, I just removed all relocation fixups in the common
fpga code as well as added some #ifdef CONFIG_RELOC_FIXUP_WORKS in
common code, and now the ALPR fits in its designated 256KB.  It looks to
be 1.8KB larger than the original, non-relocatable code.

I'll submit this patch for review shortly.  I'm assuming people are OK
with the 1.8KB image size increase?  Perhaps some of Jocke's suggestions
below can decrease the size as well.

Best,
Peter

> The size can be further decreased by looking over the use of global data:
> - Some tables can be replaced by code.
> - Combine several global variables into one struct variable.
> - reducing string literals
> 
> One day we can fit the whole relocation table into built-in CPU memory, hopefully
> that will make it possible to make u-boot a true PIC exe
> 
>  Jocke
> 

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Graeme Russ]

On Thu, Sep 17, 2009 at 10:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Joakim Tjernlund,
>
> In message <OF680476D5.A9D9D259-ONC1257634.00449AC4-C1257634.0045177E@transmode.se> you wrote:
>>
>> > But why has the GOT table to fit "into built-in CPU memory"? When we
>> > are about to relocate U-Boot to RAM, we already have RAM working. So
>> > we should also be able to copy the GOT table to RAM and modify it
>> > there as needed before we use it?
>>
>> Before you get RAM working there are lots of references to global data and
>> string literals. These point to their link address so if you have loaded
>> u-boot at a different address than you link address you have to adjust for
>> the offset. Hard to do that when GOT is in flash :(
>
> Ah, I finally GOT you :-)
>
> Hm... but being able to load (and start) U-Boot from an arbitrary RAM
> location would be a big benefit, too, even if we still have to copy it
> to the correct (linked) address when storing it in flash.

Things are starting to make a lot of sense to me now - I thought all this
relocation work on PPC was full relocation, but I see now that it only
relates to relocation of code segments.

The biggest problem (for me anyway) in not relocating data segments
(particularly .rodata) is that any time you attempt to lock or erase the
flash, U-Boot can crash. This prevents (again, for me) updating the U-Boot
image in flash on-board from within U-Boot itself.

I am working on full relocation on the x86 port (about 85% done) - It is
a real pita because not only do you need to update the GOT, you also need
to worry about any other references not known to the linker - Jumping to
C interupt handlers from inline asm is my big headache at the moment

I hope to have a patch out in the next few weeks for x86 which will
illustrate the concepts of full relocation.

Regards,

G

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Graeme Russ <graeme.russ@gmail.com> wrote on 17/09/2009 23:57:56:
>
> On Thu, Sep 17, 2009 at 10:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Joakim Tjernlund,
> >
> > In message <OF680476D5.A9D9D259-ONC1257634.00449AC4-C1257634.
> 0045177E at transmode.se> you wrote:
> >>
> >> > But why has the GOT table to fit "into built-in CPU memory"? When we
> >> > are about to relocate U-Boot to RAM, we already have RAM working. So
> >> > we should also be able to copy the GOT table to RAM and modify it
> >> > there as needed before we use it?
> >>
> >> Before you get RAM working there are lots of references to global data and
> >> string literals. These point to their link address so if you have loaded
> >> u-boot at a different address than you link address you have to adjust for
> >> the offset. Hard to do that when GOT is in flash :(
> >
> > Ah, I finally GOT you :-)
> >
> > Hm... but being able to load (and start) U-Boot from an arbitrary RAM
> > location would be a big benefit, too, even if we still have to copy it
> > to the correct (linked) address when storing it in flash.
>
> Things are starting to make a lot of sense to me now - I thought all this
> relocation work on PPC was full relocation, but I see now that it only
> relates to relocation of code segments.

Nope, this patch does full relocation but must start from flash at its
linked address. Then it relocates to RAM and bring everything into RAM.
PCC has a "fixup" (-mrelocate) option that emits relocation entries for
strings and global function pointers etc.

>
> The biggest problem (for me anyway) in not relocating data segments
> (particularly .rodata) is that any time you attempt to lock or erase the
> flash, U-Boot can crash. This prevents (again, for me) updating the U-Boot
> image in flash on-board from within U-Boot itself.

yes, although it does not seem to happen often.

>
> I am working on full relocation on the x86 port (about 85% done) - It is
> a real pita because not only do you need to update the GOT, you also need
> to worry about any other references not known to the linker - Jumping to
> C interupt handlers from inline asm is my big headache at the moment

yeah, you will need to emit relocation info for those function pointers
I guess. As I remember PPC has something similar, look in start.S

  Jocke

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Peter Tyser <ptyser@xes-inc.com> wrote on 17/09/2009 19:29:18:

> From:
>
> Peter Tyser <ptyser@xes-inc.com>
>
> To:
>
> Joakim Tjernlund <joakim.tjernlund@transmode.se>
>
> Cc:
>
> pieter.voorthuijsen at prodrive.nl, u-boot at lists.denx.de, Wolfgang Denk <wd@denx.de>
>
> Date:
>
> 17/09/2009 19:29
>
> Subject:
>
> Re: [U-Boot] [TESTING PATCH] ppc: Relocation test patch
>
> On Thu, 2009-09-17 at 09:06 +0200, Joakim Tjernlund wrote:
> > >
> > > When preparing the ppc relocation patches I noticed that the gcc
> > > -mrelocatable compiler flag increases the .reloc section by 3 or 4
> > > Kbytes.  I did a compile test, and this increase pushes the ALPR board
> > > back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> > > some features from ALPR to fit into 256k again").  No other boards
> > > appear to break size-wise.
> > >
> > > So I guess I had 2 questions:
> > > 1. Is enabling proper relocation worth the 3/4KB that will be added to
> > > every ppc binary?  I personally think so as the manual relocation fixups
> > > that currently litter the code can be removed and true relocation is
> > > much less hokey in the long run.  X-ES's U-Boot binaries also are
> > > generally much smaller than their allocated 512KB, so this increase
> > > doesn't affect me much:)
> >
> > You can get some of this space back by just #ifdef:ing out the manual relocation
> > code. Removing it completely is OK by me though.
>
> The original patchset I had planned on submitting completely removed all
> PPC-specific manual relocation fixups, but didn't do anything with the
> references to gd->reloc_off in common files.  The thought was that we
> could get other architectures to properly relocate, then get rid of
> gd->reloc_off globally.  Otherwise there's going to be a fair number of
> #ifdef CONFIG_RELOC_FIXUP_WORKS littering the code until all arches
> support proper relocation which is a bit ugly.
>
> With all PPC-specific relocation manual fixups removed, the ALPR still
> didn't fit.  However, I just removed all relocation fixups in the common
> fpga code as well as added some #ifdef CONFIG_RELOC_FIXUP_WORKS in
> common code, and now the ALPR fits in its designated 256KB.  It looks to
> be 1.8KB larger than the original, non-relocatable code.
>
> I'll submit this patch for review shortly.  I'm assuming people are OK
> with the 1.8KB image size increase?  Perhaps some of Jocke's suggestions
> below can decrease the size as well.

I remembered one thing, the reloc asm has a bug, one should not
relocate NULL values, pasting in an email from me sent to the  list
some time ago about this:

On Thu, 2008-12-04 at 13:35 +0100, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> introduce 3 new weak functions board_bdinfo, cpu_bdinfo and soc_bdinfo to allow
> board, cpu and soc to print more information
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> diff with V3
> rename cpu_bdinfo to soc_bdinfo for soc
>
> Best Regards,
> J.

Since you are starting to use weak function I think you really need to fix the relocation procedure not to relocate
NULL values too. Othervise you risk running into hard to debug problems, possibly one should do the same
for __eabi_uconvert(). The function below could be written a bit cleaner though:

void __eabi_convert(unsigned long *low, unsigned long *high,
                    unsigned long addend)
{
        unsigned long len = high - low, val;

	for(--low; len; --len) {
		val = *++low;
		if (!val)
			continue;
                *low = val + addend;
	}
}

void __eabi_uconvert(unsigned long *low, unsigned long *high,
                     unsigned long addend)
{
        unsigned long len = high - low, val, *v2p;

	for(--low; len; --len) {
                val = *++low;
                val += addend;
                v2p = (unsigned long *)val;
                *low = val;
		*v2p += added;
	}
}

Pasting part of an earlier mail:

And you need to fix the relocation not to relocate NULL values, see
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/eabi.asm?rev=1.13&content-type=text/x-cvsweb-markup
look for __eabi_uconvert.

For fun I once tried to rewrite these functions i C, not tested though:

void __eabi_convert(unsigned long *low, unsigned long *high,
                    unsigned long addend)
{
        unsigned long len = high - low, val;
        if (!len)
                return;
        low--;
        do {
                val = *++low;
                if (!val)
                        continue;
                *low = val + addend;
        } while(--len);
}

void __eabi_uconvert(unsigned long *low, unsigned long *high,
                     unsigned long addend)
{
        unsigned long len = high - low, val, val2, *v2p;
        if (!len)
                return;
        low--;
        do {
                val = *++low;
                val += addend;
                v2p = (unsigned long *)val;
                *low = val;
                val2 = *v2p;
                val2 += addend;
                *v2p = val2;
        } while(--len);
}

void __eabi_uconvert_org(unsigned long *low, unsigned long *high,
                     unsigned long addend)
{
        unsigned long len = high - low, val, val2, *v2p;
        if (!len)
                return;
        low--;
        do {
                val = *++low;
                val += addend;
                v2p = (unsigned long *)val;
                val2 = *v2p;
                *low = val;
                val2 += addend;
                *v2p = val2;
        } while(--len);
}

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

> > On Thu, 2009-09-17 at 09:06 +0200, Joakim Tjernlund wrote:
> > > >
> > > > When preparing the ppc relocation patches I noticed that the gcc
> > > > -mrelocatable compiler flag increases the .reloc section by 3 or 4
> > > > Kbytes.  I did a compile test, and this increase pushes the ALPR board
> > > > back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> > > > some features from ALPR to fit into 256k again").  No other boards
> > > > appear to break size-wise.
> > > >
> > > > So I guess I had 2 questions:
> > > > 1. Is enabling proper relocation worth the 3/4KB that will be added to
> > > > every ppc binary?  I personally think so as the manual relocation fixups
> > > > that currently litter the code can be removed and true relocation is
> > > > much less hokey in the long run.  X-ES's U-Boot binaries also are
> > > > generally much smaller than their allocated 512KB, so this increase
> > > > doesn't affect me much:)
> > >
> > > You can get some of this space back by just #ifdef:ing out the manual relocation
> > > code. Removing it completely is OK by me though.
> >
> > The original patchset I had planned on submitting completely removed all
> > PPC-specific manual relocation fixups, but didn't do anything with the
> > references to gd->reloc_off in common files.  The thought was that we
> > could get other architectures to properly relocate, then get rid of
> > gd->reloc_off globally.  Otherwise there's going to be a fair number of
> > #ifdef CONFIG_RELOC_FIXUP_WORKS littering the code until all arches
> > support proper relocation which is a bit ugly.
> >
> > With all PPC-specific relocation manual fixups removed, the ALPR still
> > didn't fit.  However, I just removed all relocation fixups in the common
> > fpga code as well as added some #ifdef CONFIG_RELOC_FIXUP_WORKS in
> > common code, and now the ALPR fits in its designated 256KB.  It looks to
> > be 1.8KB larger than the original, non-relocatable code.
> >
> > I'll submit this patch for review shortly.  I'm assuming people are OK
> > with the 1.8KB image size increase?  Perhaps some of Jocke's suggestions
> > below can decrease the size as well.
> 
> I remembered one thing, the reloc asm has a bug, one should not
> relocate NULL values, pasting in an email from me sent to the  list
> some time ago about this:

Hi Jocke,
Do you have a C snippet that would bring this issue out?  I would assume
gcc would not emit relocation fixup information for NULL values.
Variables initialized to NULL should be put in the bss segment, which
just get zeroed out, not relocated.  For example, I just tested:

char *reloc_test = NULL;

/* After relocation to RAM */
printf("RAM test_reloc @ %p\n", test_reloc);

Which prints out the correct address:
RAM test_reloc @ (null)

Let me know if I'm missing something.

Best,
Peter

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Peter Tyser <ptyser@xes-inc.com> wrote on 18/09/2009 16:28:35:
>
>
> > > On Thu, 2009-09-17 at 09:06 +0200, Joakim Tjernlund wrote:
> > > > >
> > > > > When preparing the ppc relocation patches I noticed that the gcc
> > > > > -mrelocatable compiler flag increases the .reloc section by 3 or 4
> > > > > Kbytes.  I did a compile test, and this increase pushes the ALPR board
> > > > > back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> > > > > some features from ALPR to fit into 256k again").  No other boards
> > > > > appear to break size-wise.
> > > > >
> > > > > So I guess I had 2 questions:
> > > > > 1. Is enabling proper relocation worth the 3/4KB that will be added to
> > > > > every ppc binary?  I personally think so as the manual relocation fixups
> > > > > that currently litter the code can be removed and true relocation is
> > > > > much less hokey in the long run.  X-ES's U-Boot binaries also are
> > > > > generally much smaller than their allocated 512KB, so this increase
> > > > > doesn't affect me much:)
> > > >
> > > > You can get some of this space back by just #ifdef:ing out the manual relocation
> > > > code. Removing it completely is OK by me though.
> > >
> > > The original patchset I had planned on submitting completely removed all
> > > PPC-specific manual relocation fixups, but didn't do anything with the
> > > references to gd->reloc_off in common files.  The thought was that we
> > > could get other architectures to properly relocate, then get rid of
> > > gd->reloc_off globally.  Otherwise there's going to be a fair number of
> > > #ifdef CONFIG_RELOC_FIXUP_WORKS littering the code until all arches
> > > support proper relocation which is a bit ugly.
> > >
> > > With all PPC-specific relocation manual fixups removed, the ALPR still
> > > didn't fit.  However, I just removed all relocation fixups in the common
> > > fpga code as well as added some #ifdef CONFIG_RELOC_FIXUP_WORKS in
> > > common code, and now the ALPR fits in its designated 256KB.  It looks to
> > > be 1.8KB larger than the original, non-relocatable code.
> > >
> > > I'll submit this patch for review shortly.  I'm assuming people are OK
> > > with the 1.8KB image size increase?  Perhaps some of Jocke's suggestions
> > > below can decrease the size as well.
> >
> > I remembered one thing, the reloc asm has a bug, one should not
> > relocate NULL values, pasting in an email from me sent to the  list
> > some time ago about this:
>
> Hi Jocke,
> Do you have a C snippet that would bring this issue out?  I would assume
> gcc would not emit relocation fixup information for NULL values.
> Variables initialized to NULL should be put in the bss segment, which
> just get zeroed out, not relocated.

Sorry, I don't have an example. Just a guess, weak function references:

void weak_fun(void) __attribute__ ((weak));
if (weak_fun)
	weak_fun();

On the other hand it is clear that gcc test for NULL and skips
the reloc. It is obvious why, NULL is a absolute value and should
stay that way even after relocation.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

On Fri, 2009-09-18 at 16:52 +0200, Joakim Tjernlund wrote:
> Peter Tyser <ptyser@xes-inc.com> wrote on 18/09/2009 16:28:35:
> >
> >
> > > > On Thu, 2009-09-17 at 09:06 +0200, Joakim Tjernlund wrote:
> > > > > >
> > > > > > When preparing the ppc relocation patches I noticed that the gcc
> > > > > > -mrelocatable compiler flag increases the .reloc section by 3 or 4
> > > > > > Kbytes.  I did a compile test, and this increase pushes the ALPR board
> > > > > > back over 256K (it recently had the same size issue, see "ppc4xx: Remove
> > > > > > some features from ALPR to fit into 256k again").  No other boards
> > > > > > appear to break size-wise.
> > > > > >
> > > > > > So I guess I had 2 questions:
> > > > > > 1. Is enabling proper relocation worth the 3/4KB that will be added to
> > > > > > every ppc binary?  I personally think so as the manual relocation fixups
> > > > > > that currently litter the code can be removed and true relocation is
> > > > > > much less hokey in the long run.  X-ES's U-Boot binaries also are
> > > > > > generally much smaller than their allocated 512KB, so this increase
> > > > > > doesn't affect me much:)
> > > > >
> > > > > You can get some of this space back by just #ifdef:ing out the manual relocation
> > > > > code. Removing it completely is OK by me though.
> > > >
> > > > The original patchset I had planned on submitting completely removed all
> > > > PPC-specific manual relocation fixups, but didn't do anything with the
> > > > references to gd->reloc_off in common files.  The thought was that we
> > > > could get other architectures to properly relocate, then get rid of
> > > > gd->reloc_off globally.  Otherwise there's going to be a fair number of
> > > > #ifdef CONFIG_RELOC_FIXUP_WORKS littering the code until all arches
> > > > support proper relocation which is a bit ugly.
> > > >
> > > > With all PPC-specific relocation manual fixups removed, the ALPR still
> > > > didn't fit.  However, I just removed all relocation fixups in the common
> > > > fpga code as well as added some #ifdef CONFIG_RELOC_FIXUP_WORKS in
> > > > common code, and now the ALPR fits in its designated 256KB.  It looks to
> > > > be 1.8KB larger than the original, non-relocatable code.
> > > >
> > > > I'll submit this patch for review shortly.  I'm assuming people are OK
> > > > with the 1.8KB image size increase?  Perhaps some of Jocke's suggestions
> > > > below can decrease the size as well.
> > >
> > > I remembered one thing, the reloc asm has a bug, one should not
> > > relocate NULL values, pasting in an email from me sent to the  list
> > > some time ago about this:
> >
> > Hi Jocke,
> > Do you have a C snippet that would bring this issue out?  I would assume
> > gcc would not emit relocation fixup information for NULL values.
> > Variables initialized to NULL should be put in the bss segment, which
> > just get zeroed out, not relocated.
> 
> Sorry, I don't have an example. Just a guess, weak function references:
> 
> void weak_fun(void) __attribute__ ((weak));
> if (weak_fun)
> 	weak_fun();

Using default weak functions as well as overridden weak functions both
definitely work.  So the pointers must be being updated correctly.  I
guess I'm not sure where specifically a problem could arise.  Let me
know if you have any additional details.  I'm hoping to send the patches
out later today, maybe some review/testing will make things clearer.

Best,
Peter

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Peter Tyser <ptyser@xes-inc.com> wrote on 18/09/2009 17:21:57:
> >
> > Sorry, I don't have an example. Just a guess, weak function references:
> >
> > void weak_fun(void) __attribute__ ((weak));
> > if (weak_fun)
> >    weak_fun();
>
> Using default weak functions as well as overridden weak functions both
> definitely work.  So the pointers must be being updated correctly.  I
> guess I'm not sure where specifically a problem could arise.  Let me
> know if you have any additional details.  I'm hoping to send the patches
> out later today, maybe some review/testing will make things clearer.

This does not work:

void weak_fun(void) __attribute__ ((weak));
printf("weak_fun:%p\n", weak_fun);

prints "weak 17f9c000" after relocation
for me, should be NULL when weak_fun is undefined.

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Peter Tyser]

> > > Sorry, I don't have an example. Just a guess, weak function references:
> > >
> > > void weak_fun(void) __attribute__ ((weak));
> > > if (weak_fun)
> > >    weak_fun();
> >
> > Using default weak functions as well as overridden weak functions both
> > definitely work.  So the pointers must be being updated correctly.  I
> > guess I'm not sure where specifically a problem could arise.  Let me
> > know if you have any additional details.  I'm hoping to send the patches
> > out later today, maybe some review/testing will make things clearer.
> 
> This does not work:
> 
> void weak_fun(void) __attribute__ ((weak));
> printf("weak_fun:%p\n", weak_fun);
> 
> prints "weak 17f9c000" after relocation
> for me, should be NULL when weak_fun is undefined.

Ahh, I see.  I see the same thing.  In general U-Boot declares weak
functions by either using the 'alias' attribute:

static int __def_eth_init(bd_t *bis)
{
	return -1;
}
int cpu_eth_init(bd_t *bis) __attribute__((weak, alias("__def_eth_init")));

or by declaring a function as weak:

void __attribute__((weak)) _machine_restart(void)
{
}

Both these scenarios work with the current relocation fixup scheme.
What is a real world scenario (such as your example) when someone would
declare a weak function, but not actually implement a default.  Doesn't
that defeat the purpose of having a weak function in the first place?
Eg why would someone use your example of:

void weak_fun(void) __attribute__ ((weak));
...
	if (weak_fun)
		weak_fun();
...

over:

void weak_fun(void) __attribute__ ((weak))
{
};
...
	weak_fun();
...
(or the alias implementation)

I'm trying to grasp the limitations of the current relocation mechanism
as I'm afraid I don't have time to dig through all PPC architectures'
start.S files to fix their relocation code right now:)

Thanks,
Peter

[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Peter Tyser <ptyser@xes-inc.com> wrote on 18/09/2009 18:24:48:
>
>
> > > > Sorry, I don't have an example. Just a guess, weak function references:
> > > >
> > > > void weak_fun(void) __attribute__ ((weak));
> > > > if (weak_fun)
> > > >    weak_fun();
> > >
> > > Using default weak functions as well as overridden weak functions both
> > > definitely work.  So the pointers must be being updated correctly.  I
> > > guess I'm not sure where specifically a problem could arise.  Let me
> > > know if you have any additional details.  I'm hoping to send the patches
> > > out later today, maybe some review/testing will make things clearer.
> >
> > This does not work:
> >
> > void weak_fun(void) __attribute__ ((weak));
> > printf("weak_fun:%p\n", weak_fun);
> >
> > prints "weak 17f9c000" after relocation
> > for me, should be NULL when weak_fun is undefined.
>
> Ahh, I see.  I see the same thing.  In general U-Boot declares weak
> functions by either using the 'alias' attribute:
>
> static int __def_eth_init(bd_t *bis)
> {
>    return -1;
> }
> int cpu_eth_init(bd_t *bis) __attribute__((weak, alias("__def_eth_init")));
>
> or by declaring a function as weak:
>
> void __attribute__((weak)) _machine_restart(void)
> {
> }
>
> Both these scenarios work with the current relocation fixup scheme.
> What is a real world scenario (such as your example) when someone would
> declare a weak function, but not actually implement a default.  Doesn't
> that defeat the purpose of having a weak function in the first place?
> Eg why would someone use your example of:
>
> void weak_fun(void) __attribute__ ((weak));
> ...
>    if (weak_fun)
>       weak_fun();
> ...
>
> over:
>
> void weak_fun(void) __attribute__ ((weak))
> {
> };
> ...
>    weak_fun();
> ...
> (or the alias implementation)

Instead of defining empty board function you could have in generic code
if (board_init_whatever)
   board_init_whatever()

>
> I'm trying to grasp the limitations of the current relocation mechanism
> as I'm afraid I don't have time to dig through all PPC architectures'
> start.S files to fix their relocation code right now:)

Sure, I am not saying that you need to do this to make current u-boot
work. I am just saying that the reloc funs are not complete and will
bug if someone in the future wants to use weak functions( or something else
that needs the same) in the above manner, it won't work. So these should be fixed at
some point but it should be a separate patch as it has nothing to do with your reloc patch,
it is broken as is even without your patch. I just mentioned it because I remembered it now
that you were working on relocation in general. I also think the relocation funs should be
in C ( as I posted ) as this is much easier to understand. Instead of just fixing all
the reloc assembler one should replace these with calls to generic C code.
If you want to work on making u-boot fully relocatable, I have some hints for you.

 Jocke