[OpenBIOS] ppc64 MMU issues

[Alexander Graf]

On 09.11.2010, at 21:01, Andreas Färber wrote:

> Am 09.11.2010 um 12:53 schrieb Alexander Graf:
> 
>> On 08.11.2010, at 23:18, Andreas Färber wrote:
>> 
>>> Am 08.11.2010 um 23:03 schrieb Alexander Graf:
>>> 
>>>> On 08.11.2010, at 22:48, Andreas Färber wrote:
>>>> 
>>>>> Am 08.11.2010 um 10:34 schrieb Mark Cave-Ayland:
>>>>> 
>>>>>> Andreas Färber wrote:
>>>>>> 
>>>>>>> Since r945 everything except for the trampoline issue should be in SVN.
>>>>>> 
>>>>>> Cool! :)
>>>>> 
>>>>> Ah, no! Forgot about the exception handlers (preamble, epilogue). :( I have local patches that #ifdef the whole definitions and provide alternative ones for ppc64. But I'd rather supply a 2-step patch to drop the template macros and use #ifdef'ery inside the macro definition for ppc64/ppc differences, given that long-term we won't need the second version.
>>>> 
>>>> Are you sure you need that? Most of the stuff should be generic. Please take a look at Linux's PPC_LL macro for example. The stack layout varies slightly, but that should also be reasonably easy to catch.
>>> 
>>> Problematic parts are the ULONG_SIZE comparisons for the number of registers that we can't #ifdef away inside an CPP macro
>> 
>> You're talking about this piece of code, right?
>> 
>> .ifc ULONG_SIZE, 8 ; \
>>       addi    r1,r1,-(40 * ULONG_SIZE) ;      /* push exception frame */ \
>> .else ; \
>>       addi    r1,r1,-(20 * ULONG_SIZE) ;      /* push exception frame */ \
>> .endif ; \
>> \
>> 
>> 
>> Just replace it with
>> 
>> // PPC32
>> 
>> #define EXCEPTION_STACK_LEN (20 * 4)
>> 
>> // PPC64
>> 
>> #define EXCEPTION_STACK_LEN (40 * 8)
>> 
>> // code
>> 
>> addi  r1, r1,-EXCEPTION_STACK_LEN
> 
> No, no, no! This is exactly the type of differences I wanted to rip out, but if I do so now I would break ppc64 support.
> On __powerpc64__ this doesn't need to be 40, and it's not just the above immediate but the whole .ifc around the 20 additional register stores/loads:
> 
> #define EXCEPTION_PREAMBLE_TEMPLATE \
> 	...
> 	stl	r11,(11 * ULONG_SIZE)(r1) ; \
> 	stl	r12,(12 * ULONG_SIZE)(r1) ; \
> .ifc ULONG_SIZE, 8 ; \
> 	stl	r13,(17 * ULONG_SIZE)(r1) ; \
> 	stl	r14,(18 * ULONG_SIZE)(r1) ; \
> 	...
> 
> .macro EXCEPTION_PREAMBLE
> 	EXCEPTION_PREAMBLE_TEMPLATE
> .endm
> ...
> .macro EXCEPTION_PREAMBLE_64
> 	EXCEPTION_PREAMBLE_TEMPLATE
> .endm
> 
> My problem is the reuse of the _TEMPLATE macros. It becomes a matter of macros such as those you suggest if we can get rid of that.

Why are there two different preambles? Just always use the 64-bit one for 64-bit openBIOS and 32-bit one for 32-bit openBIOS.


> 
>> Btw, are you sure this code is correct? Usually the stack frame includes the old r1 value as well. In __kvmppc_vcpu_entry (arch/powerpc/kvm/book3s_interrupts.S) I have working code that basically does an ABI compliant function in asm:
>> 
>>       /* Save host state to the stack */
>>       PPC_STLU r1, -SWITCH_FRAME_SIZE(r1)
> 
> Haven't checked. I spotted an instance where two of the three stacks have a potential stack frame overlap though.
> 
>>> , plus the m*msr
>> 
>>       mfmsr   r1 ;                            /* unset MSR_SF */ \
>>       clrlwi  r1,r1,0 ; \
>>       mtmsr   r1 ; \
>> 
>> This one? Yeah, my awesome broken code. Just replace it with:
>> 
>> // PPC32
>> 
>> #define MFMSRD mfmsr
>> #define MTMSRD mtmsr
>> 
>> // PPC64
>> 
>> #define MFMSRD mfmsrd
>> #define MTMSRD mtmsrd
>> 
>> // code
>> 
>> MFMSRD r1
>> MTMSRD r1
>> 
>> 
>> The clearing is not necessary. If it is, qemu or kvm are broken. According to the spec, mfmsr and mtmsr only handle the first 32 bits of MSR.
> 
> I've been working without on ppc64, MSR looked okay.
> 
>>> and the trailing/leading stack manipulation (16 in SVR4 vs. 48 in PowerOpen ABI).
>> 
>> Not sure I understand exactly what you mean here :).
> 
> addi r1, r1, -x
> addi r1, r1,  x
> :)
> 
> Btw I still don't understand what ABI this is supposed to be. If I can trust Hollis' ppc assembler article and the Mono and QEMU ppc64 ports, then Linux/ppc64 uses function descriptors. But our ppc64 assembler code calling setup_mmu() and entry() works without a function descriptor dereference...

Uh, sounds like a question to Segher :).


Alex