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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 25 Oct 2012, Ian Campbell wrote:
> 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?

yes


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

Yes, it does because we need to send an interrupt to cpus running in
secure mode, so this has to happen before we drop off secure state and we
enter hypervisor state.


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

I have created a processor.S file for processor specific initializations
(see ACTLR), maybe I can move it there.


> > +    /* 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.

sure

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