Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
Instead we now link the bootwrapper with -pie to get a position-
independent executable, and process the relocations in the dynamic
relocation section that the linker puts into the executable.
Hurray! Looks good, just a few nits...
+ bl .+4
+p_base:mflrr10 /* r10 now points to runtime addr of
p_base */
bl p_base instead?
+10:or. r8,r0,r9/* skip relocation if we don't have both */
beq 3f
Either the code or the comment is wrong -- the code says skip
relocation
if we don't have either.
+ cmpwi r0,22 /* R_PPC_RELATIVE */
+ bne 3f
It would be nice if there was some way to complain (at build time?)
if there are unhandled relocations present. Prevents a lot of headaches
when things go wrong (and they will ;-) )
4: dcbfr0,r9
icbir0,r9
Fix these while you're at it? It's not r0, it's 0.
+ .dynsym : { *(.dynsym) }
+ .dynstr : { *(.dynstr) }
+ .dynamic :
+ {
+__dynamic_start = .;
+*(.dynamic)
+ }
+ .hash : { *(.hash) }
+ .interp : { *(.interp) }
+ .rela.dyn : { *(.rela*) }
Do some of these sections need alignment?
Segher
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
Segher Boessenkool writes:
Hurray! Looks good, just a few nits...
Thanks for reviewing.
+ bl .+4
+p_base:mflrr10 /* r10 now points to runtime addr of
p_base */
bl p_base instead?
I went back and forth on that. I ended up with it that way to
emphasize that the bl does need to branch to the *next* instruction
for the idiom to work.
+10:or. r8,r0,r9/* skip relocation if we don't have
both */
beq 3f
Either the code or the comment is wrong -- the code says skip
relocation
if we don't have either.
Ah, operator precedence rules in English. :) I (and I think most
native English speakers) would parse my comment as don't (have both)
whereas I think you parsed it as (don't have) both. Similarly what
you say the code says parses as don't (have either) for me rather
than (don't have) either. IOW, don't have either means both are
missing.
Anyway I can change the comment to say we need both to do
relocation. OK?
+ cmpwi r0,22 /* R_PPC_RELATIVE */
+ bne 3f
It would be nice if there was some way to complain (at build time?)
if there are unhandled relocations present. Prevents a lot of headaches
when things go wrong (and they will ;-) )
Yes, that would be a good idea. There is one extra relocation when I
build for pSeries, which was for _platform_stack_top (which is
undefined), which we're OK ignoring.
4: dcbfr0,r9
icbir0,r9
Fix these while you're at it? It's not r0, it's 0.
Yeah.
+ .dynsym : { *(.dynsym) }
+ .dynstr : { *(.dynstr) }
+ .dynamic :
+ {
+__dynamic_start = .;
+*(.dynamic)
+ }
+ .hash : { *(.hash) }
+ .interp : { *(.interp) }
+ .rela.dyn : { *(.rela*) }
Do some of these sections need alignment?
I suppose they should ideally be 4-byte aligned. We don't actually
use .dynsym, .dynstr, .hash or .interp, but I couldn't find any way to
make the linker omit them.
Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
+ bl .+4
+p_base:mflrr10 /* r10 now points to runtime addr of
p_base */
bl p_base instead?
I went back and forth on that. I ended up with it that way to
emphasize that the bl does need to branch to the *next* instruction
for the idiom to work.
Right, I see. Make it even more obvious? Use bcl 20,31,$+4 instead
(so people will have to look it up before changing this code, not
because
it is a few cycles faster ;-) ), add an empty line before the label, or
even put an actual comment there? :-)
+10:or. r8,r0,r9/* skip relocation if we don't have both */
beq 3f
Either the code or the comment is wrong -- the code says skip
relocation
if we don't have either.
Ah, operator precedence rules in English. :)
While I don't think that double (and triple) negations in English are
not overused and confusing...
I (and I think most
native English speakers) would parse my comment as don't (have both)
whereas I think you parsed it as (don't have) both. Similarly what
you say the code says parses as don't (have either) for me rather
than (don't have) either. IOW, don't have either means both are
missing.
... that is exactly what I meant: the code skips relocation only if
both are missing.
Anyway I can change the comment to say we need both to do
relocation. OK?
Please do -- but also change the code to match :-)
+ .dynsym : { *(.dynsym) }
+ .dynstr : { *(.dynstr) }
+ .dynamic :
+ {
+__dynamic_start = .;
+*(.dynamic)
+ }
+ .hash : { *(.hash) }
+ .interp : { *(.interp) }
+ .rela.dyn : { *(.rela*) }
Do some of these sections need alignment?
I suppose they should ideally be 4-byte aligned.
Ideally, yes; I was questioning whether the ABI requires it. It will
only cost a few bytes of object size so let's just do it?
We don't actually
use .dynsym, .dynstr, .hash or .interp, but I couldn't find any way to
make the linker omit them.
Assign those input sections to the /DISCARD/ output section (and do
that early enough in the linker script so they aren't picked up by
some other wildcard). Something like
/DISCARD/ : { *(.dynstr .dynsym .hash .interp) }
Segher
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
Segher Boessenkool writes: ... that is exactly what I meant: the code skips relocation only if both are missing. Doh! You're right, thanks. Serves me right for being so clever. :) Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[RFC PATCH] Link the bootwrapper as a position-independent executable
This changes the way we make the boot wrapper position-independent. Before we just added the offset (the difference between runtime address and link address) to each entry in the .got2 section. That doesn't relocate pointer values in initialized variables and arrays. Instead we now link the bootwrapper with -pie to get a position- independent executable, and process the relocations in the dynamic relocation section that the linker puts into the executable. This means that initialized variables and arrays get relocated properly. Currently we only process R_PPC_RELATIVE relocations, which is all we get provided C files are compiled with -fPIC (which they are already) and assembly code is written to be position-independent. Some of the changes below are to make parts of crt0.S be PIC code (i.e. generate no relocations other than R_PPC_RELATIVE). The relocation code below does nothing if there is no dynamic section, which means that we can link without -pie and it will work provided that the bootwrapper executable is loaded at its linked-at address. Signed-off-by: Paul Mackerras [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S index f1c4dfc..a6901c3 100644 --- a/arch/powerpc/boot/crt0.S +++ b/arch/powerpc/boot/crt0.S @@ -6,16 +6,28 @@ * as published by the Free Software Foundation; either version * 2 of the License, or (at your option) any later version. * - * NOTE: this code runs in 32 bit mode and is packaged as ELF32. + * NOTE: this code runs in 32 bit mode, is position-independent, + * and is packaged as ELF32. */ #include ppc_asm.h .text - /* a procedure descriptor used when booting this as a COFF file */ + /* A procedure descriptor used when booting this as a COFF file. +* When making COFF, this comes first in the link and we're +* linked at 0x50. +*/ .globl _zimage_start_opd _zimage_start_opd: - .long _zimage_start, 0, 0, 0 + .long 0x50, 0, 0, 0 + +p_start: .long _start +p_etext: .long _etext +p_bss_start: .long __bss_start +p_end: .long _end + + .weak _platform_stack_top +p_pstack: .long _platform_stack_top .weak _zimage_start .globl _zimage_start @@ -24,37 +36,60 @@ _zimage_start: _zimage_start_lib: /* Work out the offset between the address we were linked at and the address where we're running. */ - bl 1f -1: mflrr0 - lis r9,[EMAIL PROTECTED] - addir9,r9,[EMAIL PROTECTED] - subf. r0,r9,r0 - beq 3f /* if running at same address as linked */ + bl .+4 +p_base:mflrr10 /* r10 now points to runtime addr of p_base */ + /* grab the link address of the dynamic section in r11 */ + addis r11,r10,(_GLOBAL_OFFSET_TABLE_-p_base)@ha + lwz r11,(_GLOBAL_OFFSET_TABLE_-p_base)@l(r11) + cmpwi r11,0 + beq 3f /* if not linked -pie */ + /* get the runtime address of the dynamic section in r12 */ + .weak __dynamic_start + addis r12,r10,(__dynamic_start-p_base)@ha + addir12,r12,(__dynamic_start-p_base)@l + subfr11,r11,r12 /* runtime - linktime offset */ - /* The .got2 section contains a list of addresses, so add - the address offset onto each entry. */ - lis r9,[EMAIL PROTECTED] - addir9,r9,[EMAIL PROTECTED] - lis r8,[EMAIL PROTECTED] - addir8,r8,[EMAIL PROTECTED] - subf. r8,r9,r8 + /* The dynamic section contains a series of tagged entries. +* We need the RELA and RELACOUNT entries. */ +RELA = 7 +RELACOUNT = 0x6ff9 + li r9,0 + li r0,0 +9: lwz r8,0(r12) /* get tag */ + cmpwi r8,0 + beq 10f /* end of list */ + cmpwi r8,RELA + bne 11f + lwz r9,4(r12) /* get RELA pointer in r9 */ + b 12f +11:addis r8,r8,(-RELACOUNT)@ha + cmpwi r8,[EMAIL PROTECTED] + bne 12f + lwz r0,4(r12) /* get RELACOUNT value in r0 */ +12:addir12,r12,8 + b 9b + + /* The relocation section contains a list of relocations. +* We now do the R_PPC_RELATIVE ones, which point to words +* which need to be initialized with addend + offset. +* The R_PPC_RELATIVE ones come first and there are RELACOUNT +* of them. */ +10:or. r8,r0,r9/* skip relocation if we don't have both */ beq 3f - srwi. r8,r8,2 - mtctr r8 - add r9,r0,r9 -2: lwz r8,0(r9) - add r8,r8,r0 - stw r8,0(r9) - addir9,r9,4 + mtctr r0 +2: lbz r0,4+3(r9) /* ELF32_R_INFO(reloc-r_info) */ + cmpwi r0,22 /* R_PPC_RELATIVE */ + bne 3f + lwz r12,0(r9)
