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

[Joakim Tjernlund]

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

> From:
>
> Peter Tyser <ptyser at xes-inc.com>
>
> To:
>
> Joakim Tjernlund <joakim.tjernlund at transmode.se>
>
> Cc:
>
> pieter.voorthuijsen at prodrive.nl, u-boot at lists.denx.de, Wolfgang Denk <wd at 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 at 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);
}