[OpenBIOS] [PATCH 2/4] ppc: Adjust call_elf() and CIF prologues and epilogues for ppc64

[Alexander Graf]

On 19.12.2010, at 14:08, Andreas Färber wrote:

> Am 19.12.2010 um 11:05 schrieb Alexander Graf:
> 
>> On 15.12.2010, at 01:11, Andreas Färber wrote:
>> 
>>> Respect differing register widths and stack frame ABI differences.
>>> 
>>> Fix of_client_callback stack alignment for both ppc and ppc64.
>>> 
>>> Cc: Alexander Graf <agraf at suse.de>
>>> Signed-off-by: Andreas Färber <andreas.faerber at web.de>
>>> ---
>>> arch/ppc/qemu/start.S |  183 +++++++++++++++++++++++++++----------------------
>>> 1 files changed, 100 insertions(+), 83 deletions(-)
>>> 
>>> diff --git a/arch/ppc/qemu/start.S b/arch/ppc/qemu/start.S
>>> index 4b25650..1a3548f 100644
>>> --- a/arch/ppc/qemu/start.S
>>> +++ b/arch/ppc/qemu/start.S
>>> @@ -179,6 +179,9 @@
>>> #undef stl
>>> #undef ll
>>> 
>>> +#define ULONG_SIZE 4
>>> +#define STACKFRAME_MINSIZE 16
>>> +
>>> #else /* !CONFIG_PPC_64BITSUPPORT */
>>> 
>>> #ifdef __powerpc64__
>>> @@ -471,16 +474,20 @@ GLOBL(_entry):
>>> 	 */
>>>       .data
>>> saved_stack:
>>> +#ifdef __powerpc64__
>>> +    .quad   0
>> 
>> This is to store a pointer, right? Just define a new macro here too - keeps the actual logic cleaner.
>> 
>> #ifdef __powerpc64__
>> #define DATA_LONG(x) .quad x
>> #else
>> #define DATA_LONG(x) .long x
>> #endif
>> 
>>   DATA_LONG(0)
>> 
>> :)
> 
> Good idea.
> 
>>> +#else
>>> 	.long	0
>>> +#endif
>>>       .previous
>>> 	/* void call_elf( arg1, arg2, entry ) */
>>> _GLOBAL(call_elf):
>>> 	mflr	r0
>>> -	stwu	r1,-16(r1)
>>> -	stw	r0,20(r1)
>>> +	PPC_STLU r1, -STACKFRAME_MINSIZE(r1)
>>> +	PPC_STL  r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1)
>> 
>> The PPC64 ABI also saves lr in the previous stack. I don't fully remember where, but I do remember that the ppc64 ABI was more complicated than ppc32 in that respect.
> 
> This is actually what this code does, for both ABIs.
> STACKFRAME_MINSIZE is one frame plus the ABI-specific offset into the previous one. :)

I see O_o.

> 
>>> 	mtlr	r5
>>> 	LOAD_REG_IMMEDIATE(r8, saved_stack)		// save our stack pointer
>>> -	stw	r1,0(r8)
>>> +	PPC_STL r1,0(r8)
>>> 	mfsdr1	r1
>>> 	addi	r1, r1, -32768		/* - 32 KiB exception stack */
>>> 	addis	r1, r1, -1			/* - 64 KiB stack */
>>> @@ -488,137 +495,147 @@ _GLOBAL(call_elf):
>>> 	li	r6,0			// r6 = address of client program arguments (unused)
>>> 	li	r7,0			// r7 = length of client program arguments (unused)
>>> 	li	r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR
>>> -	mtmsr	r0
>>> +	MTMSRD(r0)
>>> 	blrl
>>> 
>>> 	LOAD_REG_IMMEDIATE(r8, saved_stack)		// restore stack pointer
>>> 	mr	r1,r8
>>> -	lwz	r0,20(r1)
>>> +	PPC_LL r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1)
>>> 	mtlr	r0
>>> -	addi	r1,r1,16
>>> +	addi	r1, r1, STACKFRAME_MINSIZE
>>> 	// XXX: should restore r12-r31 etc..
>>> 	// we should not really come here though
>>> 	blr
>> 
>> The parts below make my head spin :). Are they really that complicated, or are they just written that way?
> 
> Hrm? You referring to my code or the one I converted? I think mine is much more readable than all those integers...

The one that existed was already hard to read, but adding ppc64 to the picture doesn't help :). I agree that having proper constants with names in there really helps though!

Either way, the basic essence of that comment was "I read until here, the stuff below I didn't review" :).


Alex