[OpenBIOS] [PATCH, RFC] ppc: Turn 32-bit ppc64 into a config option, to be deprecated

Andreas Färber andreas.faerber at web.de
Sat Nov 20 22:16:32 CET 2010


Am 17.11.2010 um 20:58 schrieb Blue Swirl:

> On Wed, Nov 17, 2010 at 7:45 PM, Andreas Färber <andreas.faerber at web.de 
> > wrote:
>> Am 17.11.2010 um 02:32 schrieb Alexander Graf:
>>
>>> On 14.11.2010, at 02:48, Andreas Färber wrote:
>>>
>>>> Having 64-bit support as an option allows users to disable
>>>> and test it before it gets officially removed.
>>>>
>>>> Fork single-bitness macros EXCEPTION_{PREAMBLE,EPILOGUE}:
>>>> Use assembler macros for things that are constant to avoid
>>>> ; and \, and use preprocessor macros to handle differences.
>>>> Adopt QEMU coding style for new code.
>>>>
>>>> Functional changes for ppc64:
>>>> * Don't clear MSR in preamble.
>>>> * Just save the minimum number of registers since 64-bit code will
>>>> save the full registers.
>>>> * Reserve 48 bytes of stack frame space for ppc64, according to
>>>> 64-bit PowerPC ELF ABI supplement 1.9.
>>>> * Use RFI macro.
>>>>
>>>> Cc: Alexander Graf <agraf at suse.de>
>>>> Signed-off-by: Andreas Färber <andreas.faerber at web.de>
>>>> ---
>>>> arch/ppc/qemu/init.c           |    4 ++
>>>> arch/ppc/qemu/start.S          |   92
>>>> +++++++++++++++++++++++++++++++++++++++-
>>>> config/examples/ppc_config.xml |    1 +
>>>> 3 files changed, 96 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c
>>>> index 0b781d9..d17c843 100644
>>>> --- a/arch/ppc/qemu/init.c
>>>> +++ b/arch/ppc/qemu/init.c
>>>> @@ -303,6 +303,7 @@ cpu_g4_init(const struct cpudef *cpu)
>>>>   fword("finish-device");
>>>> }
>>>>
>>>> +#ifdef CONFIG_PPC_64BITSUPPORT
>>>> /* In order to get 64 bit aware handlers that rescue all our
>>>>  GPRs from getting truncated to 32 bits, we need to patch the
>>>>  existing handlers so they jump to our 64 bit aware ones. */
>>>> @@ -322,6 +323,7 @@ ppc64_patch_handlers(void)
>>>>   asm ( "icbi 0, %0" : : "r"(dsi) );
>>>>   asm ( "icbi 0, %0" : : "r"(isi) );
>>>> }
>>>> +#endif
>>>>
>>>> static void
>>>> cpu_970_init(const struct cpudef *cpu)
>>>> @@ -341,10 +343,12 @@ cpu_970_init(const struct cpudef *cpu)
>>>>
>>>>   fword("finish-device");
>>>>
>>>> +#ifdef CONFIG_PPC_64BITSUPPORT
>>>>   /* The 970 is a PPC64 CPU, so we need to activate
>>>>    * 64bit aware interrupt handlers */
>>>>
>>>>   ppc64_patch_handlers();
>>>> +#endif
>>>>
>>>>   /* The 970 also implements the HIOR which we need to set to 0 */
>>>>
>>>> diff --git a/arch/ppc/qemu/start.S b/arch/ppc/qemu/start.S
>>>> index e86bdfd..6cf20cf 100644
>>>> --- a/arch/ppc/qemu/start.S
>>>> +++ b/arch/ppc/qemu/start.S
>>>> @@ -14,6 +14,7 @@
>>>> *
>>>> */
>>>>
>>>> +#include "autoconf.h"
>>>> #include "asm/asmdefs.h"
>>>> #include "asm/processor.h"
>>>>
>>>> @@ -24,6 +25,8 @@
>>>> #define ILLEGAL_VECTOR( v )     .org __vectors + v ; vector__##v:  
>>>> bl
>>>> trap_error ;
>>>> #define VECTOR( v, dummystr )   .org __vectors + v ; vector__##v
>>>>
>>>> +#ifdef CONFIG_PPC_64BITSUPPORT
>>>> +
>>>> /* We're trying to use the same code for the ppc32 and ppc64  
>>>> handlers
>>>> here.
>>>> * On ppc32 we only save/restore the registers, C considers  
>>>> volatile.
>>>> *
>>>> @@ -176,6 +179,89 @@
>>>> #undef stl
>>>> #undef ll
>>>>
>>>> +#else
>>>> +
>>>> +#ifdef __powerpc64__
>>>> +
>>>> +#define ULONG_SIZE 8
>>>> +#define STACKFRAME_MINSIZE 48
>>>> +#define stl std
>>>> +#define ll  ld
>>>> +
>>>> +#else
>>>> +
>>>> +#define ULONG_SIZE 4
>>>> +#define STACKFRAME_MINSIZE 16
>>>> +#define stl stw
>>>> +#define ll  lwz
>>>> +
>>>> +#endif
>>>> +
>>>> +.macro EXCEPTION_PREAMBLE
>>>> +    mtsprg1 r1 /* scratch */
>>>> +    mfsprg0 r1 /* exception stack in sprg0 */
>>>> +    addi    r1, r1, -(20 * ULONG_SIZE) /* push exception frame */
>>>> +
>>>> +    stl     r0,  ( 0 * ULONG_SIZE)(r1) /* save r0 */
>>>> +    mfsprg1 r0
>>>> +    stl     r0,  ( 1 * ULONG_SIZE)(r1) /* save r1 */
>>>> +    stl     r2,  ( 2 * ULONG_SIZE)(r1) /* save r2 */
>>>> +    stl     r3,  ( 3 * ULONG_SIZE)(r1) /* save r3 */
>>>> +    stl     r4,  ( 4 * ULONG_SIZE)(r1)
>>>> +    stl     r5,  ( 5 * ULONG_SIZE)(r1)
>>>> +    stl     r6,  ( 6 * ULONG_SIZE)(r1)
>>>> +    stl     r7,  ( 7 * ULONG_SIZE)(r1)
>>>> +    stl     r8,  ( 8 * ULONG_SIZE)(r1)
>>>> +    stl     r9,  ( 9 * ULONG_SIZE)(r1)
>>>> +    stl     r10, (10 * ULONG_SIZE)(r1)
>>>> +    stl     r11, (11 * ULONG_SIZE)(r1)
>>>> +    stl     r12, (12 * ULONG_SIZE)(r1)
>>>> +
>>>> +    mflr    r0
>>>> +    stl     r0,  (13 * ULONG_SIZE)(r1)
>>>> +    mfcr    r0
>>>> +    stl     r0,  (14 * ULONG_SIZE)(r1)
>>>> +    mfctr   r0
>>>> +    stl     r0,  (15 * ULONG_SIZE)(r1)
>>>> +    mfxer   r0
>>>> +    stl     r0,  (16 * ULONG_SIZE)(r1)
>>>> +
>>>> +    /* 76(r1) unused */
>>
>> This comment is obviously outdated, just like in the old code path.
>>
>>>> +
>>>> +    addi r1, r1, -STACKFRAME_MINSIZE /* C ABI saves LR and SP */
>>>> +.endm
>>>> +
>>>> +.macro EXCEPTION_EPILOGUE
>>>> +    addi r1, r1,  STACKFRAME_MINSIZE /* pop ABI frame */
>>>> +
>>>> +    ll    r0,  (13 * ULONG_SIZE)(r1)
>>>> +    mtlr  r0
>>>> +    ll    r0,  (14 * ULONG_SIZE)(r1)
>>>> +    mtcr  r0
>>>> +    ll    r0,  (15 * ULONG_SIZE)(r1)
>>>> +    mtctr r0
>>>> +    ll    r0,  (16 * ULONG_SIZE)(r1)
>>>> +    mtxer r0
>>>> +
>>>> +    ll    r0,  ( 0 * ULONG_SIZE)(r1)
>>>> +    ll    r2,  ( 2 * ULONG_SIZE)(r1)
>>>> +    ll    r3,  ( 3 * ULONG_SIZE)(r1)
>>>> +    ll    r4,  ( 4 * ULONG_SIZE)(r1)
>>>> +    ll    r5,  ( 5 * ULONG_SIZE)(r1)
>>>> +    ll    r6,  ( 6 * ULONG_SIZE)(r1)
>>>> +    ll    r7,  ( 7 * ULONG_SIZE)(r1)
>>>> +    ll    r8,  ( 8 * ULONG_SIZE)(r1)
>>>> +    ll    r9,  ( 9 * ULONG_SIZE)(r1)
>>>> +    ll    r10, (10 * ULONG_SIZE)(r1)
>>>> +    ll    r11, (11 * ULONG_SIZE)(r1)
>>>> +    ll    r12, (12 * ULONG_SIZE)(r1)
>>>> +
>>>> +    ll    r1,  ( 1 * ULONG_SIZE)(r1) /* restore stack at last */
>>>> +    RFI
>>>> +.endm
>>>
>>>
>>> I don't think this really belongs in this patch, no? :)
>>
>> --verbose please. The description is pretty clear on what this  
>> patch does
>> and why.
>>
>> RFI was committed in r955, so can be used here.
>>
>> It doesn't make sense to me to #ifdef out code without providing the
>> equivalent code path for ppc64, so this new code path does belong  
>> here
>> (remember you agreed that having two parallel code paths was  
>> supposedly the
>> only way for migration?).
>>
>> If you're referring to the suggested use of a .macro, please  
>> discuss this
>> with Blue in general and not some hundred lines down a patch without
>> referring to the commit message where this is discussed. Blue has  
>> been
>> asking me to turn preprocessor macros into C inline functions - this
>> construct here seems the closest equivalent in assembler code and the
>> advantages I see are detailed in the commit message. You have not  
>> yet voiced
>> any particular reason to make this a preprocessor macro, other than  
>> your
>> personal preference for preprocessor macros, which appears to  
>> conflict with
>> Blue's dislike of preprocessor macros and my dislike of unnecessary
>> multi-line preprocessor macros.
>
> I was going to comment that actually I haven't seen much gas macro use
> in Linux. But then I ran grep and while there really are no macros for
> Sparc (or x86_64), other architectures use them a lot. So macros are
> OK for me.

Thanks, applied slightly modified version in r962.

Andreas


More information about the OpenBIOS mailing list