On Thu, 21 Jul 2011 03:27:12 +0200
Alexander Graf <ag...@suse.de> wrote:
> When directly starting an SMP system with -kernel on PPC e500, we need to
> simulate the spin table code from u-boot. This code adds a small c file
> plus generated .elf file that enable secondary CPUs to spin just like they
> would with u-boot.
Why not just handle the spin table as an MMIO region?
Besides being simpler, it avoids any spinning overhead if the guest doesn't
spin up all the cpus.
> diff --git a/pc-bios/ppc_spin.c b/pc-bios/ppc_spin.c
> new file mode 100644
> index 0000000..e46a6a7
> --- /dev/null
> +++ b/pc-bios/ppc_spin.c
> @@ -0,0 +1,97 @@
> +#include <stdio.h>
> +#include <stdint.h>
> +
> +/*
> + * Secondary CPU spin code
> + *
> + * Compile using: gcc -m32 -nostdlib ppc_spin.c -o ppc_spin.elf -Os \
> + * -fno-stack-protector -Wl,-Ttext,0x7700000
> + */
> +
> +/* Initialize stack pointer */
> +asm (
> +" .global _start \n"
> +" _start: \n"
> +" addis 1, 3, 0x10000@h \n"
> +" b spin \n");
> +
> +typedef struct spin_info {
> + uint32_t addr_hi;
> + uint32_t addr;
> + uint32_t r3_hi;
> + uint32_t r3;
> + uint32_t resv;
> + uint32_t pir;
> + uint32_t r6_hi;
> + uint32_t r6;
> +} SpinInfo;
> +
> +#define __stringify_1(x...) #x
> +#define __stringify(x...) __stringify_1(x)
> +
> +#define mfspr(rn) ({unsigned long rval; \
> + asm volatile("mfspr %0," __stringify(rn) \
> + : "=r" (rval)); rval;})
> +#define mtspr(rn, v) asm volatile("mtspr " __stringify(rn) ",%0" : : "r"
> (v)\
> + : "memory")
> +static inline void mtdec(unsigned long v)
> +{
> + asm volatile("mtdec %0" : : "r" (v));
> +}
> +
> +static inline void mtmsr(unsigned long msr)
> +{
> + asm("mtmsr %0" : : "r"(msr));
> +}
> +
> +#define __MASK(X) (1UL<<(X))
> +#define MSR_WE_LG 18 /* Wait State Enable */
> +#define MSR_EE_LG 15 /* External Interrupt Enable */
> +#define MSR_WE __MASK(MSR_WE_LG) /* Wait State Enable */
> +#define MSR_EE __MASK(MSR_EE_LG) /* External Interrupt
> Enable */
> +#define SPR_PIR 0x11E
> +#define SPR_HID0 (0x3F0)
> +#define SPR_BOOKE_IVOR10 (0x19A)
> +#define SPR_BOOKE_IVPR (0x03F)
> +
> +void loop(void);
> +
> +__attribute__((noreturn)) void spin(unsigned long ptr)
> +{
> + volatile SpinInfo *info = (void*)ptr;
> + uint32_t pir = mfspr(SPR_PIR);
> + __attribute__((noreturn)) void (*entry)(
> + unsigned long r3, unsigned long r4, unsigned long r5, unsigned long
> r6,
> + unsigned long r7, unsigned long r8, unsigned long r9);
> + unsigned long dec_p = (unsigned long)loop;
> +
> + info->pir = pir;
> + info->r3 = pir;
> + info->addr = 1;
> + info->r6 = 0;
> +
> + /* we don't want to keep the other CPUs from running, so set the IVOR for
> + DEC to our loop and only check for info->addr every other cycle */
> +
> + mtspr(SPR_HID0, 0x00E00000);
> + mtspr(SPR_BOOKE_IVOR10, dec_p & 0xfff);
> + mtspr(SPR_BOOKE_IVPR, dec_p & ~0xfff);
> +loop:
> + asm volatile (".global loop\n"
> + " loop:" : : : "memory", "cc");
You're jumping through a lot of hoops to (nominally) do this in C.
> + info->pir = pir;
While I'm fine with not allowing the guest to set PIR (the ISA says it's
read-only in virtualized implementations), I'm not sure we should be
overwriting the spintable value here.
> + entry = (void*)(unsigned long)info->addr;
You're ignoring addr_hi, and you should create an IMA appropriate for the
guest's chosen entry point rather than assume it's in the first 64M.
And don't forget about r3_hi and r6_hi on future 64-bit targets.
The function pointer approach might also run into trouble on 64-bit, due
to ABI weirdness.
> diff --git a/pc-bios/ppc_spin.elf b/pc-bios/ppc_spin.elf
> new file mode 100755
> index
> 0000000000000000000000000000000000000000..71c872b2d4685100b0050d549735662d7d763e08
> GIT binary patch
> literal 66553
Must this go into the tree as a binary? Why can't it be built when qemu is
built (if it's needed at all)?
This has proven to be rather annoying with the dtb as well.
-Scott
