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

[Peter Tyser <ptyser@xes-inc.com>]

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