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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 24 Oct 2012, Tim Deegan wrote:
> 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@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
> 
> Shouldn't that be wf_i_ne?

Nope. We need wfe here, in fact the corresponding code is
xen/arch/arm/smpboot.c:make_cpus_ready that uses sev to wake the other
cpus up. The code running wfi on secondary cpus is actually in the
firmware.


> >     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)?

Good idea


> > +   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?

Yep

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