diff mbox

Align TOC to 256 bytes

Message ID 1431581140.4160.23.camel@kernel.crashing.org
State Accepted
Headers show

Commit Message

Benjamin Herrenschmidt May 14, 2015, 5:25 a.m. UTC
Anton sent a similar patch to Linus, here is his comment:

<<
Recent toolchains force the TOC to be 256 byte aligned. We need
to enforce this alignment in our linker script, otherwise pointers
to our TOC variables (...) could be incorrect.
>>

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Comments

Patrick Williams May 14, 2015, 12:37 p.m. UTC | #1

Benjamin Herrenschmidt May 14, 2015, 9:28 p.m. UTC | #2
On Thu, 2015-05-14 at 06:37 -0600, Patrick Williams III wrote:
> Ben,
> 
> 
> For my own understanding:
> 
> 
> * What part of the toolchain is forcing the TOC to 256 alignment?
> * Is the alignment on the beginning of the TOC section or the TOC-base
> part of the function pointer tuples  (ABIv1) ?

Anton, I just copied your cset over here, do you know the answers to
Patrick questions ?

Cheers,
Ben.

> Patrick Williams
> Power Firmware Development, 
> Bldg 045-2/C034
> 512-286-6369, T/L: 363-6369
> iawillia@us.ibm.com
> 
> 
> 
> -----"Skiboot" <skiboot-bounces+iawillia=us.ibm.com@lists.ozlabs.org>
> wrote: -----
> To: skiboot@lists.ozlabs.org
> From: Benjamin Herrenschmidt 
> Sent by: "Skiboot" 
> Date: 05/14/2015 12:25AM
> Subject: [Skiboot] [PATCH] Align TOC to 256 bytes
> 
> Anton sent a similar patch to Linus, here is his comment:
> 
> <<
> Recent toolchains force the TOC to be 256 byte aligned. We need
> to enforce this alignment in our linker script, otherwise pointers
> to our TOC variables (...) could be incorrect.
> >>
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> diff --git a/skiboot.lds.S b/skiboot.lds.S
> index a54ff5f..d5e493a 100644
> --- a/skiboot.lds.S
> +++ b/skiboot.lds.S
> @@ -82,7 +82,7 @@ SECTIONS
>   *(.opd)
>   }
>    
> - . = ALIGN(0x10);
> + . = ALIGN(0x100);
>   .got : {
>   __toc_start = . + 0x8000;
>   *(.got)
> 
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
>
Anton Blanchard May 14, 2015, 9:34 p.m. UTC | #3
Hi Patrick,

> * What part of the toolchain is forcing the TOC to 256 alignment?
> * Is the alignment on the beginning of the TOC section or the
> TOC-base part of the function pointer tuples  (ABIv1) ?

It's a recent binutils patch from Alan:

commit a27e685fa0a6480bdb07e3be359558524cec89b7
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Apr 21 19:18:24 2015 +0930

    Align .TOC. for PowerPC64
    
    This change, with prerequisite 0e5fabeb, provides a toc base aligned
    to 256 bytes rather than 8 bytes.  This is necessary for a minor gcc
    optimisation, allowing use of d-form instructions to correctly access
    toc-relative items larger than 8 bytes.

Anton
Patrick Williams May 14, 2015, 10:09 p.m. UTC | #4

Alan Modra May 16, 2015, 2:20 p.m. UTC | #5
Patrick Williams III/Rochester/IBM wrote on 15/05/2015 07:39:51:

> My understanding is that the TOC base points somewhere "in the
> middle" of the GOT, so that both positive and negative load offsets
> can be used.  It isn't immediately obvious how aligning the GOT to
> 256 bytes also aligns the TOC base itself.
>
> I guess this works because the TOC base is actually a constant from the
GOT:
> #define TOC_BASE_OFF   0x8000

The linker change aligns the TOC base regardless of whether your script
aligns the .got section.  Which means that if you don't align .got, then
the TOC base is *not* .got + 0x8000.  (It's rounded down to a 256 byte
boundary.)  So that anyone using a linker script symbol to guess the TOC
base, rather than using .TOC. (for example because you need to be
compatible with older linkers that don't define .TOC), is in trouble.

One backwards compatible solution is to align .got to 256 bytes.

Another is to use something like the following to define your symbol
guessing the TOC base.
  .got          : ALIGN(at_least_8)
  {
    __toc_base = DEFINED (.TOC.) ? .TOC. : . + 0x8000 ;
    *(.got .toc)
  }
With a new linker you won't be guessing, but actually using the linker
defined TOC base.  And Alan Modra won't be breaking things for you if
the .got layout changes..  Sorry about that.

> I see the linker script change Alan made is basically the same
> proposed here, so I trust it is right and I'll make correspondingly
> similar changes to Hostboot.
diff mbox

Patch

diff --git a/skiboot.lds.S b/skiboot.lds.S
index a54ff5f..d5e493a 100644
--- a/skiboot.lds.S
+++ b/skiboot.lds.S
@@ -82,7 +82,7 @@  SECTIONS
 		*(.opd)
 	}
   
-	. = ALIGN(0x10);
+	. = ALIGN(0x100);
 	.got : {
 		__toc_start = . + 0x8000;
 		*(.got)