This patch series is part of a much larger WIP series designed to remove various hacks from the load/init-program/go words. However in order for the follow-on series to work, all architectures must build an execution context similar to the IEEE1275 specification description of saved-program-state.
PPC is the only architecture which doesn't make use of contexts when entering/leaving the Forth environment, so start by adding this functionality in order that it can be expanded on further with later patches. While no functionality is changed by this patch, there should be no regressions when attempting to boot existing client images.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Mark Cave-Ayland (4): ppc: move call_elf() to separate switch.S file ppc: add new context handler ppc: use separate context to call client images ppc: unify CIF save stack layout with that of new context stack
openbios-devel/arch/ppc/build.xml | 2 + openbios-devel/arch/ppc/qemu/context.c | 138 ++++++++++++++++++ openbios-devel/arch/ppc/qemu/context.h | 34 +++++ openbios-devel/arch/ppc/qemu/init.c | 4 +- openbios-devel/arch/ppc/qemu/ldscript | 6 +- openbios-devel/arch/ppc/qemu/start.S | 244 +++++++++++++------------------- openbios-devel/arch/ppc/qemu/switch.S | 211 +++++++++++++++++++++++++++ openbios-devel/include/arch/ppc/io.h | 2 +- 8 files changed, 492 insertions(+), 149 deletions(-) create mode 100644 openbios-devel/arch/ppc/qemu/context.c create mode 100644 openbios-devel/arch/ppc/qemu/context.h create mode 100644 openbios-devel/arch/ppc/qemu/switch.S
Separate out the context switch code into a separate file, similar as to how is already done with the other architectures.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/build.xml | 1 + openbios-devel/arch/ppc/qemu/start.S | 47 ++--------------------------- openbios-devel/arch/ppc/qemu/switch.S | 52 +++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 44 deletions(-) create mode 100644 openbios-devel/arch/ppc/qemu/switch.S
diff --git a/openbios-devel/arch/ppc/build.xml b/openbios-devel/arch/ppc/build.xml index 29f6601..b40c81c 100644 --- a/openbios-devel/arch/ppc/build.xml +++ b/openbios-devel/arch/ppc/build.xml @@ -182,6 +182,7 @@ $(call quiet-command,$(NM) $@.nostrip | sort > $(ODIR)/openbios-qemu.syms," GEN $(TARGET_DIR)$@.syms") $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="qemu/start.S"/> + <object source="qemu/switch.S"/> <object source="timebase.S"/> <external-object source="libqemu.a"/> <external-object source="libbootstrap.a"/> diff --git a/openbios-devel/arch/ppc/qemu/start.S b/openbios-devel/arch/ppc/qemu/start.S index ae2fd53..483c498 100644 --- a/openbios-devel/arch/ppc/qemu/start.S +++ b/openbios-devel/arch/ppc/qemu/start.S @@ -486,52 +486,11 @@ real_entry: 1: nop b 1b
- - /* According to IEEE 1275, PPC bindings: - * - * MSR = FP, ME + (DR|IR) - * r1 = stack (32 K + 32 bytes link area above) - * r5 = client interface handler - * r6 = address of client program arguments (unused) - * r7 = length of client program arguments (unused) - * - * Yaboot and Linux use r3 and r4 for initrd address and size - */ .data -saved_stack: - DATA_LONG(0) +_GLOBAL(saved_stack): + DATA_LONG(0) + .previous - /* void call_elf( arg1, arg2, entry ) */ -_GLOBAL(call_elf): - mflr r0 - PPC_STLU r1, -STACKFRAME_MINSIZE(r1) - PPC_STL r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1) - mtlr r5 - LOAD_REG_IMMEDIATE(r8, saved_stack) // save our stack pointer - PPC_STL r1,0(r8) - mfsdr1 r1 - addi r1, r1, -32768 /* - 32 KiB exception stack */ - addis r1, r1, -1 /* - 64 KiB stack */ - LOAD_REG_IMMEDIATE(r5, of_client_callback) // r5 = callback - li r6,0 // r6 = address of client program arguments (unused) - li r7,0 // r7 = length of client program arguments (unused) - li r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR - MTMSRD(r0) - blrl - -#ifdef CONFIG_PPC64 - /* Restore SF bit */ - LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR) - MTMSRD(r0) -#endif - LOAD_REG_IMMEDIATE(r8, saved_stack) // restore stack pointer - mr r1,r8 - PPC_LL r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1) - mtlr r0 - addi r1, r1, STACKFRAME_MINSIZE - // XXX: should restore r12-r31 etc.. - // we should not really come here though - blr
#ifdef __powerpc64__ #define STKOFF STACKFRAME_MINSIZE diff --git a/openbios-devel/arch/ppc/qemu/switch.S b/openbios-devel/arch/ppc/qemu/switch.S new file mode 100644 index 0000000..eabd6d0 --- /dev/null +++ b/openbios-devel/arch/ppc/qemu/switch.S @@ -0,0 +1,52 @@ +#include "autoconf.h" +#include "asm/asmdefs.h" +#include "asm/processor.h" + +#ifdef CONFIG_PPC_64BITSUPPORT + #define STACKFRAME_MINSIZE 48 +#else /* !CONFIG_PPC_64BITSUPPORT */ + #define STACKFRAME_MINSIZE 16 +#endif + + /* According to IEEE 1275, PPC bindings: + * + * MSR = FP, ME + (DR|IR) + * r1 = stack (32 K + 32 bytes link area above) + * r5 = client interface handler + * r6 = address of client program arguments (unused) + * r7 = length of client program arguments (unused) + * + * Yaboot and Linux use r3 and r4 for initrd address and size + */ + + /* void call_elf( arg1, arg2, entry ) */ +_GLOBAL(call_elf): + mflr r0 + PPC_STLU r1, -STACKFRAME_MINSIZE(r1) + PPC_STL r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1) + mtlr r5 + LOAD_REG_IMMEDIATE(r8, saved_stack) // save our stack pointer + PPC_STL r1,0(r8) + mfsdr1 r1 + addi r1, r1, -32768 /* - 32 KiB exception stack */ + addis r1, r1, -1 /* - 64 KiB stack */ + LOAD_REG_IMMEDIATE(r5, of_client_callback) // r5 = callback + li r6,0 // r6 = address of client program arguments (unused) + li r7,0 // r7 = length of client program arguments (unused) + li r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR + MTMSRD(r0) + blrl + +#ifdef CONFIG_PPC64 + /* Restore SF bit */ + LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR) + MTMSRD(r0) +#endif + LOAD_REG_IMMEDIATE(r8, saved_stack) // restore stack pointer + mr r1,r8 + PPC_LL r0, (STACKFRAME_MINSIZE + PPC_LR_STKOFF)(r1) + mtlr r0 + addi r1, r1, STACKFRAME_MINSIZE + // XXX: should restore r12-r31 etc.. + // we should not really come here though + blrl
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/build.xml | 1 + openbios-devel/arch/ppc/qemu/context.c | 123 ++++++++++++++++++++++++ openbios-devel/arch/ppc/qemu/context.h | 34 +++++++ openbios-devel/arch/ppc/qemu/ldscript | 6 +- openbios-devel/arch/ppc/qemu/start.S | 2 +- openbios-devel/arch/ppc/qemu/switch.S | 165 +++++++++++++++++++++++++++++++- openbios-devel/include/arch/ppc/io.h | 2 +- 7 files changed, 327 insertions(+), 6 deletions(-) create mode 100644 openbios-devel/arch/ppc/qemu/context.c create mode 100644 openbios-devel/arch/ppc/qemu/context.h
diff --git a/openbios-devel/arch/ppc/build.xml b/openbios-devel/arch/ppc/build.xml index b40c81c..e606313 100644 --- a/openbios-devel/arch/ppc/build.xml +++ b/openbios-devel/arch/ppc/build.xml @@ -183,6 +183,7 @@ $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="qemu/start.S"/> <object source="qemu/switch.S"/> + <object source="qemu/context.c"/> <object source="timebase.S"/> <external-object source="libqemu.a"/> <external-object source="libbootstrap.a"/> diff --git a/openbios-devel/arch/ppc/qemu/context.c b/openbios-devel/arch/ppc/qemu/context.c new file mode 100644 index 0000000..e56da71 --- /dev/null +++ b/openbios-devel/arch/ppc/qemu/context.c @@ -0,0 +1,123 @@ +/* + * context switching + * 2003-10 by SONE Takeshi + */ + +#include "config.h" +#include "kernel/kernel.h" +#include "context.h" +#include "libopenbios/sys_info.h" + +#define MAIN_STACK_SIZE 16384 +#define IMAGE_STACK_SIZE 4096*2 + +#define debug printk + +#ifdef CONFIG_PPC_64BITSUPPORT + #ifdef __powerpc64__ + #define ULONG_SIZE 8 + #define STACKFRAME_MINSIZE 48 + #define STKOFF STACKFRAME_MINSIZE + #define SAVE_SPACE 320 + #else + #define ULONG_SIZE 4 + #define STACKFRAME_MINSIZE 16 + #define STKOFF 8 + #define SAVE_SPACE 144 + #endif +#endif + +static void start_main(void); /* forward decl. */ +void __exit_context(void); /* assembly routine */ + +unsigned int start_elf(unsigned long entry_point, unsigned long param); +void entry(void); + +/* + * Main context structure + * It is placed at the bottom of our stack, and loaded by assembly routine + * to start us up. + */ +static struct context main_ctx = { + .sp = (unsigned long) &_estack - SAVE_SPACE, + .pc = (unsigned long) start_main, + .return_addr = (unsigned long) __exit_context, +}; + +/* This is used by assembly routine to load/store the context which + * it is to switch/switched. */ +struct context *__context = &main_ctx; + +/* Stack for loaded ELF image */ +static uint8_t image_stack[IMAGE_STACK_SIZE]; + +/* Pointer to startup context (physical address) */ +unsigned long __boot_ctx; + +/* + * Main starter + * This is the C function that runs first. + */ +static void start_main(void) +{ + /* Save startup context, so we can refer to it later. + * We have to keep it in physical address since we will relocate. */ + __boot_ctx = virt_to_phys(__context); + + /* Start the real fun */ + entry(); + + /* Returning from here should jump to __exit_context */ + __context = boot_ctx; +} + +/* Setup a new context using the given stack. + */ +struct context * +init_context(uint8_t *stack, uint32_t stack_size, int num_params) +{ + struct context *ctx; + + ctx = (struct context *) + (stack + stack_size - (sizeof(*ctx) + num_params*sizeof(unsigned long))); + memset(ctx, 0, sizeof(*ctx)); + + /* Fill in reasonable default for flat memory model */ + ctx->sp = virt_to_phys(SP_LOC(ctx)); + ctx->return_addr = virt_to_phys(__exit_context); + + return ctx; +} + +/* Switch to another context. */ +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; +} + +/* Start ELF Boot image */ +unsigned int start_elf(unsigned long entry_point, unsigned long param) +{ + struct context *ctx; + + ctx = init_context(image_stack, sizeof image_stack, 1); + ctx->pc = entry_point; + ctx->param[0] = param; + + ctx = switch_to(ctx); + return ctx->regs[REG_R3]; +} diff --git a/openbios-devel/arch/ppc/qemu/context.h b/openbios-devel/arch/ppc/qemu/context.h new file mode 100644 index 0000000..8135bb4 --- /dev/null +++ b/openbios-devel/arch/ppc/qemu/context.h @@ -0,0 +1,34 @@ +#ifndef PPC_CONTEXT_H +#define PPC_CONTEXT_H + +struct context { +#define SP_LOC(ctx) (&(ctx)->sp) + unsigned long _sp; + unsigned long return_addr; + unsigned long sp; + unsigned long pc; + /* General registers */ + unsigned long regs[34]; +#define REG_R3 3 +#define REG_R5 8 +#define REG_R6 9 +#define REG_R7 10 + /* Flags */ + /* Optional stack contents */ + unsigned long param[0]; +}; + +/* Create a new context in the given stack */ +struct context * +init_context(uint8_t *stack, uint32_t stack_size, int num_param); + +/* Switch context */ +struct context *switch_to(struct context *); + +/* Holds physical address of boot context */ +extern unsigned long __boot_ctx; + +/* This can always be safely used to refer to the boot context */ +#define boot_ctx ((struct context *) phys_to_virt(__boot_ctx)) + +#endif /* PPC_CONTEXT_H */ diff --git a/openbios-devel/arch/ppc/qemu/ldscript b/openbios-devel/arch/ppc/qemu/ldscript index 8027b39..11ebf4b 100644 --- a/openbios-devel/arch/ppc/qemu/ldscript +++ b/openbios-devel/arch/ppc/qemu/ldscript @@ -51,7 +51,11 @@ SECTIONS *(.bss) *(.bss.*) *(COMMON) - _ebss = .; + + _stack = .; + . += CSTACK_SIZE; + . = ALIGN(16); + _estack = .; }
. = HRESET_ADDR; diff --git a/openbios-devel/arch/ppc/qemu/start.S b/openbios-devel/arch/ppc/qemu/start.S index 483c498..a62d46e 100644 --- a/openbios-devel/arch/ppc/qemu/start.S +++ b/openbios-devel/arch/ppc/qemu/start.S @@ -482,7 +482,7 @@ real_entry: #endif
bl BRANCH_LABEL(setup_mmu) - bl BRANCH_LABEL(entry) + bl BRANCH_LABEL(__switch_context_nosave) 1: nop b 1b
diff --git a/openbios-devel/arch/ppc/qemu/switch.S b/openbios-devel/arch/ppc/qemu/switch.S index eabd6d0..c3d9d70 100644 --- a/openbios-devel/arch/ppc/qemu/switch.S +++ b/openbios-devel/arch/ppc/qemu/switch.S @@ -2,10 +2,19 @@ #include "asm/asmdefs.h" #include "asm/processor.h"
+ #ifdef CONFIG_PPC_64BITSUPPORT - #define STACKFRAME_MINSIZE 48 -#else /* !CONFIG_PPC_64BITSUPPORT */ - #define STACKFRAME_MINSIZE 16 + #ifdef __powerpc64__ + #define ULONG_SIZE 8 + #define STACKFRAME_MINSIZE 48 + #define STKOFF STACKFRAME_MINSIZE + #define SAVE_SPACE 320 + #else + #define ULONG_SIZE 4 + #define STACKFRAME_MINSIZE 16 + #define STKOFF 8 + #define SAVE_SPACE 144 + #endif #endif
/* According to IEEE 1275, PPC bindings: @@ -50,3 +59,153 @@ _GLOBAL(call_elf): // XXX: should restore r12-r31 etc.. // we should not really come here though blrl + +/* + * Switch execution context + * This saves registers in the stack, then + * switches the stack, and restores everything from the new stack. + * This function takes no argument. New stack pointer is + * taken from global variable __context, and old stack pointer + * is also saved to __context. This way we can just jump to + * this routine to get back to the original context. + */ + +_GLOBAL(__switch_context): + /* save internal stack pointer */ +#ifdef CONFIG_PPC64 + PPC_STL r1, -(SAVE_SPACE + 16) + STKOFF(r1) +#else + PPC_STL r1, -SAVE_SPACE + STKOFF(r1) +#endif + +#ifdef CONFIG_PPC64 + PPC_STLU r1, -(SAVE_SPACE + 16)(r1) +#else + PPC_STLU r1, -SAVE_SPACE(r1) /* fits within alignment */ +#endif + + /* r4, r5 */ + PPC_STL r4, (STKOFF + 9 * ULONG_SIZE)(r1) + PPC_STL r5, (STKOFF + 10 * ULONG_SIZE)(r1) + + /* link register */ + mflr r4 + PPC_STL r4, PPC_LR_STKOFF(r1) + PPC_STL r4, (STKOFF + ULONG_SIZE)(r1) + + PPC_STL r3, (STKOFF + 5 * ULONG_SIZE)(r1) + PPC_STL r2, (STKOFF + 4 * ULONG_SIZE)(r1) + PPC_STL r0, (STKOFF + 3 * ULONG_SIZE)(r1) + + /* ctr, cr and xer */ + mfctr r4 + PPC_STL r4, (STKOFF + 6 * ULONG_SIZE)(r1) + mfcr r4 + PPC_STL r4, (STKOFF + 7 * ULONG_SIZE)(r1) + mfxer r4 + PPC_STL r4, (STKOFF + 8 * ULONG_SIZE)(r1) + + /* r6-r31 */ + PPC_STL r6, (STKOFF + 11 * ULONG_SIZE)(r1) + PPC_STL r7, (STKOFF + 12 * ULONG_SIZE)(r1) + PPC_STL r8, (STKOFF + 13 * ULONG_SIZE)(r1) + PPC_STL r9, (STKOFF + 14 * ULONG_SIZE)(r1) + PPC_STL r10, (STKOFF + 15 * ULONG_SIZE)(r1) + PPC_STL r11, (STKOFF + 16 * ULONG_SIZE)(r1) + PPC_STL r12, (STKOFF + 17 * ULONG_SIZE)(r1) + PPC_STL r13, (STKOFF + 18 * ULONG_SIZE)(r1) + PPC_STL r14, (STKOFF + 19 * ULONG_SIZE)(r1) + PPC_STL r15, (STKOFF + 20 * ULONG_SIZE)(r1) + PPC_STL r16, (STKOFF + 21 * ULONG_SIZE)(r1) + PPC_STL r17, (STKOFF + 22 * ULONG_SIZE)(r1) + PPC_STL r18, (STKOFF + 23 * ULONG_SIZE)(r1) + PPC_STL r19, (STKOFF + 24 * ULONG_SIZE)(r1) + PPC_STL r20, (STKOFF + 25 * ULONG_SIZE)(r1) + PPC_STL r21, (STKOFF + 26 * ULONG_SIZE)(r1) + PPC_STL r22, (STKOFF + 27 * ULONG_SIZE)(r1) + PPC_STL r23, (STKOFF + 28 * ULONG_SIZE)(r1) + PPC_STL r24, (STKOFF + 29 * ULONG_SIZE)(r1) + PPC_STL r25, (STKOFF + 30 * ULONG_SIZE)(r1) + PPC_STL r26, (STKOFF + 31 * ULONG_SIZE)(r1) + PPC_STL r27, (STKOFF + 32 * ULONG_SIZE)(r1) + PPC_STL r28, (STKOFF + 33 * ULONG_SIZE)(r1) + PPC_STL r29, (STKOFF + 34 * ULONG_SIZE)(r1) + PPC_STL r30, (STKOFF + 35 * ULONG_SIZE)(r1) + PPC_STL r31, (STKOFF + 36 * ULONG_SIZE)(r1) + + /* swap context */ + LOAD_REG_IMMEDIATE(r4, __context) + PPC_LL r5, 0(r4) + PPC_STL r1, 0(r4) + mr r4, r5 + + b __set_context + +_GLOBAL(__switch_context_nosave): + LOAD_REG_IMMEDIATE(r4, __context) + PPC_LL r4, 0(r4) + +__set_context: + /* link register */ + PPC_LL r5, (STKOFF + ULONG_SIZE)(r4) + mtlr r5 + + PPC_LL r3, (STKOFF + 5 * ULONG_SIZE)(r4) + PPC_LL r2, (STKOFF + 4 * ULONG_SIZE)(r4) + PPC_LL r0, (STKOFF + 3 * ULONG_SIZE)(r4) + + /* ctr, cr and xer */ + PPC_LL r5, (STKOFF + 6 * ULONG_SIZE)(r4) + mtctr r5 + PPC_LL r5, (STKOFF + 7 * ULONG_SIZE)(r4) + mtcr r5 + PPC_LL r5, (STKOFF + 8 * ULONG_SIZE)(r4) + mtxer r5 + + /* r5-r31 */ + PPC_LL r5, (STKOFF + 10 * ULONG_SIZE)(r4) + PPC_LL r6, (STKOFF + 11 * ULONG_SIZE)(r4) + PPC_LL r7, (STKOFF + 12 * ULONG_SIZE)(r4) + PPC_LL r8, (STKOFF + 13 * ULONG_SIZE)(r4) + PPC_LL r9, (STKOFF + 14 * ULONG_SIZE)(r4) + PPC_LL r10, (STKOFF + 15 * ULONG_SIZE)(r4) + PPC_LL r11, (STKOFF + 16 * ULONG_SIZE)(r4) + PPC_LL r12, (STKOFF + 17 * ULONG_SIZE)(r4) + PPC_LL r13, (STKOFF + 18 * ULONG_SIZE)(r4) + PPC_LL r14, (STKOFF + 19 * ULONG_SIZE)(r4) + PPC_LL r15, (STKOFF + 20 * ULONG_SIZE)(r4) + PPC_LL r16, (STKOFF + 21 * ULONG_SIZE)(r4) + PPC_LL r17, (STKOFF + 22 * ULONG_SIZE)(r4) + PPC_LL r18, (STKOFF + 23 * ULONG_SIZE)(r4) + PPC_LL r19, (STKOFF + 24 * ULONG_SIZE)(r4) + PPC_LL r20, (STKOFF + 25 * ULONG_SIZE)(r4) + PPC_LL r21, (STKOFF + 26 * ULONG_SIZE)(r4) + PPC_LL r22, (STKOFF + 27 * ULONG_SIZE)(r4) + PPC_LL r23, (STKOFF + 28 * ULONG_SIZE)(r4) + PPC_LL r24, (STKOFF + 29 * ULONG_SIZE)(r4) + PPC_LL r25, (STKOFF + 30 * ULONG_SIZE)(r4) + PPC_LL r26, (STKOFF + 31 * ULONG_SIZE)(r4) + PPC_LL r27, (STKOFF + 32 * ULONG_SIZE)(r4) + PPC_LL r28, (STKOFF + 33 * ULONG_SIZE)(r4) + PPC_LL r29, (STKOFF + 34 * ULONG_SIZE)(r4) + PPC_LL r30, (STKOFF + 35 * ULONG_SIZE)(r4) + PPC_LL r31, (STKOFF + 36 * ULONG_SIZE)(r4) + + /* r4, r1 */ + PPC_LL r1, STKOFF(r4) + PPC_LL r4, (STKOFF + 8 * ULONG_SIZE)(r4) + + LOAD_REG_IMMEDIATE(r0, MSR_FP | MSR_ME | MSR_DR | MSR_IR) + MTMSRD(r0) + + blrl + +#ifdef CONFIG_PPC64 + /* Restore SF bit */ + LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR) + MTMSRD(r0) +#endif + +_GLOBAL(__exit_context): + /* Get back to the original context */ + b __switch_context diff --git a/openbios-devel/include/arch/ppc/io.h b/openbios-devel/include/arch/ppc/io.h index 3449c5b..39c60d7 100644 --- a/openbios-devel/include/arch/ppc/io.h +++ b/openbios-devel/include/arch/ppc/io.h @@ -6,7 +6,7 @@ #define NO_QEMU_PROTOS #include "arch/common/fw_cfg.h"
-extern char _start, _end; +extern char _start, _end, _estack; extern unsigned long virt_offset;
#define phys_to_virt(phys) ((void *) ((unsigned long) (phys) - virt_offset))
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/qemu/context.c | 15 +++++++++++++++ openbios-devel/arch/ppc/qemu/init.c | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/openbios-devel/arch/ppc/qemu/context.c b/openbios-devel/arch/ppc/qemu/context.c index e56da71..8227919 100644 --- a/openbios-devel/arch/ppc/qemu/context.c +++ b/openbios-devel/arch/ppc/qemu/context.c @@ -32,6 +32,7 @@ void __exit_context(void); /* assembly routine */
unsigned int start_elf(unsigned long entry_point, unsigned long param); void entry(void); +void of_client_callback(void);
/* * Main context structure @@ -114,8 +115,22 @@ unsigned int start_elf(unsigned long entry_point, unsigned long param) { struct context *ctx;
+ /* According to IEEE 1275, PPC bindings: + * + * MSR = FP, ME + (DR|IR) + * r1 = stack (32 K + 32 bytes link area above) + * r5 = client interface handler + * r6 = address of client program arguments (unused) + * r7 = length of client program arguments (unused) + * + * Yaboot and Linux use r3 and r4 for initrd address and size + */ + ctx = init_context(image_stack, sizeof image_stack, 1); ctx->pc = entry_point; + ctx->regs[REG_R5] = (unsigned int)of_client_callback; + ctx->regs[REG_R6] = 0; + ctx->regs[REG_R7] = 0; ctx->param[0] = param;
ctx = switch_to(ctx); diff --git a/openbios-devel/arch/ppc/qemu/init.c b/openbios-devel/arch/ppc/qemu/init.c index b76c570..f2c8201 100644 --- a/openbios-devel/arch/ppc/qemu/init.c +++ b/openbios-devel/arch/ppc/qemu/init.c @@ -35,6 +35,7 @@ #define NO_QEMU_PROTOS #include "arch/common/fw_cfg.h" #include "arch/ppc/processor.h" +#include "context.h"
#define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
@@ -595,6 +596,7 @@ id_cpu(void) }
static void go(void); +unsigned int start_elf(unsigned long entry_point, unsigned long param);
static void go(void) @@ -609,7 +611,7 @@ go(void) feval("saved-program-state >sps.entry @"); addr = POP();
- call_elf(0, 0, addr); + start_elf((unsigned int)addr, 0); }
static void kvm_of_init(void)
This makes it possible to define a Forth structure in future that can access the contents of either context directly.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/qemu/start.S | 195 ++++++++++++++++------------------ 1 file changed, 94 insertions(+), 101 deletions(-)
diff --git a/openbios-devel/arch/ppc/qemu/start.S b/openbios-devel/arch/ppc/qemu/start.S index a62d46e..33ca1e0 100644 --- a/openbios-devel/arch/ppc/qemu/start.S +++ b/openbios-devel/arch/ppc/qemu/start.S @@ -499,133 +499,126 @@ _GLOBAL(saved_stack): #define STKOFF 8 #define SAVE_SPACE 144 #endif -GLOBL(of_client_callback):
+GLOBL(of_client_callback): #ifdef CONFIG_PPC64 - PPC_STLU r1, -(STACKFRAME_MINSIZE + 16)(r1) + PPC_STLU r1, -(STACKFRAME_MINSIZE + 16)(r1) #else - PPC_STLU r1, -STACKFRAME_MINSIZE(r1) /* fits within alignment */ + PPC_STLU r1, -STACKFRAME_MINSIZE(r1) /* fits within alignment */ #endif
/* save r4 */ - - PPC_STL r4, STKOFF(r1) - + PPC_STL r4, STKOFF(r1) + /* save lr */ - mflr r4 - PPC_STL r4, PPC_LR_STKOFF(r1) - + PPC_STL r4, PPC_LR_STKOFF(r1) + /* restore OF stack */ - LOAD_REG_IMMEDIATE(r4, saved_stack) - PPC_LL r4, 0(r4) - - PPC_STLU r4,-SAVE_SPACE(r4) - PPC_STL r1,(STKOFF)(r4) // save caller stack + PPC_LL r4, 0(r4) + + PPC_STLU r4, -SAVE_SPACE(r4) + PPC_STL r1, (STKOFF)(r4) // save caller stack mr r1,r4 - - PPC_STL r2, (STKOFF + 1 * ULONG_SIZE)(r1) - PPC_STL r0, (STKOFF + 2 * ULONG_SIZE)(r1) - + + PPC_STL r3, (STKOFF + 5 * ULONG_SIZE)(r1) + PPC_STL r2, (STKOFF + 4 * ULONG_SIZE)(r1) + PPC_STL r0, (STKOFF + 3 * ULONG_SIZE)(r1) + /* save ctr, cr and xer */ - mfctr r2 - PPC_STL r2, (STKOFF + 3 * ULONG_SIZE)(r1) + PPC_STL r2, (STKOFF + 6 * ULONG_SIZE)(r1) mfcr r2 - PPC_STL r2, (STKOFF + 4 * ULONG_SIZE)(r1) + PPC_STL r2, (STKOFF + 7 * ULONG_SIZE)(r1) mfxer r2 - PPC_STL r2, (STKOFF + 5 * ULONG_SIZE)(r1) - + PPC_STL r2, (STKOFF + 8 * ULONG_SIZE)(r1) + /* save r5 - r31 */ - - PPC_STL r5, (STKOFF + 6 * ULONG_SIZE)(r1) - PPC_STL r6, (STKOFF + 7 * ULONG_SIZE)(r1) - PPC_STL r7, (STKOFF + 8 * ULONG_SIZE)(r1) - PPC_STL r8, (STKOFF + 9 * ULONG_SIZE)(r1) - PPC_STL r9, (STKOFF + 10 * ULONG_SIZE)(r1) - PPC_STL r10, (STKOFF + 11 * ULONG_SIZE)(r1) - PPC_STL r11, (STKOFF + 12 * ULONG_SIZE)(r1) - PPC_STL r12, (STKOFF + 13 * ULONG_SIZE)(r1) - PPC_STL r13, (STKOFF + 14 * ULONG_SIZE)(r1) - PPC_STL r14, (STKOFF + 15 * ULONG_SIZE)(r1) - PPC_STL r15, (STKOFF + 16 * ULONG_SIZE)(r1) - PPC_STL r16, (STKOFF + 17 * ULONG_SIZE)(r1) - PPC_STL r17, (STKOFF + 18 * ULONG_SIZE)(r1) - PPC_STL r18, (STKOFF + 19 * ULONG_SIZE)(r1) - PPC_STL r19, (STKOFF + 20 * ULONG_SIZE)(r1) - PPC_STL r20, (STKOFF + 21 * ULONG_SIZE)(r1) - PPC_STL r21, (STKOFF + 22 * ULONG_SIZE)(r1) - PPC_STL r22, (STKOFF + 23 * ULONG_SIZE)(r1) - PPC_STL r23, (STKOFF + 24 * ULONG_SIZE)(r1) - PPC_STL r24, (STKOFF + 25 * ULONG_SIZE)(r1) - PPC_STL r25, (STKOFF + 26 * ULONG_SIZE)(r1) - PPC_STL r26, (STKOFF + 27 * ULONG_SIZE)(r1) - PPC_STL r27, (STKOFF + 28 * ULONG_SIZE)(r1) - PPC_STL r28, (STKOFF + 29 * ULONG_SIZE)(r1) - PPC_STL r29, (STKOFF + 30 * ULONG_SIZE)(r1) - PPC_STL r30, (STKOFF + 31 * ULONG_SIZE)(r1) - PPC_STL r31, (STKOFF + 32 * ULONG_SIZE)(r1) - + PPC_STL r5, (STKOFF + 10 * ULONG_SIZE)(r1) + PPC_STL r6, (STKOFF + 11 * ULONG_SIZE)(r1) + PPC_STL r7, (STKOFF + 12 * ULONG_SIZE)(r1) + PPC_STL r8, (STKOFF + 13 * ULONG_SIZE)(r1) + PPC_STL r9, (STKOFF + 14 * ULONG_SIZE)(r1) + PPC_STL r10, (STKOFF + 15 * ULONG_SIZE)(r1) + PPC_STL r11, (STKOFF + 16 * ULONG_SIZE)(r1) + PPC_STL r12, (STKOFF + 17 * ULONG_SIZE)(r1) + PPC_STL r13, (STKOFF + 18 * ULONG_SIZE)(r1) + PPC_STL r14, (STKOFF + 19 * ULONG_SIZE)(r1) + PPC_STL r15, (STKOFF + 20 * ULONG_SIZE)(r1) + PPC_STL r16, (STKOFF + 21 * ULONG_SIZE)(r1) + PPC_STL r17, (STKOFF + 22 * ULONG_SIZE)(r1) + PPC_STL r18, (STKOFF + 23 * ULONG_SIZE)(r1) + PPC_STL r19, (STKOFF + 24 * ULONG_SIZE)(r1) + PPC_STL r20, (STKOFF + 25 * ULONG_SIZE)(r1) + PPC_STL r21, (STKOFF + 26 * ULONG_SIZE)(r1) + PPC_STL r22, (STKOFF + 27 * ULONG_SIZE)(r1) + PPC_STL r23, (STKOFF + 28 * ULONG_SIZE)(r1) + PPC_STL r24, (STKOFF + 29 * ULONG_SIZE)(r1) + PPC_STL r25, (STKOFF + 30 * ULONG_SIZE)(r1) + PPC_STL r26, (STKOFF + 31 * ULONG_SIZE)(r1) + PPC_STL r27, (STKOFF + 32 * ULONG_SIZE)(r1) + PPC_STL r28, (STKOFF + 33 * ULONG_SIZE)(r1) + PPC_STL r29, (STKOFF + 34 * ULONG_SIZE)(r1) + PPC_STL r30, (STKOFF + 35 * ULONG_SIZE)(r1) + PPC_STL r31, (STKOFF + 36 * ULONG_SIZE)(r1) + #ifdef CONFIG_PPC64 - LOAD_REG_IMMEDIATE(r2, of_client_interface) - ld r2, 8(r2) + LOAD_REG_IMMEDIATE(r2, of_client_interface) + ld r2, 8(r2) #endif - bl BRANCH_LABEL(of_client_interface) - + + bl BRANCH_LABEL(of_client_interface) + /* restore r5 - r31 */ - - PPC_LL r5, (STKOFF + 6 * ULONG_SIZE)(r1) - PPC_LL r6, (STKOFF + 7 * ULONG_SIZE)(r1) - PPC_LL r7, (STKOFF + 8 * ULONG_SIZE)(r1) - PPC_LL r8, (STKOFF + 9 * ULONG_SIZE)(r1) - PPC_LL r9, (STKOFF + 10 * ULONG_SIZE)(r1) - PPC_LL r10, (STKOFF + 11 * ULONG_SIZE)(r1) - PPC_LL r11, (STKOFF + 12 * ULONG_SIZE)(r1) - PPC_LL r12, (STKOFF + 13 * ULONG_SIZE)(r1) - PPC_LL r13, (STKOFF + 14 * ULONG_SIZE)(r1) - PPC_LL r14, (STKOFF + 15 * ULONG_SIZE)(r1) - PPC_LL r15, (STKOFF + 16 * ULONG_SIZE)(r1) - PPC_LL r16, (STKOFF + 17 * ULONG_SIZE)(r1) - PPC_LL r17, (STKOFF + 18 * ULONG_SIZE)(r1) - PPC_LL r18, (STKOFF + 19 * ULONG_SIZE)(r1) - PPC_LL r19, (STKOFF + 20 * ULONG_SIZE)(r1) - PPC_LL r20, (STKOFF + 21 * ULONG_SIZE)(r1) - PPC_LL r21, (STKOFF + 22 * ULONG_SIZE)(r1) - PPC_LL r22, (STKOFF + 23 * ULONG_SIZE)(r1) - PPC_LL r23, (STKOFF + 24 * ULONG_SIZE)(r1) - PPC_LL r24, (STKOFF + 25 * ULONG_SIZE)(r1) - PPC_LL r25, (STKOFF + 26 * ULONG_SIZE)(r1) - PPC_LL r26, (STKOFF + 27 * ULONG_SIZE)(r1) - PPC_LL r27, (STKOFF + 28 * ULONG_SIZE)(r1) - PPC_LL r28, (STKOFF + 29 * ULONG_SIZE)(r1) - PPC_LL r29, (STKOFF + 30 * ULONG_SIZE)(r1) - PPC_LL r30, (STKOFF + 31 * ULONG_SIZE)(r1) - PPC_LL r31, (STKOFF + 32 * ULONG_SIZE)(r1) - + PPC_LL r5, (STKOFF + 10 * ULONG_SIZE)(r1) + PPC_LL r6, (STKOFF + 11 * ULONG_SIZE)(r1) + PPC_LL r7, (STKOFF + 12 * ULONG_SIZE)(r1) + PPC_LL r8, (STKOFF + 13 * ULONG_SIZE)(r1) + PPC_LL r9, (STKOFF + 14 * ULONG_SIZE)(r1) + PPC_LL r10, (STKOFF + 15 * ULONG_SIZE)(r1) + PPC_LL r11, (STKOFF + 16 * ULONG_SIZE)(r1) + PPC_LL r12, (STKOFF + 17 * ULONG_SIZE)(r1) + PPC_LL r13, (STKOFF + 18 * ULONG_SIZE)(r1) + PPC_LL r14, (STKOFF + 19 * ULONG_SIZE)(r1) + PPC_LL r15, (STKOFF + 20 * ULONG_SIZE)(r1) + PPC_LL r16, (STKOFF + 21 * ULONG_SIZE)(r1) + PPC_LL r17, (STKOFF + 22 * ULONG_SIZE)(r1) + PPC_LL r18, (STKOFF + 23 * ULONG_SIZE)(r1) + PPC_LL r19, (STKOFF + 24 * ULONG_SIZE)(r1) + PPC_LL r20, (STKOFF + 25 * ULONG_SIZE)(r1) + PPC_LL r21, (STKOFF + 26 * ULONG_SIZE)(r1) + PPC_LL r22, (STKOFF + 27 * ULONG_SIZE)(r1) + PPC_LL r23, (STKOFF + 28 * ULONG_SIZE)(r1) + PPC_LL r24, (STKOFF + 29 * ULONG_SIZE)(r1) + PPC_LL r25, (STKOFF + 30 * ULONG_SIZE)(r1) + PPC_LL r26, (STKOFF + 31 * ULONG_SIZE)(r1) + PPC_LL r27, (STKOFF + 32 * ULONG_SIZE)(r1) + PPC_LL r28, (STKOFF + 33 * ULONG_SIZE)(r1) + PPC_LL r29, (STKOFF + 34 * ULONG_SIZE)(r1) + PPC_LL r30, (STKOFF + 35 * ULONG_SIZE)(r1) + PPC_LL r31, (STKOFF + 36 * ULONG_SIZE)(r1) + /* restore ctr, cr and xer */ - - PPC_LL r2, (STKOFF + 3 * ULONG_SIZE)(r1) + PPC_LL r2, (STKOFF + 6 * ULONG_SIZE)(r1) mtctr r2 - PPC_LL r2, (STKOFF + 4 * ULONG_SIZE)(r1) + PPC_LL r2, (STKOFF + 7 * ULONG_SIZE)(r1) mtcr r2 - PPC_LL r2, (STKOFF + 5 * ULONG_SIZE)(r1) + PPC_LL r2, (STKOFF + 8 * ULONG_SIZE)(r1) mtxer r2 - + /* restore r0 and r2 */ - - PPC_LL r2, (STKOFF + 1 * ULONG_SIZE)(r1) - PPC_LL r0, (STKOFF + 2 * ULONG_SIZE)(r1) - + PPC_LL r2, (STKOFF + 4 * ULONG_SIZE)(r1) + PPC_LL r0, (STKOFF + 3 * ULONG_SIZE)(r1) + /* restore caller stack */ - - PPC_LL r1, (STKOFF)(r1) - - PPC_LL r4, PPC_LR_STKOFF(r1) + PPC_LL r1, (STKOFF)(r1) + + PPC_LL r4, PPC_LR_STKOFF(r1) mtlr r4 - PPC_LL r4, STKOFF(r1) - PPC_LL r1, 0(r1) - + PPC_LL r4, STKOFF(r1) + PPC_LL r1, 0(r1) + blr
/* rtas glue (must be reloctable) */
On May 1, 2016, at 6:23 AM, Mark Cave-Ayland wrote:
+/* Switch to another context. */ +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) : : );
This line breaks compatibility with gcc 4.2.3. A simple fix is this: asm __volatile__ ("mflr %0\n\t" : "=r" (lr)); and this
- asm __volatile__ ("mtlr %0\n\t" : : "r" (lr) : );
asm __volatile__ ("mtlr %0\n\t" : : "r" (lr));
On May 1, 2016, at 6:22 AM, Mark Cave-Ayland wrote:
This patch series is part of a much larger WIP series designed to remove various hacks from the load/init-program/go words. However in order for the follow-on series to work, all architectures must build an execution context similar to the IEEE1275 specification description of saved-program-state.
PPC is the only architecture which doesn't make use of contexts when entering/leaving the Forth environment, so start by adding this functionality in order that it can be expanded on further with later patches. While no functionality is changed by this patch, there should be no regressions when attempting to boot existing client images.
I tried Debian Linux, Mac OS 10.4.0, Darwin 8 ppc, and Mac OS 9.2. All of them did not boot. Sorry. I am using QEMU 2.5.93.
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.
- #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.
Segher
On Sun, May 01, 2016 at 02:29:29PM -0400, Programmingkid wrote:
- asm __volatile__ ("mflr %0\n\t" : "=r" (lr) : : );
This line breaks compatibility with gcc 4.2.3. A simple fix is this:
What is the actual error message?
Does this work with compilers less than eight years old?
The current GCC manual recommends this syntax.
Segher
On Sun, May 01, 2016 at 11:23:01AM +0100, Mark Cave-Ayland wrote:
unsigned int start_elf(unsigned long entry_point, unsigned long param);
@@ -609,7 +611,7 @@ go(void) feval("saved-program-state >sps.entry @"); addr = POP();
- call_elf(0, 0, addr);
- start_elf((unsigned int)addr, 0);
That should cast to unsigned long, instead?
Segher
On May 1, 2016, at 4:12 PM, Segher Boessenkool wrote:
On Sun, May 01, 2016 at 02:29:29PM -0400, Programmingkid wrote:
- asm __volatile__ ("mflr %0\n\t" : "=r" (lr) : : );
This line breaks compatibility with gcc 4.2.3. A simple fix is this:
What is the actual error message?
Sure Segher here they are:
/openbios-devel/arch/ppc/qemu/context.c:104: error: expected string literal before ‘)’ token
/openbios-devel/arch/ppc/qemu/context.c:106: error: expected string literal before ‘)’ token make[1]: *** [target/arch/ppc/qemu/context.o] Error 1 make: *** [subdir-ppc] Error 2
On May 1, 2016, at 4:18 PM, Segher Boessenkool wrote:
On Sun, May 01, 2016 at 11:23:01AM +0100, Mark Cave-Ayland wrote:
unsigned int start_elf(unsigned long entry_point, unsigned long param);
@@ -609,7 +611,7 @@ go(void) feval("saved-program-state >sps.entry @"); addr = POP();
- call_elf(0, 0, addr);
- start_elf((unsigned int)addr, 0);
That should cast to unsigned long, instead?
The ppc target is only 32 bits, and so it int. Did you want this code to work on ppc64?
On Sun, May 01, 2016 at 04:35:53PM -0400, Programmingkid wrote:
On May 1, 2016, at 4:18 PM, Segher Boessenkool wrote:
On Sun, May 01, 2016 at 11:23:01AM +0100, Mark Cave-Ayland wrote:
unsigned int start_elf(unsigned long entry_point, unsigned long param);
@@ -609,7 +611,7 @@ go(void) feval("saved-program-state >sps.entry @"); addr = POP();
- call_elf(0, 0, addr);
- start_elf((unsigned int)addr, 0);
That should cast to unsigned long, instead?
The ppc target is only 32 bits, and so it int. Did you want this code to work on ppc64?
The declaration is "long" (as it should be).
Segher
On 01/05/16 19:33, Programmingkid wrote:
On May 1, 2016, at 6:22 AM, Mark Cave-Ayland wrote:
This patch series is part of a much larger WIP series designed to remove various hacks from the load/init-program/go words. However in order for the follow-on series to work, all architectures must build an execution context similar to the IEEE1275 specification description of saved-program-state.
PPC is the only architecture which doesn't make use of contexts when entering/leaving the Forth environment, so start by adding this functionality in order that it can be expanded on further with later patches. While no functionality is changed by this patch, there should be no regressions when attempting to boot existing client images.
I tried Debian Linux, Mac OS 10.4.0, Darwin 8 ppc, and Mac OS 9.2. All of them did not boot. Sorry. I am using QEMU 2.5.93.
Hmmm I see. My local build wasn't clean in that I still had debugging enabled in Makefile.target which seems to be required for this to work. Thanks for testing, will follow up separately.
ATB,
Mark.
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.
On 01/05/16 21:18, Segher Boessenkool wrote:
On Sun, May 01, 2016 at 11:23:01AM +0100, Mark Cave-Ayland wrote:
unsigned int start_elf(unsigned long entry_point, unsigned long param);
@@ -609,7 +611,7 @@ go(void) feval("saved-program-state >sps.entry @"); addr = POP();
- call_elf(0, 0, addr);
- start_elf((unsigned int)addr, 0);
That should cast to unsigned long, instead?
Duh, yes. That's just a plain old bug.
ATB,
Mark.
On Mon, May 02, 2016 at 09:52:01AM +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?
Oh, yeah, that's another problem: a call can clobber all volatile (caller-save) registers. The best solution is to write this whole thing in actual assembler, instead of fighting the compiler to get just the assembler you want ;-)
Segher
On 02/05/16 18:35, Segher Boessenkool wrote:
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?
Oh, yeah, that's another problem: a call can clobber all volatile (caller-save) registers. The best solution is to write this whole thing in actual assembler, instead of fighting the compiler to get just the assembler you want ;-)
Heh actually the bug was a missing volatile qualifier from the __context pointer which is interesting since this code is used almost boilerplate for other architectures - I'll spin up a patch to update the other archs once this has gone in. The v2 version of the patchset contains this correction though.
ATB,
Mark.
On Tue, May 03, 2016 at 09:03:48PM +0100, Mark Cave-Ayland wrote:
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?
Oh, yeah, that's another problem: a call can clobber all volatile (caller-save) registers. The best solution is to write this whole thing in actual assembler, instead of fighting the compiler to get just the assembler you want ;-)
Heh actually the bug was a missing volatile qualifier from the __context pointer which is interesting since this code is used almost boilerplate for other architectures - I'll spin up a patch to update the other archs once this has gone in. The v2 version of the patchset contains this correction though.
That is just a workaround. It might well work, and it can also break at any time.
Segher
On 03/05/16 22:06, Segher Boessenkool wrote:
On Tue, May 03, 2016 at 09:03:48PM +0100, Mark Cave-Ayland wrote:
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?
Oh, yeah, that's another problem: a call can clobber all volatile (caller-save) registers. The best solution is to write this whole thing in actual assembler, instead of fighting the compiler to get just the assembler you want ;-)
Heh actually the bug was a missing volatile qualifier from the __context pointer which is interesting since this code is used almost boilerplate for other architectures - I'll spin up a patch to update the other archs once this has gone in. The v2 version of the patchset contains this correction though.
That is just a workaround. It might well work, and it can also break at any time.
Can you explain further? The v2 patch uses a single register marked as clobber for mflr/mtlr and the volatile modifier for __context ensures that the old value being written to the stack doesn't get optimised away by the compiler. What else am I missing?
ATB,
Mark.
On Tue, May 03, 2016 at 10:12:09PM +0100, Mark Cave-Ayland wrote:
Oh, yeah, that's another problem: a call can clobber all volatile (caller-save) registers. The best solution is to write this whole thing in actual assembler, instead of fighting the compiler to get just the assembler you want ;-)
Heh actually the bug was a missing volatile qualifier from the __context pointer which is interesting since this code is used almost boilerplate for other architectures - I'll spin up a patch to update the other archs once this has gone in. The v2 version of the patchset contains this correction though.
That is just a workaround. It might well work, and it can also break at any time.
Can you explain further? The v2 patch uses a single register marked as clobber for mflr/mtlr and the volatile modifier for __context ensures that the old value being written to the stack doesn't get optimised away by the compiler. What else am I missing?
A normal (ABI-following) function can clobber all of r9..r12, so that "bl" instruction can, too. If the function it calls is special (so at least written as assembler), things are different of course.
Segher
On 04/05/16 16:31, Segher Boessenkool wrote:
Can you explain further? The v2 patch uses a single register marked as clobber for mflr/mtlr and the volatile modifier for __context ensures that the old value being written to the stack doesn't get optimised away by the compiler. What else am I missing?
A normal (ABI-following) function can clobber all of r9..r12, so that "bl" instruction can, too. If the function it calls is special (so at least written as assembler), things are different of course.
Right, we're talking about this section of code in the caller:
asm __volatile__ ("mflr %%r9\n\t" "stw %%r9, %0\n\t" "bl __switch_context\n\t" "lwz %%r9, %0\n\t" "mtlr %%r9\n\t" : "=m" (lr) : "m" (lr) : "%r9" );
The mflr/mtlr are being used to preserve the original link register before branching to the assembler routine at __switch_context which does all the hard work.
ATB,
Mark.
On Wed, May 04, 2016 at 07:30:03PM +0100, Mark Cave-Ayland wrote:
On 04/05/16 16:31, Segher Boessenkool wrote:
Can you explain further? The v2 patch uses a single register marked as clobber for mflr/mtlr and the volatile modifier for __context ensures that the old value being written to the stack doesn't get optimised away by the compiler. What else am I missing?
A normal (ABI-following) function can clobber all of r9..r12, so that "bl" instruction can, too. If the function it calls is special (so at least written as assembler), things are different of course.
[ r3..r12, I can't type ]
Right, we're talking about this section of code in the caller:
asm __volatile__ ("mflr %%r9\n\t" "stw %%r9, %0\n\t" "bl __switch_context\n\t" "lwz %%r9, %0\n\t" "mtlr %%r9\n\t" : "=m" (lr) : "m" (lr) : "%r9" );The mflr/mtlr are being used to preserve the original link register before branching to the assembler routine at __switch_context which does all the hard work.
And the bl clobbers at least r4,r5.
Just write this in actual assembler...
Segher
On 04/05/16 19:40, Segher Boessenkool wrote:
On Wed, May 04, 2016 at 07:30:03PM +0100, Mark Cave-Ayland wrote:
On 04/05/16 16:31, Segher Boessenkool wrote:
Can you explain further? The v2 patch uses a single register marked as clobber for mflr/mtlr and the volatile modifier for __context ensures that the old value being written to the stack doesn't get optimised away by the compiler. What else am I missing?
A normal (ABI-following) function can clobber all of r9..r12, so that "bl" instruction can, too. If the function it calls is special (so at least written as assembler), things are different of course.
[ r3..r12, I can't type ]
Right, we're talking about this section of code in the caller:
asm __volatile__ ("mflr %%r9\n\t" "stw %%r9, %0\n\t" "bl __switch_context\n\t" "lwz %%r9, %0\n\t" "mtlr %%r9\n\t" : "=m" (lr) : "m" (lr) : "%r9" );The mflr/mtlr are being used to preserve the original link register before branching to the assembler routine at __switch_context which does all the hard work.
And the bl clobbers at least r4,r5.
I'm sorry but I still don't understand? __switch_context loads a completely new CPU state from the stack in order to switch context so actually every single register gets clobbered. But we preserve the original link register in a local variable to ensure that we can jump back to where we were upon return.
ATB,
Mark.
On Wed, May 04, 2016 at 07:54:03PM +0100, Mark Cave-Ayland wrote:
And the bl clobbers at least r4,r5.
I'm sorry but I still don't understand? __switch_context loads a completely new CPU state from the stack in order to switch context so actually every single register gets clobbered. But we preserve the original link register in a local variable to ensure that we can jump back to where we were upon return.
If your asm clobbers all regs, the asm needs to say it does.
So this function clobbers all callee-save registers, but you want it to return to the caller anyway? And that is expected to work?
Segher
On Wed, May 04, 2016 at 02:07:06PM -0500, Segher Boessenkool wrote:
On Wed, May 04, 2016 at 07:54:03PM +0100, Mark Cave-Ayland wrote:
And the bl clobbers at least r4,r5.
I'm sorry but I still don't understand? __switch_context loads a completely new CPU state from the stack in order to switch context so actually every single register gets clobbered. But we preserve the original link register in a local variable to ensure that we can jump back to where we were upon return.
If your asm clobbers all regs, the asm needs to say it does.
So this function clobbers all callee-save registers, but you want it to return to the caller anyway? And that is expected to work?
Mark and I talked on IRC. It turns out __switch_context does not switch the context, but also calls the target and switches the context back. So there are no such issues; and we don't need to do the call in asm at all (pure C would work fine), but that is a detail.
Segher
-
Mark Cave-Ayland -
Programmingkid -
Segher Boessenkool