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.
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...
Also rfid, both here and for the FPU vector.
// PPC32
#define RFID rfi
// PPC64
#define RFID rfid
should work, no?
Yeah, if that's the naming convention we want to go with.
Btw, you should probably define those helpers somewhere generic depending on #ifdef __powerpc64__
Note that the other ppc targets don't use assembler macros, just preprocessor macros, so we have kind of a double-macro situation. ;)
Yes, please don't use assembler macros. If I did that anywhere, I did it wrong :). preprocessor macros are the way to go! :)
Both have their advantages imo when used correctly.
Andreas