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

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2012-10-25 at 18:45 +0100, Stefano Stabellini wrote:
> > >  
> > >  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.

Hrm, so maybe this stuff does belong in mode_switch.S after all?

Or is there perhaps some register (e.g. in the GIC?) which would allow
non-secure hyp mode to sent an event to a processor in secure monitor
mode? Or are secondary CPUs actually spinning in secure supervisor mode?

I guess this works in Linux because the boot CPU is in *secure* kernel
mode and that is allowed to send events to other secure modes? That's a
further argument that this is related to the firmware not bringing us up
in Hyp / NS mode and therefore that the fix should be in mode_switch.S.

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

proc-ca15.S perhaps? So we can add proc-exynos.S etc in the future?

Ian