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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-10-24 at 16:03 +0100, Stefano Stabellini wrote:
> Secondary cpus are held by the firmware until we send an IPI to them.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  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
>       bne   1b

This is a semi-independent bug fix relating to unnecessarily waiting
(and perhaps blocking indefinitely) on the first iteration of the loop?

>  
>  boot_cpu:
> @@ -98,6 +98,10 @@ boot_cpu:
>       PRINT(" booting -\r\n")
>  #endif
>  
> +     /* Wake up secondary cpus */
> +     teq   r12, #0
> +     bleq  kick_cpus

Does this have to be done this early? Couldn't we defer it to C land
where it would be easier to isolate the processor/platform specific
behaviour?

>       /* 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:

My understanding was the mode_switch.S was intended to be a place to
hold the hacks and fixups required because there is no firmware on the
models and/or to address short comings in the firmware. This kick
function is a normal/expected part of booting on a vexpress so I don't
think it especially belongs here.

> +    /* write start paddr to v2m sysreg FLAGSSET register */
> +     ldr   r0, =0x1c010000 /* base V2M sysreg MMIO address */
> +     add   r1, r0, #0x34   /* SYS_FLAGSCLR register */

As well as Tim's comment about using the GICD_* registers you should
define names for the V2M registers somewhere too.

> +     dsb
> +     mov   r2, #0xffffffff
> +     str   r2, [r1]
> +     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 */
> +     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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel