[OpenBIOS] ppc64 MMU issues

[Alexander Graf]

On 09.11.2010, at 22:58, Andreas Färber wrote:

> Am 09.11.2010 um 22:48 schrieb Alexander Graf:
> 
>> On 09.11.2010, at 22:34, Andreas Färber wrote:
>> 
>>> Am 09.11.2010 um 21:24 schrieb 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.
>>> 
>>> Quoting myself: "I would break ppc64 support"
>>> 
>>> obj-ppc64/openbios-qemu.elf:
>>> EXCEPTION_PREAMBLE => optionally used for qemu-system-ppc64, not yet working
>>> 
>>> obj-ppc/openbios-qemu.elf:
>>> EXCEPTION_PREAMBLE => used for qemu-system-ppc
>>> EXCEPTION_PREAMBLE_64 => currently used for qemu-system-ppc64 by patching the exception vectors based on PVR-determined CPU init function
>>> 
>>> Thus, if we drop the latter before ppc64 is up and running we would no longer have a working openbios-ppc for qemu-system-ppc64.
>> 
>> I still don't see why. We can just use a ppc32 compiled version for working ppc64 support, no? The only reason I put us back to 32bit mode was because otherwise lis -1 would end up being -1 instead of 0xffff0000 :).
>> 
>> So why don't you just keep the current code working, duplicate it for the time being for ppc64 and then once everything works merge it back together?
> 
> Pretty sure I mentioned that alternative already... :)
> 
> http://repo.or.cz/w/openbios/afaerber.git/blobdiff/97003b1ff3860847ce9ad5c6ba13d3860ceb5509..6ef21780f74d36e1d80d745c331a6d4f8d84a610:/arch/ppc/qemu/start.S
> 
> Tried to avoid that for SVN. But if that's consensus, it sure is the easiest for me!

I don't see any better way really. It's a lot easier to worry about those pieces when the rest is good. As long as you promise to clean up the mess once you're ready of course.

So apart from the rfi change which should really be done using an RFI define, that change looks good.


Alex