On Thu, Jun 13, 2013 at 01:01:09PM +0200, Andre Przywara wrote: > While actually switching to non-secure state is one thing, the > more important part of this process is to make sure that we still super nit: not sure it's more important - it's just another thing we need to do. > have full access to the interrupt controller (GIC). > The GIC is fully aware of secure vs. non-secure state, some > registers are banked, others may be configured to be accessible from > secure state only. > To be as generic as possible, we get the GIC memory mapped address > based on the PERIPHBASE value in the CBAR register. Since this > register is not architecturally defined, we check the MIDR before to > be from an A15 or A7. > For CPUs not having the CBAR or boards with wrong information herein > we allow providing the base address as a configuration variable. > > With the GIC accessible, we: "With the GIC accessible" ? > a) allow private interrupts to be delivered to the core > (GICD_IGROUPR0 = 0xFFFFFFFF) > b) enable the CPU interface (GICC_CTLR[0] = 1) > c) set the priority filter to allow non-secure interrupts > (GICC_PMR = 0xFF) > > After having switched to non-secure state, we also enable the > non-secure GIC CPU interface, since this register is banked. > > Also we allow access to all coprocessor interfaces from non-secure > state by writing the appropriate bits in the NSACR register. super nit 2: move this last paragraph above the non-secure stuff, so there's no confusion that this is done from secure mode. > > Since we need to call this routine also directly from the smp_pen > later (where we don't have any stack), we can only use caller saved > registers r0-r3 and r12 to not disturb the compiler. the compiler certainly does seem to get cranky when we disturb it ;) > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx> > --- > arch/arm/cpu/armv7/nonsec_virt.S | 66 ++++++++++++++++++++++++++++++++++++++++ > arch/arm/include/asm/armv7.h | 16 ++++++++++ > arch/arm/include/asm/gic.h | 17 +++++++++++ > 3 files changed, 99 insertions(+) > create mode 100644 arch/arm/include/asm/gic.h > > diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S > index f5572f5..656d99b 100644 > --- a/arch/arm/cpu/armv7/nonsec_virt.S > +++ b/arch/arm/cpu/armv7/nonsec_virt.S > @@ -23,6 +23,8 @@ > */ > > #include <config.h> > +#include <asm/gic.h> > +#include <asm/armv7.h> > > /* the vector table for secure state */ > _secure_vectors: > @@ -52,3 +54,67 @@ _secure_monitor: > > movs pc, lr @ return to non-secure SVC > > +#define lo(x) ((x) & 0xFFFF) > +#define hi(x) ((x) >> 16) > + > +/* > + * Routine to switch a core to non-secure state. > + * Initializes the GIC per-core interface, allows coprocessor access in > + * non-secure modes and uses smc #0 to do the non-secure transition. > + * To be called by smp_pen for secondary cores and directly for the BSP. > + * For those two cases to work we must not use any stack and thus are > + * limited to the caller saved registers r0-r3. you also use r12 (ip) ? Also, I think you can rewrite this comment to make it a little nicer. May I propose something along the lines of: /* * Switch a core to non-secure state. * * 1. initialize the GIC per-core interface * 2. allow coprocessor access in non-secure modes * 3. switch the cpu mode (by calling "smc #0") * * Called from smp_pen by non-primary cores and directly by the BSP. * Do not assume that the stack is available and only use registers * r0-r3. * * PERIPHBASE is used to get the GIC address. This could be 40 bits long, * though, but we check this in C before calling this function. */ (I only propose this to match the high standard of these patches) > + * PERIPHBASE is used to get the GIC address. This could be 40 bits long, > + * though, but we check this in C before calling this function. > + */ > +.globl _nonsec_init > +_nonsec_init: > +#ifdef CONFIG_ARM_GIC_BASE_ADDRESS > + ldr r2, =CONFIG_ARM_GIC_BASE_ADDRESS > +#else > + mrc p15, 4, r2, c15, c0, 0 @ read CBAR > +#endif > + add r3, r2, #GIC_DIST_OFFSET @ GIC dist i/f offset > + mvn r1, #0 @ all bits to 1 > + str r1, [r3, #GICD_IGROUPRn] @ allow private interrupts > + > + mrc p15, 0, r0, c0, c0, 0 @ read MIDR > + bfc r0, #20, #4 @ mask out variant > + bfc r0, #0, #4 @ and revision > + > + movw r1, #lo(MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK) in the git repo branch you pointed me to in the cover e-mail, this refers to MIDR_CORTEX_A6_R0P0 ? Forgot to push the last revision? > + movt r1, #hi(MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK) > + cmp r0, r1 @ check for Cortex-A7 > + > + orr r1, #(MIDR_CORTEX_A15_R0P0 & 0xf0) > + cmpne r0, r1 @ check for Cortex-A15 > + > + movne r1, #GIC_CPU_OFFSET_A9 @ GIC CPU offset for A9 > + moveq r1, #GIC_CPU_OFFSET_A15 @ GIC CPU offset for A15/A7 > + add r3, r2, r1 @ r3 = GIC CPU i/f addr > + > + mov r1, #1 @ set GICC_CTLR[enable] > + str r1, [r3, #GICC_CTLR] @ and clear all other bits > + mov r1, #0xff > + str r1, [r3, #GICC_PMR] @ set priority mask register > + > + movw r1, #0x3fff > + movt r1, #0x0006 > + mcr p15, 0, r1, c1, c1, 2 @ NSACR = all copros to non-sec > + > + adr r1, _secure_vectors > + mcr p15, 0, r1, c12, c0, 1 @ set MVBAR to secure vectors > + > + mrc p15, 0, ip, c12, c0, 0 @ save secure copy of VBAR > + > + isb > + smc #0 @ call into MONITOR mode > + > + mcr p15, 0, ip, c12, c0, 0 @ write non-secure copy of VBAR > + > + mov r1, #1 > + str r1, [r3, #GICC_CTLR] @ enable non-secure CPU i/f > + add r2, r2, #GIC_DIST_OFFSET > + str r1, [r2] @ allow private interrupts For those who don't remember which register is at offset zero, Could you do: str r1, [r2, #GICD_CTLR] > + > + bx lr > diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h > index 20caa7c..989bb72 100644 > --- a/arch/arm/include/asm/armv7.h > +++ b/arch/arm/include/asm/armv7.h > @@ -34,6 +34,17 @@ > #define MIDR_CORTEX_A15_R0P0 0x410FC0F0 > #define MIDR_CORTEX_A15_R2P2 0x412FC0F2 > > +/* Cortex-A7 revisions */ > +#define MIDR_CORTEX_A7_R0P0 0x410FC070 > + > +#define MIDR_PRIMARY_PART_MASK 0xFF0FFFF0 > + > +/* ID_PFR1 feature fields */ > +#define CPUID_ARM_VIRT_SHIFT 12 > +#define CPUID_ARM_VIRT_MASK (0xF << CPUID_ARM_VIRT_SHIFT) > +#define CPUID_ARM_TIMER_SHIFT 16 > +#define CPUID_ARM_TIMER_MASK (0xF << CPUID_ARM_TIMER_SHIFT) > + > /* CCSIDR */ > #define CCSIDR_LINE_SIZE_OFFSET 0 > #define CCSIDR_LINE_SIZE_MASK 0x7 > @@ -76,6 +87,11 @@ void v7_outer_cache_inval_all(void); > void v7_outer_cache_flush_range(u32 start, u32 end); > void v7_outer_cache_inval_range(u32 start, u32 end); > > +#ifdef CONFIG_ARMV7_VIRT > +/* defined in cpu/armv7/nonsec_virt.S */ > +void _nonsec_init(void); > +#endif /* CONFIG_ARMV7_VIRT */ > + > #endif /* ! __ASSEMBLY__ */ > > #endif > diff --git a/arch/arm/include/asm/gic.h b/arch/arm/include/asm/gic.h > new file mode 100644 > index 0000000..c2b1e28 > --- /dev/null > +++ b/arch/arm/include/asm/gic.h > @@ -0,0 +1,17 @@ > +#ifndef __GIC_V2_H__ > +#define __GIC_V2_H__ > + > +/* register offsets for the ARM generic interrupt controller (GIC) */ > + > +#define GIC_DIST_OFFSET 0x1000 > +#define GICD_CTLR 0x0000 > +#define GICD_TYPER 0x0004 > +#define GICD_IGROUPRn 0x0080 > +#define GICD_SGIR 0x0F00 > + > +#define GIC_CPU_OFFSET_A9 0x0100 > +#define GIC_CPU_OFFSET_A15 0x2000 > +#define GICC_CTLR 0x0000 > +#define GICC_PMR 0x0004 > + > +#endif > -- Looks great otherwise. I cannot build this with the Ubuntu Raring arm-linux-gnueabihf- cross compiler without adding ".arch_extension sec" into this file. Perhaps you need to play with a few compilers to be sure this builds properly. You may also want to look in arch/arm/kvm/Makefile in the kernel to see how we use the as-instr to make sure the proper directives are used in the source file or option given to the assembler. You should be able to easily port the as-instr into U-boot if needed (TI does this in their U-boot for for example). -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
