[U-Boot] [RFC PATCH 4/6] ARM: add SMP support for non-secure switch

[Christoffer Dall <cdall@cs.columbia.edu>]

On Mon, May 06, 2013 at 03:19:40PM +0200, Andre Przywara wrote:
> On 04/27/2013 12:13 AM, Christoffer Dall wrote:
> >On Fri, Apr 26, 2013 at 6:14 AM, Andre Przywara
> ><andre.przywara@linaro.org> wrote:
> >>Currently the non-secure switch is only done for the boot processor.
> >>To later allow full SMP support, we have to switch all secondary
> >>cores into non-secure state also.
> >>
> >>So we add an entry point for secondary CPUs coming out of low-power
> >>state and make sure we put them into WFI again after having switched
> >>to non-secure state.
> >>For this we acknowledge and EOI the wake-up IPI, then go into WFI.
> >>Once being kicked out of it later, we sanity check that the start
> >>address has actually been changed (since another attempt to switch
> >>to non-secure would block the core) and jump to the new address.
> >>
> >>The actual CPU kick is done by sending an inter-processor interrupt
> >>via the GIC to all CPU interfaces except the requesting processor.
> >>The secondary cores will then setup their respective GIC CPU
> >>interface.
> >>
> >>The address secondary cores jump to is board specific, we provide
> >>the value here for the Versatile Express board.
> >>
> >>Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >>---
> >>  arch/arm/cpu/armv7/start.S          | 27 ++++++++++++++++++++++++++-
> >>  arch/arm/lib/virt-v7.c              | 10 +++++++++-
> >>  include/configs/vexpress_ca15_tc2.h |  1 +
> >>  3 files changed, 36 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >>index 401b0eb..2b47881 100644
> >>--- a/arch/arm/cpu/armv7/start.S
> >>+++ b/arch/arm/cpu/armv7/start.S
> >>@@ -563,8 +563,19 @@ fiq:
> >>  #endif /* CONFIG_SPL_BUILD */
> >>
> >>  /* Routine to initialize GIC CPU interface and switch to nonsecure state.
> >>+ * Will be executed directly by secondary CPUs after coming out of
> >>+ * WFI, or can be called directly by C code for CPU 0.
> >>+ * Those two paths mandate to not use any stack and to only use registers
> >>+ * r0-r3 to comply with both the C ABI and the requirement of SMP startup
> >>+ * code.
> >>   */
> >>  .globl _nonsec_gic_switch
> >>+.globl _smp_pen
> >>+_smp_pen:
> >>+       mrs     r0, cpsr
> >>+       orr     r0, r0, #0xc0
> >>+       msr     cpsr, r0                        @ disable interrupts
> >>+       mov     lr, #0                          @ clear LR to mark secondary
> >>  _nonsec_gic_switch:
> >>         mrc     p15, 4, r2, c15, c0, 0          @ r2 = PERIPHBASE
> >>         add     r3, r2, #0x1000                 @ GIC dist i/f offset
> >>@@ -605,4 +616,18 @@ _nonsec_gic_switch:
> >>         add     r2, r2, #0x1000                 @ GIC dist i/f offset
> >>         str     r1, [r2]                        @ allow private interrupts
> >>
> >>-       mov     pc, lr
> >>+       cmp     lr, #0
> >>+       movne   pc, lr                          @ CPU 0 to return
> >>+                                               @ all others: go to sleep
> >>+_ack_int:
> >>+       ldr     r1, [r3, #0x0c]                 @ read GICD acknowledge
> >>+       str     r1, [r3, #0x10]                 @ write GICD EOI
> >>+
> >>+       adr     r1, _smp_pen
> >>+waitloop:
> >>+       wfi
> >>+       ldr     r0, =CONFIG_SYSFLAGS_ADDR       @ load start address
> >>+       ldr     r0, [r0]
> >>+       cmp     r0, r1                  @ make sure we dont execute this code
> >>+       beq     waitloop                @ again (due to a spurious wakeup)
> >>+       mov     pc, r0
> >>diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
> >>index 416ca29..5ca093a 100644
> >>--- a/arch/arm/lib/virt-v7.c
> >>+++ b/arch/arm/lib/virt-v7.c
> >>@@ -29,6 +29,7 @@
> >>
> >>  /* the assembly routine doing the actual work in start.S */
> >>  void _nonsec_gic_switch(void);
> >>+void _smp_pen(void);
> >>
> >>  #define GICD_CTLR      0x000
> >>  #define GICD_TYPER     0x004
> >>@@ -51,6 +52,7 @@ int armv7_switch_nonsec(void)
> >>         unsigned int reg;
> >>         volatile unsigned int *gicdptr;
> >>         unsigned itlinesnr, i;
> >>+       unsigned int *sysflags;
> >>
> >>         /* check whether the CPU supports the security extensions */
> >>         asm("mrc p15, 0, %0, c0, c1, 1\n" : "=r"(reg));
> >>@@ -109,7 +111,13 @@ int armv7_switch_nonsec(void)
> >>         for (i = 0; i <= itlinesnr; i++)
> >>                 gicdptr[GICD_IGROUPR0 / 4 + i] = (unsigned)-1;
> >>
> >>-       /* call the non-sec switching code on this CPU */
> >>+       /* now kick all CPUs (expect this one) by writing to GICD_SIGR */
> >>+       sysflags = (void *)CONFIG_SYSFLAGS_ADDR;
> >>+       sysflags[1] = (unsigned)-1;
> >>+       sysflags[0] = (uintptr_t)_smp_pen;
> >>+       gicdptr[GICD_SGIR / 4] = 1U << 24;
> >>+
> >>+       /* call the non-sec switching code on this CPU also */
> >>         _nonsec_gic_switch();
> >>
> >>         return 0;
> >>diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
> >>index 9e230ad..210a27c 100644
> >>--- a/include/configs/vexpress_ca15_tc2.h
> >>+++ b/include/configs/vexpress_ca15_tc2.h
> >>@@ -32,5 +32,6 @@
> >>  #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
> >>
> >>  #define CONFIG_SYS_CLK_FREQ 24000000
> >>+#define CONFIG_SYSFLAGS_ADDR 0x1c010030
> >>
> >>  #endif
> >>--
> >
> >
> >hmm, is all this likely to work across all armv7 cores?
> 
> Probably not.
> 
> >I imagined that the SMP boot protocol could be potentially vastly different on
> >different implementations and this stuff would have to call into board
> >specific code...?
> 
> Right. For now I am trying to get this code upstream. Support for
> other boards can then be merged in on demand. Looks like at least
> the Arndale board can be kicked very similarly, only the SYSFLAGS
> address is different. I am willing to re-architecture the SMP code
> as needed.
> 
Sorry for the late response, I've been out sick.

So, I'm not very familiar with conventions and practices for the U-boot
project, but it seems quite clear to me that since these changes go in
arch/arm/lib/... paths those changes must be generic.

Therefore, I'm pretty sure that you need to factor out anything that's
specific to vexpress into a part of the code that's specific to vexpress
and thereby establish a board interface that generic code can call into
for other boards too, for example Arndale; in fact it would be cool if
you could show this working for both platforms at the same time, since
the working code is already available.

-Christoffer