Re: [PATCH 5/7] xen/arm: wake up secondary cpus

[Tim Deegan <tim@xen.org>]

At 16:03 +0100 on 24 Oct (1351094625), Stefano Stabellini wrote:
> Secondary cpus are held by the firmware until we send an IPI to them.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/head.S        |    8 ++++++--
>  xen/arch/arm/mode_switch.S |   31 +++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index c784f4d..39c4774 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -79,12 +79,12 @@ start:
>  	beq   boot_cpu               /* If we're CPU 0, boot now */
>  
>  	/* Non-boot CPUs wait here to be woken up one at a time. */
> -1:	wfe
> -	dsb
> +1:	dsb
>  	ldr   r0, =smp_up_cpu        /* VA of gate */
>  	add   r0, r0, r10            /* PA of gate */
>  	ldr   r1, [r0]               /* Which CPU is being booted? */
>  	teq   r1, r12                /* Is it us? */
> +	wfene

Shouldn't that be wf_i_ne?

>  	bne   1b
>  
>  boot_cpu:
> @@ -98,6 +98,10 @@ boot_cpu:
>  	PRINT(" booting -\r\n")
>  #endif
>  
> +	/* Wake up secondary cpus */
> +	teq   r12, #0
> +	bleq  kick_cpus
> +
>  	/* Check that this CPU has Hyp mode */
>  	mrc   CP32(r0, ID_PFR1)
>  	and   r0, r0, #0xf000        /* Bits 12-15 define virt extensions */
> diff --git a/xen/arch/arm/mode_switch.S b/xen/arch/arm/mode_switch.S
> index 83a682b..39d80e8 100644
> --- a/xen/arch/arm/mode_switch.S
> +++ b/xen/arch/arm/mode_switch.S
> @@ -21,6 +21,37 @@
>  #include <asm/page.h>
>  #include <asm/asm_defns.h>
>  
> +/* XXX: Versatile Express specific code */
> +/* wake up secondary cpus */
> +.globl kick_cpus
> +kick_cpus:
> +    /* write start paddr to v2m sysreg FLAGSSET register */
> +	ldr   r0, =0x1c010000 /* base V2M sysreg MMIO address */
> +	add   r1, r0, #0x34   /* SYS_FLAGSCLR register */
> +	dsb
> +	mov   r2, #0xffffffff
> +	str   r2, [r1]

Why not str r2, [r0, #0x34] and save the explicit add (and similarly below)?

> +	dsb
> +	add   r1, r0, #0x30   /* SYS_FLAGSSET register */
> +	ldr   r2, =start
> +	add   r2, r2, r10
> +	str   r2, [r1]
> +	dsb
> +	/* send an interrupt */
> +	ldr   r0, =0x2c001000 /* base GICD MMIO address */
> +	mov   r1, r0
> +	mov   r2, #0x1        /* GICD_CTLR */
> +	str   r2, [r1]        /* enable distributor */
> +	add   r1, r0, #0xf00  /* GICD_SGIR */

These GIGD_* addresses and constants are already defined in config.h and
gic.h; can you reuse those definitions?

Tim.

> +	mov   r2, #0xfe0000
> +	str   r2, [r1]        /* send IPI to everybody */
> +	dsb
> +	mov   r1, r0
> +	mov   r2, #0x0        /* GICD_CTLR */
> +	str   r2, [r1]        /* disable distributor */
> +	mov   pc, lr
> +
> +
>  /* Get up a CPU into Hyp mode.  Clobbers r0-r3.
>   *
>   * Expects r12 == CPU number
> -- 
> 1.7.2.5
>