[OpenBIOS] [commit] r969 - in trunk/openbios-devel/arch: ppc/qemu ppc64/qemu

[Blue Swirl]

On Sat, Nov 27, 2010 at 6:42 PM, Andreas Färber <andreas.faerber at web.de> wrote:
> Am 27.11.2010 um 16:03 schrieb Blue Swirl:
>
>> On Sat, Nov 27, 2010 at 10:36 AM, Andreas Färber <andreas.faerber at web.de>
>> wrote:
>>>
>>> Am 27.11.2010 um 11:22 schrieb Blue Swirl:
>>>
>>>> On Sat, Nov 27, 2010 at 1:16 AM, Andreas Färber <andreas.faerber at web.de>
>>>> wrote:
>>>>>
>>>>> Am 26.11.2010 um 20:52 schrieb Blue Swirl:
>>>>>
>>>>>> On Thu, Nov 25, 2010 at 10:04 PM, Andreas Färber
>>>>>> <andreas.faerber at web.de>
>>>>>> wrote:
>>>>>>>
>>>>>>> Starting with [r969],
>>>>>>>
>>>>>>> qemu-system-ppc64 -bios .../obj-ppc64/openbios-qemu.elf -prom-env
>>>>>>> 'auto-boot?=false'
>>>>>>>
>>>>>>> should take you to the Forth prompt.
>>>>>>
>>>>>> I built forthstrap on a 32 bit host to get further, but then I got:
>>>>>>  LINK  openbios-qemu.elf
>>>>>> libqemu.a(init.o): In function `go':
>>>>>> /src/openbios-devel/obj-ppc64/../arch/ppc/qemu/init.c:533: undefined
>>>>>> reference to `.call_elf'
>>>>>
>>>>> [snip]
>>>>>
>>>>> Sounds as if -m32 or the like is being passed to powerpc64-linux-ld?
>>>>> Dotted
>>>>> symbols only get generated for ppc64 target.
>>>>
>>>> No, LINK equals to (with V=1):
>>>>
>>>> powerpc64-linux-ld --warn-common -N -T ../arch/ppc64/qemu/ldscript -o
>>>> openbios-qemu.elf.nostrip --whole-archive target/arch/ppc/qemu/start.o
>>>> target/arch/ppc/timebase.o libqemu.a libbootstrap.a libopenbios.a
>>>> libpackages.a libdrivers.a libfs.a liblibc.a libgcc.a
>>>
>>> Ah, right, it would've warned about architecture mismatch then. What
>>> about
>>> CC for start.o?
>>> And could you try 4.5.1 to rule out your 4.6 snapshot? I'll try if
>>> Darwin/ppc64 works.
>>
>> It looks like GLOBL macro is not correct.
>
> Yeah, I stumbled across that for call_elf(), too.
>
>> With this patch (using Linux
>> arch/powerpc/include/asm/ppc_asm.h except for the .sections which
>> don't work for some reason),
>
> (the Linux macros force .text while we use .text.vectors for placement; .opd
> should be right though)

But then there are strange errors:
../arch/ppc/qemu/start.S: Assembler messages:
../arch/ppc/qemu/start.S:293: Error: invalid segment ".opd"

>> the link succeeds:
>> diff --git a/arch/ppc/qemu/start.S b/arch/ppc/qemu/start.S
>> index 4b6df3f..8439542 100644
>> --- a/arch/ppc/qemu/start.S
>> +++ b/arch/ppc/qemu/start.S
>> @@ -285,14 +285,13 @@ call_isi_exception:
>> exception_return:
>>       EXCEPTION_EPILOGUE
>>
>> -       .globl  __divide_error
>> -__divide_error:
>> +GLOBL(__divide_error):
>
> Acked.
>
>> trap_error:
>>       mflr    r3
>>       b       BRANCH_LABEL(unexpected_excep)
>>
>> VECTOR( 0x100, "SRE" ):
>> -        b       _entry
>> +        b       BRANCH_LABEL(_entry)
>>
>> ILLEGAL_VECTOR( 0x200 )
>>
>> @@ -358,7 +357,7 @@ VECTOR( 0x2200, "ISI_64" ):
>> GLOBL(__vectors_end):
>>
>> /************************************************************************/
>> -/*     entry                                                           */
>> +/*     _entry                                                          */
>> /************************************************************************/
>>
>> GLOBL(_entry):
>> @@ -453,7 +452,7 @@ GLOBL(_entry):
>> #endif
>>
>>       bl      BRANCH_LABEL(setup_mmu)
>> -       bl      BRANCH_LABEL(entry)
>> +        bl      BRANCH_LABEL(_entry)
>
> Nack, infinite recursion. Original code is right, unless .entry gets hidden
> by the compiler somehow. If the latter, we need to dereference the function
> descriptor instead and do a bctrl.

But then there would be a link time conflict with entry() in
arch/ppc/qemu/init.c.

>> 1:     nop
>>       b       1b
>>
>> @@ -678,4 +677,4 @@ compute_ramsize:
>>
>>        /* Hard reset vector */
>>       .section .romentry,"ax"
>> -       bl      _entry
>> +        bl      BRANCH_LABEL(_entry)
>> diff --git a/include/arch/ppc/asmdefs.h b/include/arch/ppc/asmdefs.h
>> index 9c85ea5..b2cfa48 100644
>> --- a/include/arch/ppc/asmdefs.h
>> +++ b/include/arch/ppc/asmdefs.h
>> @@ -110,8 +110,25 @@
>> #endif
>>
>> #ifndef __darwin__
>> +#ifdef __powerpc64__
>> +#define XGLUE(a,b) a##b
>> +#define GLUE(a,b) XGLUE(a,b)
>> +
>> +#define GLOBL(name) \
>> +        .align 2 ; \
>> +        .globl name; \
>> +        .globl GLUE(.,name); \
>> +name: \
>> +        .quad GLUE(.,name); \
>> +        .quad .TOC. at tocbase; \
>> +        .quad 0; \
>> +        .type GLUE(.,name), at function; \
>> +GLUE(.,name)
>
> Doesn't look right without the sections... or at least strange.
>
>> +#define EXTERN(name)            _##name
>> +#else
>> #define GLOBL( name )          .globl name ; name
>> #define EXTERN( name )         name
>> +#endif
>> #else
>> /* an underscore is needed on Darwin */
>> #define GLOBL( name )          .globl _##name ; name: ; _##name
>
> I was rather considering to add a new macro for externally called functions
> like call_elf() and the ciface, i.e. leave GLOBL() as it is with
> compatibility stuff and add a new GLOBAL() macro for instance that does the
> ppc64 .opd, using our GLOBL() internally. The advantage would be that we
> don't need to clutter start.S with unneeded BRANCH_LABEL() calls. What do
> you think?

I think that's also the approach Linux uses. I don't have preference.