All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Tyser <ptyser@xes-inc.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [TESTING PATCH] ppc: Relocation test patch
Date: Fri, 18 Sep 2009 10:21:57 -0500	[thread overview]
Message-ID: <1253287317.617.459.camel@localhost.localdomain> (raw)
In-Reply-To: <OF801CA4E8.BA4014D7-ONC1257635.0050D560-C1257635.0051ACB5@transmode.se>

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

  reply	other threads:[~2009-09-18 15:21 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-11 22:45 [U-Boot] [TESTING PATCH] ppc: Relocation test patch Peter Tyser
2009-09-14 21:26 ` Wolfgang Denk
2009-09-14 22:54   ` Peter Tyser
2009-09-16 23:20   ` Peter Tyser
2009-09-17  7:06     ` Joakim Tjernlund
2009-09-17  7:50       ` Wolfgang Denk
2009-09-17  8:39         ` Joakim Tjernlund
2009-09-17 10:15           ` Wolfgang Denk
2009-09-17 12:34             ` Joakim Tjernlund
2009-09-17 12:53               ` Wolfgang Denk
2009-09-17 13:25                 ` Joakim Tjernlund
2009-09-17 21:57                 ` Graeme Russ
2009-09-18  5:44                   ` Joakim Tjernlund
2009-09-17 17:29       ` Peter Tyser
2009-09-18 11:40         ` Joakim Tjernlund
2009-09-18 14:28           ` Peter Tyser
2009-09-18 14:52             ` Joakim Tjernlund
2009-09-18 15:21               ` Peter Tyser [this message]
2009-09-18 15:33                 ` Joakim Tjernlund
2009-09-18 16:24                   ` Peter Tyser
2009-09-18 17:21                     ` Joakim Tjernlund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1253287317.617.459.camel@localhost.localdomain \
    --to=ptyser@xes-inc.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.