[U-Boot] relocation problem on powerpc

[Chris Packham <judge.packham@gmail.com>]

Hi Jocke,

On Tue, Sep 9, 2014 at 12:22 AM, Joakim Tjernlund
<joakim.tjernlund@transmode.se> wrote:
>>
>> >
>> > There are some disadvantages to living in a weird timezone, mostly
>> > that you end up having conversations with yourself.
>> >
>> > On Mon, Sep 8, 2014 at 4:32 PM, Chris Packham
> <judge.packham@gmail.com>
>> wrote:
>> > > Hi All,
>> > >
>> > > I have come across what I think is a relocation problem for powerpc.
>> > >
>> > > I've added the following to ArpTimeoutCheck
>> > >
>> > > +       printf("NetArpWaitTimerStart = %ld\n",
> NetArpWaitTimerStart);
>> > > +       printf("&NetArpWaitTimerStart = %p\n",
> &NetArpWaitTimerStart);
>> > > +       printf("NetArpWaitTry = %d\n", NetArpWaitTry);
>> > > +       printf("&NetArpWaitTry = %p\n", &NetArpWaitTry);
>> > > +       printf("NetArpWaitTxPacketSize = %d\n",
>> NetArpWaitTxPacketSize);
>> > > +       printf("&NetArpWaitTxPacketSize = %p\n",
>> &NetArpWaitTxPacketSize);
>> > >
>> > > Which yields the following output
>> > >
>> > >   NetArpWaitTimerStart = 0
>> > >   &NetArpWaitTimerStart = f00000d0
>> > >   NetArpWaitTry = 1
>> > >   &NetArpWaitTry = 7ffb0058
>> > >   NetArpWaitTxPacketSize = 42
>> > >   &NetArpWaitTxPacketSize = 7ffb0078
>> > >
>> > > That looks to me like NetArpWaitTimerStart hasn't been relocated for
>> > > some reason. Looking at my u-boot.map NetArpWaitTimerStart is the
> last
>> > > item in the .sbss section
>> > >
>> > > Here's the relevant snippets for the variables I'm displaying
>> > >
>> > >  0x00000000f0000058                NetArpWaitTry
>> > >  0x00000000f0000078                NetArpWaitTxPacketSize
>> > >  0x00000000f00000d0                NetArpWaitTimerStart
>> > >
>> > > The actual problem for me is that ARPs timeout and various network
>> > > things fail. Has anyone got any clues as to why this one particular
>> > > variable isn't getting relocated.
>> > >
>> > > Thanks,
>> > > Chris
>> > >
>> > > More info:
>> > >   I'm building P2041RDB_config
>> > >
>> > >   $ git describe
>> > >   v2014.10-rc2-15-g2ec8915
>> > >
>> > >   => version
>> > >   U-Boot 2014.10-rc2-00015-g2ec8915-dirty (Sep 08 2014 - 16:18:42)
>> > >   powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3
>> > >   GNU ld (GNU Binutils) 2.21
>> >
>> > I'm not confident enough to say it's a fix but the following seems to
>> > solve the relocation problem for NetArpWaitTimerStart.
>> >
>> > diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds
>> > b/arch/powerpc/cpu/mpc85xx/u-boot.lds
>> > index 2cf0b25..36711b0 100644
>> > --- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
>> > +++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
>> > @@ -54,7 +54,7 @@ SECTIONS
>> >      _FIXUP_TABLE_ = .;
>> >      KEEP(*(.fixup))
>> >    }
>> > -  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
>> > +  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
>> >    __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
>>
>> This looks like the correct fix, The relevant code that uses
>> __got2_entries is in start.S:
>>
>> in_ram:
>>
>>         /*
>>          * Relocation Function, r12 point to got2+0x8000
>>          *
>>          * Adjust got2 pointers, no need to check for 0, this code
>>          * already puts a few entries in the table.
>>          */
>>         li      r0,__got2_entries at sectoff@l
>>         la      r3,GOT(_GOT2_TABLE_)
>>         lwz     r11,GOT(_GOT2_TABLE_)
>>         mtctr   r0
>>         sub     r11,r3,r11
>>         addi    r3,r3,-4
>> 1:      lwzu    r0,4(r3)
>>         cmpwi   r0,0
>>         beq-    2f
>>         add     r0,r0,r11
>>         stw     r0,0(r3)
>> 2:      bdnz    1b
>>
>> bdnz does decrement then test for zero so __got2_entries should hold the
>
>> number of entries to relocate.
>>
>> Seems like most(all?) ppc cpu has this error.
>
> Looking at u-boot.lds:
>  .reloc   :
>   {
>     _GOT2_TABLE_ = .;
>     KEEP(*(.got2))
>     KEEP(*(.got))
>     PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4);
>     _FIXUP_TABLE_ = .;
>     KEEP(*(.fixup))
>   }
>   __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
>   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
>
>
> There is something more to this error. Since _GLOBAL_OFFSET_TABLE_  is
> defined to
> _GLOBAL_OFFSET_TABLE_ = . + 4
> there is one extra entry added( for brlr insn inserted by the linker) so
> something is
> amiss here, is there a brlr insns at _GLOBAL_OFFSET_TABLE_-4 in your
> u-boot?
>
>  Jocke
>

Here's some more info from a build without any of my modifications.

  $ git describe
  v2014.10-rc2-12-g0b703db

  $ powerpc-e500-linux-gnu-gcc --version
  powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3

  $ grep '_GOT2_TABLE_\|_GLOBAL_OFFSET_TABLE_\|__got2_entries' u-boot.map
  0x00000000effb5100                _GOT2_TABLE_ = .
  0x00000000effb7d38                _GLOBAL_OFFSET_TABLE_
  0x00000000effb7d48                PROVIDE (_GLOBAL_OFFSET_TABLE_, (. + 0x4))
  0x0000000000000b0d                __got2_entries =
(((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 0x2) - 0x1)

  $ gdb-multiarch u-boot
  (gdb) disassemble /r 0xeffb5100,0xeffb7d48
  Dump of assembler code from 0xeffb5100 to 0xeffb7d48:
     0xeffb5100 <_GOT2_TABLE_+0>: ef fb 51 00     .long 0xeffb5100
     0xeffb5104 <_GOT2_TABLE_+4>: ef fb 7d 44     dtstsf  cr7,f27,f15
     0xeffb5108 <_GOT2_TABLE_+8>: ef f4 00 00     .long 0xeff40000
     0xeffb510c <_GOT2_TABLE_+12>:        ef f4 01 00     .long 0xeff40100
    <snip>
     0xeffb7d30 <_GOT2_TABLE_+11312>:     ef f6 6e 74     .long 0xeff66e74
     0xeffb7d34 <_GOT2_TABLE_+11316>:     f0 00 00 d0     psq_st  f0,208(r0),0,0
     0xeffb7d38 <_GLOBAL_OFFSET_TABLE_+0>:        00 00 00 00     .long 0x0
     0xeffb7d3c <_GLOBAL_OFFSET_TABLE_+4>:        00 00 00 00     .long 0x0
     0xeffb7d40 <_GLOBAL_OFFSET_TABLE_+8>:        00 00 00 00     .long 0x0
     0xeffb7d44 <_FIXUP_TABLE_+0>:        ef fb 99 d4     .long 0xeffb99d4

So no there is no brlr at the end of the table.

Also I think the __got2_entries calculation does not take into account
the +0x4, ((0xeffb7d38-0xeffb5100)>>2)-1) = 0xb0d