[OpenBIOS] [PATCH 2/4] ppc: add new context handler

[Mark Cave-Ayland]

On 01/05/16 21:10, Segher Boessenkool wrote:

> On Sun, May 01, 2016 at 11:23:00AM +0100, Mark Cave-Ayland wrote:
>> +struct context *switch_to(struct context *ctx)
>> +{
>> +    volatile struct context *save;
>> +    struct context *ret;
>> +    unsigned int lr;
>> +
>> +    debug("switching to new context:\n");
>> +    save = __context;
>> +    __context = ctx;
>> +
>> +    asm __volatile__ ("mflr %0\n\t" : "=r" (lr) : : );
>> +    asm __volatile__ ("bl __switch_context\n");
>> +    asm __volatile__ ("mtlr %0\n\t" : : "r" (lr) : );
>> +    
>> +    ret = __context;
>> +    __context = (struct context *)save;
>> +    return ret;
>> +}
> 
> This isn't correct; GCC is free to put LR-modifying code between
> the asm statements.  You should do it in one asm statement.

It seems the problem John was seeing is related to the compile flags in
that the asm above don't work correctly when the binary is compiled with
-Os. For comparison here is what I see with the different optimisation
settings:

-O0 (works):

   0xfff08350 <+68>:    mflr    r9
   0xfff08354 <+72>:    stw     r9,8(r31)
   0xfff08358 <+76>:    bl      0xfff08060 <__switch_context>
   0xfff0835c <+80>:    lwz     r9,8(r31)
   0xfff08360 <+84>:    mtlr    r9

-Os (broken):

   0xfff082a8 <+32>:    mflr    r9
   0xfff082ac <+36>:    bl      0xfff08060 <__switch_context>
   0xfff082b0 <+40>:    mtlr    r9

So the optimiser has decided to remove the storage to/from the lr stack
variable which of course means that the lr value isn't being preserved.
Do I somehow need to mark the "r" (lr) as clobbered or something else?

>> +  #ifdef __powerpc64__
>> +    #define ULONG_SIZE 8
>> +    #define STACKFRAME_MINSIZE 48
> 
> The miminum frame size for elfv1 is 112 and for elfv2 is 32.  I think.
> But not 48, in any case.

I took this directly from Andreas' code in arch/ppc/qemu/start.S as I
assumed it was correct? It looks like the PPC64 code tries to align to
the nearest 16 bytes which would account for 32 + 16. Andreas, can you
give a bit more background on this?



ATB,

Mark.