[U-Boot] [PATCH v3 0/7] ARMv7: Add HYP mode switching support

[U-Boot] [PATCH v3 0/7] ARMv7: Add HYP mode switching support

[Andre Przywara]

(for GIT URL and Changelog see below)

ARM CPUs with the virtualization extension have a new mode called
HYP mode, which allows hypervisors to safely control and monitor
guests. The current hypervisor implementations (KVM and Xen)
require the kernel to be entered in that HYP mode.

This patch series introduces a configuration variable
CONFIG_ARMV7_VIRT which enables code to switch all cores into HYP
mode. This is done automatically during execution of the bootm
command.

The process of switching into HYP mode requires the CPU to be in
secure state initially when entering u-boot, it will then setup some
register and switch to non-secure state. This requires the GIC to be
programmed properly first. Explanations about the details are in the
commit messages of the respective patches.

The patches are structured like this:
1/7: prepare header file
2/7: add monitor handler (assembly)
3/7: add per CPU non-secure switch routine (assembly)
4/7: add system wide non-secure setup and link to bootm command (C)
5/7: add SMP functionality
6/7: add HYP mode switching
7/7: enable code on Versatile Express TC2

Since up to patch 5/7 this code works on non-virtualization capable
CPUs also and there has been a request, there is now a second
configuration variable CONFIG_ARMV7_NONSEC, which omits the final
HYP mode switch and just goes into non-secure SVC state.
You can specify either (or none) of them, the code cares about
the dependency.

The code aims to be as generic as possible, though currently it has
only been tested on the Versatile Express TC-2 board. The last patch
thus enables the feature for that board.

For convenience there is a GIT tree which you can pull these patches
from ("hypmode_v3" branch):
git://git.linaro.org/people/aprzywara/u-boot.git

Changes RFC..v1
* not a dedicated command anymore, code run by bootm & friends
* protecting code with #ifdefs to avoid unnecessary inclusion and
  accidental crashing (when accessing restricted registers)
* moving prototypes to header file to meet checkpatch recommendation
* adding comment as proposed by Christoffer

Changes v1..v2
mostly style and code layout changes 
* restructure assembly code to live in a new file and not start.S
* split smp, nonsec_init and hyp_init to be separate functions
* used named constants from common header files
* split C function to be more readable
* extend comments to be more precise and elaborate
* add provision to override GIC base address (needed for Arndale?)
* add configuration variable to enable VExpress specific SMP code
* use writel/readl for MMIO GIC accesses
* remove superfluous isb instructions
* more minor fixes

Changes v2..v3
* fix clobbering of GICC address actually spoiling the stack
* do CNTFRQ setup in assembly per core (and not only once per SoC)
* moving the new code files into arch/arm/cpu/armv7
* add config variable for doing non-secure switch only
* use actual HYP and secure instructions mnemonics instead of
  the encoded byte sequence. This requires more recent compilers.
* make the identification of the CPU core more robust and saner
* use enum for error codes and rename them
* lots of smaller layout and style fixes


Please review and comment!
I am pretty sure I missed some of the comments from the earlier
series, so if you find something that was mentioned before, feel
free to repeat it.

Contributions and comments to support other boards are welcome.

Andre Przywara (7):
  ARM: prepare armv7.h to be included from assembly source
  ARM: add secure monitor handler to switch to non-secure state
  ARM: add assembly routine to switch to non-secure state
  ARM: switch to non-secure state during bootm execution
  ARM: add SMP support for non-secure switch
  ARM: extend non-secure switch to also go into HYP mode
  ARM: VExpress: enable ARMv7 virt support for VExpress A15

 arch/arm/cpu/armv7/Makefile         |   5 +
 arch/arm/cpu/armv7/nonsec_virt.S    | 199 ++++++++++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/virt-v7.c        | 165 ++++++++++++++++++++++++++++++
 arch/arm/include/asm/armv7.h        |  40 +++++++-
 arch/arm/include/asm/gic.h          |  19 ++++
 arch/arm/lib/bootm.c                |  41 ++++++++
 include/configs/vexpress_ca15_tc2.h |   5 +-
 7 files changed, 472 insertions(+), 2 deletions(-)

-- 
1.7.12.1

[U-Boot] [PATCH v3 1/7] ARM: prepare armv7.h to be included from assembly source

[Andre Przywara]

armv7.h contains some useful constants, but also C prototypes.
To include it also in assembly files, protect the non-assembly
part appropriately.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 arch/arm/include/asm/armv7.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index a73630b..20caa7c 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -23,7 +23,6 @@
  */
 #ifndef ARMV7_H
 #define ARMV7_H
-#include <linux/types.h>
 
 /* Cortex-A9 revisions */
 #define MIDR_CORTEX_A9_R0P1	0x410FC091
@@ -57,6 +56,9 @@
 #define ARMV7_CLIDR_CTYPE_INSTRUCTION_DATA	3
 #define ARMV7_CLIDR_CTYPE_UNIFIED		4
 
+#ifndef __ASSEMBLY__
+#include <linux/types.h>
+
 /*
  * CP15 Barrier instructions
  * Please note that we have separate barrier instructions in ARMv7
@@ -74,4 +76,6 @@ 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);
 
+#endif /* ! __ASSEMBLY__ */
+
 #endif
-- 
1.7.12.1

[U-Boot] [PATCH v3 2/7] ARM: add secure monitor handler to switch to non-secure state

[Andre Przywara]

A prerequisite for using virtualization is to be in HYP mode, which
requires the CPU to be in non-secure state first.
Add new file in arch/arm/cpu/armv7 to hold a monitor handler routine
which switches the CPU to non-secure state by setting the NS and
associated bits.
According to the ARM architecture reference manual this should not be
done in SVC mode, so we have to setup a SMC handler for this.
We create a new vector table to avoid interference with other boards.
The MVBAR register will be programmed later just before the smc call.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 arch/arm/cpu/armv7/Makefile      |  4 +++
 arch/arm/cpu/armv7/nonsec_virt.S | 54 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/nonsec_virt.S

diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index 7a8c2d0..5d75077 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -36,6 +36,10 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
 SOBJS	+= lowlevel_init.o
 endif
 
+ifneq ($(CONFIG_ARMV7_NONSEC),)
+SOBJS   += nonsec_virt.o
+endif
+
 SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
 START	:= $(addprefix $(obj),$(START))
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
new file mode 100644
index 0000000..68a6b38
--- /dev/null
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -0,0 +1,54 @@
+/*
+ * code for switching cores into non-secure state
+ *
+ * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <config.h>
+
+/* the vector table for secure state */
+_monitor_vectors:
+	.word 0	/* reset */
+	.word 0 /* undef */
+	adr pc, _secure_monitor
+	.word 0
+	.word 0
+	.word 0
+	.word 0
+	.word 0
+	.word 0	/* pad */
+
+/*
+ * software interrupt aka. secure monitor handler
+ * This is executed on a "smc" instruction, we use a "smc #0" to switch
+ * to non-secure state.
+ * We use only r0 and r1 here, due to constraints in the caller.
+ */
+	.align	5
+_secure_monitor:
+	mrc	p15, 0, r1, c1, c1, 0		@ read SCR
+	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
+	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
+
+	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
+
+	movs	pc, lr				@ return to non-secure SVC
+
-- 
1.7.12.1

[U-Boot] [PATCH v3 3/7] ARM: add assembly routine to switch to non-secure state

[Andre Przywara]

While actually switching to non-secure state is one thing, another
part of this process is to make sure that we still 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.

Now that we know the GIC address, we:
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)

Also we allow access to all coprocessor interfaces from non-secure
state by writing the appropriate bits in the NSACR register.

The generic timer base frequency register is only accessible from
secure state, so we have to program it now. Actually this should be
done from primary firmware before, but some boards seems to omit
this, so if needed we do this here with a board specific value.
The Versatile Express board does not need this, so we remove the
frequency from the configuration file here.

After having switched to non-secure state, we also enable the
non-secure GIC CPU interface, since this register is banked.

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 mess with the compiler.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 arch/arm/cpu/armv7/nonsec_virt.S    | 85 +++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/armv7.h        | 18 ++++++++
 arch/arm/include/asm/gic.h          | 17 ++++++++
 include/configs/vexpress_ca15_tc2.h |  2 -
 4 files changed, 120 insertions(+), 2 deletions(-)
 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 68a6b38..e9ee831 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -23,6 +23,11 @@
  */
 
 #include <config.h>
+#include <linux/linkage.h>
+#include <asm/gic.h>
+#include <asm/armv7.h>
+
+.arch_extension sec
 
 /* the vector table for secure state */
 _monitor_vectors:
@@ -52,3 +57,83 @@ _secure_monitor:
 
 	movs	pc, lr				@ return to non-secure SVC
 
+/*
+ * 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 secondary cores and directly by the BSP.
+ * Do not assume that the stack is available and only use registers
+ * r0-r3 and r12.
+ *
+ * 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.
+ */
+ENTRY(_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
+	ldr	r1, =MIDR_PRIMARY_PART_MASK
+	and	r0, r0, r1			@ mask out variant and revision
+
+	ldr	r1, =MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK
+	cmp	r0, r1				@ check for Cortex-A7
+
+	ldr	r1, =MIDR_CORTEX_A15_R0P0 & MIDR_PRIMARY_PART_MASK
+	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
+
+/* The CNTFRQ register of the generic timer needs to be
+ * programmed in secure state. Some primary bootloaders / firmware
+ * omit this, so if the frequency is provided in the configuration,
+ * we do this here instead.
+ * But first check if we have the generic timer.
+ */
+#ifdef CONFIG_SYS_CLK_FREQ
+	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
+	and	r0, r0, #CPUID_ARM_GENTIMER_MASK	@ mask arch timer bits
+	cmp	r0, #(1 << CPUID_ARM_GENTIMER_SHIFT)
+	ldreq	r1, =CONFIG_SYS_CLK_FREQ
+	mcreq	p15, 0, r1, c14, c0, 0		@ write CNTFRQ
+#endif
+
+	adr	r1, _monitor_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, #GICD_CTLR]		@ allow private interrupts
+
+	mov	r0, r3				@ return GICC address
+
+	bx	lr
+ENDPROC(_nonsec_init)
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index 20caa7c..ab9fa58 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -34,6 +34,19 @@
 #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_SEC_SHIFT		4
+#define CPUID_ARM_SEC_MASK		(0xF << CPUID_ARM_SEC_SHIFT)
+#define CPUID_ARM_VIRT_SHIFT		12
+#define CPUID_ARM_VIRT_MASK		(0xF << CPUID_ARM_VIRT_SHIFT)
+#define CPUID_ARM_GENTIMER_SHIFT	16
+#define CPUID_ARM_GENTIMER_MASK		(0xF << CPUID_ARM_GENTIMER_SHIFT)
+
 /* CCSIDR */
 #define CCSIDR_LINE_SIZE_OFFSET		0
 #define CCSIDR_LINE_SIZE_MASK		0x7
@@ -76,6 +89,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_NONSEC
+/* defined in assembly file */
+unsigned int _nonsec_init(void);
+#endif /* CONFIG_ARMV7_NONSEC */
+
 #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
diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
index 9e230ad..4f425ac 100644
--- a/include/configs/vexpress_ca15_tc2.h
+++ b/include/configs/vexpress_ca15_tc2.h
@@ -31,6 +31,4 @@
 #include "vexpress_common.h"
 #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
 
-#define CONFIG_SYS_CLK_FREQ 24000000
-
 #endif
-- 
1.7.12.1

[U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution

[Andre Przywara]

To actually trigger the non-secure switch we just implemented, call
the switching routine from within the bootm command implementation.
This way we automatically enable this feature without further user
intervention.

The core specific part of the work is done in the assembly routine
in nonsec_virt.S, introduced with the previous patch, but for the full
glory we need to setup the GIC distributor interface once for the
whole system, which is done in C here.
The routine is placed in arch/arm/cpu/armv7 to allow easy access from
other ARMv7 boards.

We check the availability of the security extensions first.

Since we need a safe way to access the GIC, we use the PERIPHBASE
registers on Cortex-A15 and A7 CPUs and do some sanity checks.
Board not implementing the CBAR can override this value via a
configuration file variable.

Then we actually do the GIC enablement:
a) enable the GIC distributor, both for non-secure and secure state
   (GICD_CTLR[1:0] = 11b)
b) allow all interrupts to be handled from non-secure state
   (GICD_IGROUPRn = 0xFFFFFFFF)

The core specific GIC setup is then done in the assembly routine.

The actual bootm trigger is pretty small: calling the routine and
doing some error reporting.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 arch/arm/cpu/armv7/Makefile  |   1 +
 arch/arm/cpu/armv7/virt-v7.c | 117 +++++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/armv7.h |  10 ++++
 arch/arm/lib/bootm.c         |  28 +++++++++++
 4 files changed, 156 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/virt-v7.c

diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index 5d75077..b59f59e 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -38,6 +38,7 @@ endif
 
 ifneq ($(CONFIG_ARMV7_NONSEC),)
 SOBJS   += nonsec_virt.o
+COBJS	+= virt-v7.o
 endif
 
 SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
new file mode 100644
index 0000000..54f9746
--- /dev/null
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -0,0 +1,117 @@
+/*
+ * (C) Copyright 2013
+ * Andre Przywara, Linaro
+ *
+ * Routines to transition ARMv7 processors from secure into non-secure state
+ * needed to enable ARMv7 virtualization for current hypervisors
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/armv7.h>
+#include <asm/gic.h>
+#include <asm/io.h>
+
+static unsigned int read_id_pfr1(void)
+{
+	unsigned int reg;
+
+	asm("mrc p15, 0, %0, c0, c1, 1\n" : "=r"(reg));
+	return reg;
+}
+
+static int get_gicd_base_address(unsigned int *gicdaddr)
+{
+#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
+	*gicdaddr = CONFIG_ARM_GIC_BASE_ADDRESS + GIC_DIST_OFFSET;
+	return 0;
+#else
+	unsigned midr;
+	unsigned periphbase;
+
+	/* check whether we are an Cortex-A15 or A7.
+	 * The actual HYP switch should work with all CPUs supporting
+	 * the virtualization extension, but we need the GIC address,
+	 * which we know only for sure for those two CPUs.
+	 */
+	asm("mrc p15, 0, %0, c0, c0, 0\n" : "=r"(midr));
+	switch (midr & MIDR_PRIMARY_PART_MASK) {
+	case MIDR_CORTEX_A9_R0P1:
+	case MIDR_CORTEX_A15_R0P0:
+	case MIDR_CORTEX_A7_R0P0:
+		break;
+	default:
+		return NONSEC_ERR_NO_GIC_ADDRESS;
+	}
+
+	/* get the GIC base address from the CBAR register */
+	asm("mrc p15, 4, %0, c15, c0, 0\n" : "=r" (periphbase));
+
+	/* the PERIPHBASE can be mapped above 4 GB (lower 8 bits used to
+	 * encode this). Bail out here since we cannot access this without
+	 * enabling paging.
+	 */
+	if ((periphbase & 0xff) != 0)
+		return NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB;
+
+	*gicdaddr = periphbase + GIC_DIST_OFFSET;
+
+	return 0;
+#endif
+}
+
+enum nonsec_virt_errors armv7_switch_nonsec(void)
+{
+	unsigned int reg, ret;
+	unsigned int gicdaddr = 0;
+	unsigned itlinesnr, i;
+
+	/* check whether the CPU supports the security extensions */
+	reg = read_id_pfr1();
+	if ((reg & 0xF0) == 0)
+		return NONSEC_ERR_NO_SEC_EXT;
+
+	/* the SCR register will be set directly in the monitor mode handler,
+	 * according to the spec one should not tinker with it in secure state
+	 * in SVC mode. Do not try to read it once in non-secure state,
+	 * any access to it will trap.
+	 */
+
+	ret = get_gicd_base_address(&gicdaddr);
+	if (ret != 0)
+		return ret;
+
+	/* enable the GIC distributor */
+	writel(readl(gicdaddr + GICD_CTLR) | 0x03, gicdaddr + GICD_CTLR);
+
+	/* TYPER[4:0] contains an encoded number of available interrupts */
+	itlinesnr = readl(gicdaddr + GICD_TYPER) & 0x1f;
+
+	/* set all bits in the GIC group registers to one to allow access
+	 * from non-secure state
+	 */
+	for (i = 0; i <= itlinesnr; i++)
+		writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
+
+	/* call the non-sec switching code on this CPU */
+	_nonsec_init();
+
+	return NONSEC_VIRT_SUCCESS;
+}
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index ab9fa58..e5c0279 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -90,6 +90,16 @@ void v7_outer_cache_flush_range(u32 start, u32 end);
 void v7_outer_cache_inval_range(u32 start, u32 end);
 
 #ifdef CONFIG_ARMV7_NONSEC
+
+enum nonsec_virt_errors {
+	NONSEC_VIRT_SUCCESS,
+	NONSEC_ERR_NO_SEC_EXT,
+	NONSEC_ERR_NO_GIC_ADDRESS,
+	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
+};
+
+enum nonsec_virt_errors armv7_switch_nonsec(void);
+
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
 #endif /* CONFIG_ARMV7_NONSEC */
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1b6e0ac..7b0619e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -34,6 +34,10 @@
 #include <asm/bootm.h>
 #include <linux/compiler.h>
 
+#ifdef CONFIG_ARMV7_NONSEC
+#include <asm/armv7.h>
+#endif
+
 DECLARE_GLOBAL_DATA_PTR;
 
 static struct tag *params;
@@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
 
 __weak void setup_board_tags(struct tag **in_params) {}
 
+static void do_nonsec_virt_switch(void)
+{
+#ifdef CONFIG_ARMV7_NONSEC
+	int ret;
+
+	ret = armv7_switch_nonsec();
+	switch (ret) {
+	case NONSEC_VIRT_SUCCESS:
+		debug("entered non-secure state\n");
+		break;
+	case NONSEC_ERR_NO_SEC_EXT:
+		printf("nonsec: Security extensions not implemented.\n");
+		break;
+	case NONSEC_ERR_NO_GIC_ADDRESS:
+		printf("nonsec: could not determine GIC address.\n");
+		break;
+	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
+		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
+		break;
+	}
+#endif
+}
+
 /* Subcommand: PREP */
 static void boot_prep_linux(bootm_headers_t *images)
 {
@@ -222,6 +249,7 @@ static void boot_prep_linux(bootm_headers_t *images)
 		printf("FDT and ATAGS support not compiled in - hanging\n");
 		hang();
 	}
+	do_nonsec_virt_switch();
 }
 
 /* Subcommand: GO */
-- 
1.7.12.1

[U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch

[Andre Przywara]

Currently the non-secure switch is only done for the boot processor.
To enable 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/nonsec_virt.S    | 27 +++++++++++++++++++++++++++
 arch/arm/cpu/armv7/virt-v7.c        | 19 ++++++++++++++++++-
 arch/arm/include/asm/armv7.h        |  1 +
 arch/arm/include/asm/gic.h          |  2 ++
 include/configs/vexpress_ca15_tc2.h |  3 +++
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
index e9ee831..f9b6b39 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -58,6 +58,33 @@ _secure_monitor:
 	movs	pc, lr				@ return to non-secure SVC
 
 /*
+ * Secondary CPUs start here and call the code for the core specific parts
+ * of the non-secure and HYP mode transition. The GIC distributor specific
+ * code has already been executed by a C function before.
+ * Then they go back to wfi and wait to be woken up by the kernel again.
+ */
+ENTRY(_smp_pen)
+	mrs	r0, cpsr
+	orr	r0, r0, #0xc0
+	msr	cpsr, r0			@ disable interrupts
+	ldr	r1, =_start
+	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
+
+	bl	_nonsec_init
+
+	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
+	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
+	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
+ENDPROC(_smp_pen)
+
+/*
  * Switch a core to non-secure state.
  *
  *  1. initialize the GIC per-core interface
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index 54f9746..a0d0b34 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
 #endif
 }
 
+static void kick_secondary_cpus(unsigned int gicdaddr)
+{
+	unsigned int *secondary_boot_addr;
+
+	secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
+#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
+	secondary_boot_addr[1] = (unsigned)-1;
+#endif
+	*secondary_boot_addr = (uintptr_t)_smp_pen;
+	dmb();
+
+	/* now kick all CPUs (except this one) by writing to GICD_SGIR */
+	writel(1U << 24, gicdaddr + GICD_SGIR);
+}
+
 enum nonsec_virt_errors armv7_switch_nonsec(void)
 {
 	unsigned int reg, ret;
@@ -110,7 +125,9 @@ enum nonsec_virt_errors armv7_switch_nonsec(void)
 	for (i = 0; i <= itlinesnr; i++)
 		writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
 
-	/* call the non-sec switching code on this CPU */
+	kick_secondary_cpus(gicdaddr);
+
+	/* call the non-sec switching code on this CPU also */
 	_nonsec_init();
 
 	return NONSEC_VIRT_SUCCESS;
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index e5c0279..f6582a1 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -102,6 +102,7 @@ enum nonsec_virt_errors armv7_switch_nonsec(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
+void _smp_pen(void);
 #endif /* CONFIG_ARMV7_NONSEC */
 
 #endif /* ! __ASSEMBLY__ */
diff --git a/arch/arm/include/asm/gic.h b/arch/arm/include/asm/gic.h
index c2b1e28..a0891cc 100644
--- a/arch/arm/include/asm/gic.h
+++ b/arch/arm/include/asm/gic.h
@@ -13,5 +13,7 @@
 #define GIC_CPU_OFFSET_A15	0x2000
 #define GICC_CTLR		0x0000
 #define GICC_PMR		0x0004
+#define GICC_IAR		0x000C
+#define GICC_EOIR		0x0010
 
 #endif
diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
index 4f425ac..ade9e5b 100644
--- a/include/configs/vexpress_ca15_tc2.h
+++ b/include/configs/vexpress_ca15_tc2.h
@@ -31,4 +31,7 @@
 #include "vexpress_common.h"
 #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
 
+#define CONFIG_SYSFLAGS_ADDR 0x1c010030
+#define CONFIG_SYSFLAGS_NEED_CLEAR_BITS
+
 #endif
-- 
1.7.12.1

[U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode

[Andre Przywara]

For the KVM and XEN hypervisors to be usable, we need to enter the
kernel in HYP mode. Now that we already are in non-secure state,
HYP mode switching is within short reach.

While doing the non-secure switch, we have to enable the HVC
instruction and setup the HYP mode HVBAR (while still secure).

The actual switch is done by dropping back from a HYP mode handler
without actually leaving HYP mode, so we introduce a new handler
routine in our new secure exception vector table.

In the assembly switching routine we save and restore the banked LR
and SP registers around the hypercall to do the actual HYP mode
switch.

The C routine first checks whether we are in HYP mode already and
also whether the virtualization extensions are available. It also
checks whether the HYP mode switch was finally successful.
The bootm command part only adds and adjusts some error reporting.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 arch/arm/cpu/armv7/Makefile      |  2 +-
 arch/arm/cpu/armv7/nonsec_virt.S | 43 +++++++++++++++++++++++++++++++++++-----
 arch/arm/cpu/armv7/virt-v7.c     | 31 +++++++++++++++++++++++++++++
 arch/arm/include/asm/armv7.h     |  9 +++++++--
 arch/arm/lib/bootm.c             | 19 +++++++++++++++---
 5 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index b59f59e..e5eaa56 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -36,7 +36,7 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
 SOBJS	+= lowlevel_init.o
 endif
 
-ifneq ($(CONFIG_ARMV7_NONSEC),)
+ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),)
 SOBJS   += nonsec_virt.o
 COBJS	+= virt-v7.o
 endif
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
index f9b6b39..895c3b0 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -1,5 +1,5 @@
 /*
- * code for switching cores into non-secure state
+ * code for switching cores into non-secure state and into HYP mode
  *
  * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
  *
@@ -28,15 +28,16 @@
 #include <asm/armv7.h>
 
 .arch_extension sec
+.arch_extension virt
 
-/* the vector table for secure state */
+/* the vector table for secure state and HYP mode */
 _monitor_vectors:
 	.word 0	/* reset */
 	.word 0 /* undef */
 	adr pc, _secure_monitor
 	.word 0
 	.word 0
-	.word 0
+	adr pc, _hyp_trap
 	.word 0
 	.word 0
 	.word 0	/* pad */
@@ -53,10 +54,27 @@ _secure_monitor:
 	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
 	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
 
+#ifdef CONFIG_ARMV7_VIRT
+	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
+	and	r0, r0, #CPUID_ARM_VIRT_MASK	@ mask virtualization bits
+	cmp	r0, #(1 << CPUID_ARM_VIRT_SHIFT)
+	orreq	r1, r1, #0x100			@ allow HVC instruction
+#endif
+
 	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
 
+#ifdef CONFIG_ARMV7_VIRT
+	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
+	mcreq	p15, 4, r0, c12, c0, 0		@ write HVBAR
+#endif
+
 	movs	pc, lr				@ return to non-secure SVC
 
+_hyp_trap:
+	mrs	lr, elr_hyp	@ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1
+	mov pc, lr				@ do no switch modes, but
+						@ return to caller
+
 /*
  * Secondary CPUs start here and call the code for the core specific parts
  * of the non-secure and HYP mode transition. The GIC distributor specific
@@ -71,9 +89,13 @@ ENTRY(_smp_pen)
 	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
 
 	bl	_nonsec_init
+	mov	r12, r0				@ save GICC address
+#ifdef CONFIG_ARMV7_VIRT
+	bl	_switch_to_hyp
+#endif
 
-	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
-	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
+	ldr	r1, [r12, #GICC_IAR]		@ acknowledge IPI
+	str	r1, [r12, #GICC_EOIR]		@ signal end of interrupt
 	adr	r1, _smp_pen
 waitloop:
 	wfi
@@ -164,3 +186,14 @@ ENTRY(_nonsec_init)
 
 	bx	lr
 ENDPROC(_nonsec_init)
+
+ENTRY(_switch_to_hyp)
+	mov	r0, lr
+	mov	r1, sp				@ save SVC copy of LR and SP
+	isb
+	hvc #0			 @ for older asm: .byte 0x70, 0x00, 0x40, 0xe1
+	mov	sp, r1
+	mov	lr, r0				@ restore SVC copy of LR and SP
+
+	bx	lr
+ENDPROC(_switch_to_hyp)
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index a0d0b34..3645572 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -3,6 +3,7 @@
  * Andre Przywara, Linaro
  *
  * Routines to transition ARMv7 processors from secure into non-secure state
+ * and from non-secure SVC into HYP mode
  * needed to enable ARMv7 virtualization for current hypervisors
  *
  * See file CREDITS for list of people who contributed to this
@@ -29,6 +30,14 @@
 #include <asm/gic.h>
 #include <asm/io.h>
 
+static unsigned int read_cpsr(void)
+{
+	unsigned int reg;
+
+	asm volatile ("mrs %0, cpsr\n" : "=r" (reg));
+	return reg;
+}
+
 static unsigned int read_id_pfr1(void)
 {
 	unsigned int reg;
@@ -92,6 +101,28 @@ static void kick_secondary_cpus(unsigned int gicdaddr)
 	writel(1U << 24, gicdaddr + GICD_SGIR);
 }
 
+enum nonsec_virt_errors armv7_switch_hyp(void)
+{
+	unsigned int reg;
+
+	/* check whether we are in HYP mode already */
+	if ((read_cpsr() & 0x1f) == 0x1a)
+		return VIRT_ALREADY_HYP_MODE;
+
+	/* check whether the CPU supports the virtualization extensions */
+	reg = read_id_pfr1();
+	if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT)
+		return VIRT_ERR_NO_VIRT_EXT;
+
+	/* call the HYP switching code on this CPU also */
+	_switch_to_hyp();
+
+	if ((read_cpsr() & 0x1F) != 0x1a)
+		return VIRT_ERR_NOT_HYP_MODE;
+
+	return NONSEC_VIRT_SUCCESS;
+}
+
 enum nonsec_virt_errors armv7_switch_nonsec(void)
 {
 	unsigned int reg, ret;
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index f6582a1..baa22fe 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -89,21 +89,26 @@ 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_NONSEC
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 
 enum nonsec_virt_errors {
 	NONSEC_VIRT_SUCCESS,
 	NONSEC_ERR_NO_SEC_EXT,
 	NONSEC_ERR_NO_GIC_ADDRESS,
 	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
+	VIRT_ALREADY_HYP_MODE,
+	VIRT_ERR_NO_VIRT_EXT,
+	VIRT_ERR_NOT_HYP_MODE
 };
 
 enum nonsec_virt_errors armv7_switch_nonsec(void);
+enum nonsec_virt_errors armv7_switch_hyp(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
 void _smp_pen(void);
-#endif /* CONFIG_ARMV7_NONSEC */
+void _switch_to_hyp(void);
+#endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
 
 #endif /* ! __ASSEMBLY__ */
 
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 7b0619e..90875b3 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -34,7 +34,7 @@
 #include <asm/bootm.h>
 #include <linux/compiler.h>
 
-#ifdef CONFIG_ARMV7_NONSEC
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 #include <asm/armv7.h>
 #endif
 
@@ -192,13 +192,17 @@ __weak void setup_board_tags(struct tag **in_params) {}
 
 static void do_nonsec_virt_switch(void)
 {
-#ifdef CONFIG_ARMV7_NONSEC
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 	int ret;
 
 	ret = armv7_switch_nonsec();
+#ifdef CONFIG_ARMV7_VIRT
+	if (ret == NONSEC_VIRT_SUCCESS)
+		ret = armv7_switch_hyp();
+#endif
 	switch (ret) {
 	case NONSEC_VIRT_SUCCESS:
-		debug("entered non-secure state\n");
+		debug("entered non-secure state or HYP mode\n");
 		break;
 	case NONSEC_ERR_NO_SEC_EXT:
 		printf("nonsec: Security extensions not implemented.\n");
@@ -209,6 +213,15 @@ static void do_nonsec_virt_switch(void)
 	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
 		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
 		break;
+	case VIRT_ERR_NO_VIRT_EXT:
+		printf("HYP mode: Virtualization extensions not implemented.\n");
+		break;
+	case VIRT_ALREADY_HYP_MODE:
+		debug("CPU already in HYP mode\n");
+		break;
+	case VIRT_ERR_NOT_HYP_MODE:
+		printf("HYP mode: switch not successful.\n");
+		break;
 	}
 #endif
 }
-- 
1.7.12.1

[U-Boot] [PATCH v3 7/7] ARM: VExpress: enable ARMv7 virt support for VExpress A15

[Andre Przywara]

To enable hypervisors utilizing the ARMv7 virtualization extension
on the Versatile Express board with the A15 core tile, we add the
respective configuration variable.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 include/configs/vexpress_ca15_tc2.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
index ade9e5b..5d6928f 100644
--- a/include/configs/vexpress_ca15_tc2.h
+++ b/include/configs/vexpress_ca15_tc2.h
@@ -34,4 +34,6 @@
 #define CONFIG_SYSFLAGS_ADDR 0x1c010030
 #define CONFIG_SYSFLAGS_NEED_CLEAR_BITS
 
+#define CONFIG_ARMV7_VIRT
+
 #endif
-- 
1.7.12.1

[U-Boot] [PATCH v3 3/7] ARM: add assembly routine to switch to non-secure state

[Nikolay Nikolaev]

Hello Andre,


On Wed, Jul 10, 2013 at 2:54 AM, Andre Przywara
<andre.przywara@linaro.org>wrote:

> While actually switching to non-secure state is one thing, another
> part of this process is to make sure that we still 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.
>
> Now that we know the GIC address, we:
> 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)
>
> Also we allow access to all coprocessor interfaces from non-secure
> state by writing the appropriate bits in the NSACR register.
>
> The generic timer base frequency register is only accessible from
> secure state, so we have to program it now. Actually this should be
> done from primary firmware before, but some boards seems to omit
> this, so if needed we do this here with a board specific value.
> The Versatile Express board does not need this, so we remove the
> frequency from the configuration file here.
>
> After having switched to non-secure state, we also enable the
> non-secure GIC CPU interface, since this register is banked.
>
> 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 mess with the compiler.
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  arch/arm/cpu/armv7/nonsec_virt.S    | 85
> +++++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/armv7.h        | 18 ++++++++
>  arch/arm/include/asm/gic.h          | 17 ++++++++
>  include/configs/vexpress_ca15_tc2.h |  2 -
>  4 files changed, 120 insertions(+), 2 deletions(-)
>  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 68a6b38..e9ee831 100644
> --- a/arch/arm/cpu/armv7/nonsec_virt.S
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -23,6 +23,11 @@
>   */
>
>  #include <config.h>
> +#include <linux/linkage.h>
> +#include <asm/gic.h>
> +#include <asm/armv7.h>
> +
> +.arch_extension sec
>
>  /* the vector table for secure state */
>  _monitor_vectors:
> @@ -52,3 +57,83 @@ _secure_monitor:
>
>         movs    pc, lr                          @ return to non-secure SVC
>
> +/*
> + * 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 secondary cores and directly by the BSP.
> + * Do not assume that the stack is available and only use registers
> + * r0-r3 and r12.
> + *
> + * 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.
> + */
> +ENTRY(_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
>
PERIPHBASE[39:32] is CBAR[7:0] which we ignore here - OK.
PERIPHBASE[31:15] is CBAR[31:15] - OK
The rest of the bits is UNK/SBZP - we should not rely they are 0s.

I'd suggest to mask r2 with 0xFFFF8000 here (figure out some name like
PERIPHBASE_MASK)


> +#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
> +       ldr     r1, =MIDR_PRIMARY_PART_MASK
> +       and     r0, r0, r1                      @ mask out variant and
> revision
> +
> +       ldr     r1, =MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK
> +       cmp     r0, r1                          @ check for Cortex-A7
> +
> +       ldr     r1, =MIDR_CORTEX_A15_R0P0 & MIDR_PRIMARY_PART_MASK
> +       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
> +
> +/* The CNTFRQ register of the generic timer needs to be
> + * programmed in secure state. Some primary bootloaders / firmware
> + * omit this, so if the frequency is provided in the configuration,
> + * we do this here instead.
> + * But first check if we have the generic timer.
> + */
> +#ifdef CONFIG_SYS_CLK_FREQ
> +       mrc     p15, 0, r0, c0, c1, 1           @ read ID_PFR1
> +       and     r0, r0, #CPUID_ARM_GENTIMER_MASK        @ mask arch timer
> bits
> +       cmp     r0, #(1 << CPUID_ARM_GENTIMER_SHIFT)
> +       ldreq   r1, =CONFIG_SYS_CLK_FREQ
> +       mcreq   p15, 0, r1, c14, c0, 0          @ write CNTFRQ
> +#endif
> +
> +       adr     r1, _monitor_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, #GICD_CTLR]            @ allow private interrupts
> +
> +       mov     r0, r3                          @ return GICC address
> +
> +       bx      lr
> +ENDPROC(_nonsec_init)
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index 20caa7c..ab9fa58 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -34,6 +34,19 @@
>  #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_SEC_SHIFT            4
> +#define CPUID_ARM_SEC_MASK             (0xF << CPUID_ARM_SEC_SHIFT)
> +#define CPUID_ARM_VIRT_SHIFT           12
> +#define CPUID_ARM_VIRT_MASK            (0xF << CPUID_ARM_VIRT_SHIFT)
> +#define CPUID_ARM_GENTIMER_SHIFT       16
> +#define CPUID_ARM_GENTIMER_MASK                (0xF <<
> CPUID_ARM_GENTIMER_SHIFT)
> +
>  /* CCSIDR */
>  #define CCSIDR_LINE_SIZE_OFFSET                0
>  #define CCSIDR_LINE_SIZE_MASK          0x7
> @@ -76,6 +89,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_NONSEC
> +/* defined in assembly file */
> +unsigned int _nonsec_init(void);
> +#endif /* CONFIG_ARMV7_NONSEC */
> +
>  #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
> diff --git a/include/configs/vexpress_ca15_tc2.h
> b/include/configs/vexpress_ca15_tc2.h
> index 9e230ad..4f425ac 100644
> --- a/include/configs/vexpress_ca15_tc2.h
> +++ b/include/configs/vexpress_ca15_tc2.h
> @@ -31,6 +31,4 @@
>  #include "vexpress_common.h"
>  #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
>
> -#define CONFIG_SYS_CLK_FREQ 24000000
> -
>  #endif
> --
> 1.7.12.1
>
>
regards,
Nikolay Nikolaev

[U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution

[Nikolay Nikolaev]

Hello Andre,


On Wed, Jul 10, 2013 at 2:54 AM, Andre Przywara
<andre.przywara@linaro.org>wrote:

> To actually trigger the non-secure switch we just implemented, call
> the switching routine from within the bootm command implementation.
> This way we automatically enable this feature without further user
> intervention.
>
> The core specific part of the work is done in the assembly routine
> in nonsec_virt.S, introduced with the previous patch, but for the full
> glory we need to setup the GIC distributor interface once for the
> whole system, which is done in C here.
> The routine is placed in arch/arm/cpu/armv7 to allow easy access from
> other ARMv7 boards.
>
> We check the availability of the security extensions first.
>
> Since we need a safe way to access the GIC, we use the PERIPHBASE
> registers on Cortex-A15 and A7 CPUs and do some sanity checks.
> Board not implementing the CBAR can override this value via a
> configuration file variable.
>
> Then we actually do the GIC enablement:
> a) enable the GIC distributor, both for non-secure and secure state
>    (GICD_CTLR[1:0] = 11b)
> b) allow all interrupts to be handled from non-secure state
>    (GICD_IGROUPRn = 0xFFFFFFFF)
>
> The core specific GIC setup is then done in the assembly routine.
>
> The actual bootm trigger is pretty small: calling the routine and
> doing some error reporting.
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  arch/arm/cpu/armv7/Makefile  |   1 +
>  arch/arm/cpu/armv7/virt-v7.c | 117
> +++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/armv7.h |  10 ++++
>  arch/arm/lib/bootm.c         |  28 +++++++++++
>  4 files changed, 156 insertions(+)
>  create mode 100644 arch/arm/cpu/armv7/virt-v7.c
>
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index 5d75077..b59f59e 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -38,6 +38,7 @@ endif
>
>  ifneq ($(CONFIG_ARMV7_NONSEC),)
>  SOBJS   += nonsec_virt.o
> +COBJS  += virt-v7.o
>  endif
>
>  SRCS   := $(START:.o=.S) $(COBJS:.o=.c)
> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
> new file mode 100644
> index 0000000..54f9746
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/virt-v7.c
> @@ -0,0 +1,117 @@
> +/*
> + * (C) Copyright 2013
> + * Andre Przywara, Linaro
> + *
> + * Routines to transition ARMv7 processors from secure into non-secure
> state
> + * needed to enable ARMv7 virtualization for current hypervisors
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <asm/armv7.h>
> +#include <asm/gic.h>
> +#include <asm/io.h>
> +
> +static unsigned int read_id_pfr1(void)
> +{
> +       unsigned int reg;
> +
> +       asm("mrc p15, 0, %0, c0, c1, 1\n" : "=r"(reg));
> +       return reg;
> +}
> +
> +static int get_gicd_base_address(unsigned int *gicdaddr)
> +{
> +#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
> +       *gicdaddr = CONFIG_ARM_GIC_BASE_ADDRESS + GIC_DIST_OFFSET;
> +       return 0;
> +#else
> +       unsigned midr;
> +       unsigned periphbase;
> +
> +       /* check whether we are an Cortex-A15 or A7.
> +        * The actual HYP switch should work with all CPUs supporting
> +        * the virtualization extension, but we need the GIC address,
> +        * which we know only for sure for those two CPUs.
> +        */
> +       asm("mrc p15, 0, %0, c0, c0, 0\n" : "=r"(midr));
> +       switch (midr & MIDR_PRIMARY_PART_MASK) {
> +       case MIDR_CORTEX_A9_R0P1:
> +       case MIDR_CORTEX_A15_R0P0:
> +       case MIDR_CORTEX_A7_R0P0:
> +               break;
> +       default:
> +               return NONSEC_ERR_NO_GIC_ADDRESS;
> +       }
> +
> +       /* get the GIC base address from the CBAR register */
> +       asm("mrc p15, 4, %0, c15, c0, 0\n" : "=r" (periphbase));
> +
> +       /* the PERIPHBASE can be mapped above 4 GB (lower 8 bits used to
> +        * encode this). Bail out here since we cannot access this without
> +        * enabling paging.
> +        */
> +       if ((periphbase & 0xff) != 0)
> +               return NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB;
> +
> +       *gicdaddr = periphbase + GIC_DIST_OFFSET;
>

The same as in _nonsec_init

periphbase &= PERIPHBASE_MASK; // 0xffff8000


> +
> +       return 0;
> +#endif
> +}
> +
> +enum nonsec_virt_errors armv7_switch_nonsec(void)
> +{
> +       unsigned int reg, ret;
> +       unsigned int gicdaddr = 0;
> +       unsigned itlinesnr, i;
> +
> +       /* check whether the CPU supports the security extensions */
> +       reg = read_id_pfr1();
> +       if ((reg & 0xF0) == 0)
> +               return NONSEC_ERR_NO_SEC_EXT;
> +
> +       /* the SCR register will be set directly in the monitor mode
> handler,
> +        * according to the spec one should not tinker with it in secure
> state
> +        * in SVC mode. Do not try to read it once in non-secure state,
> +        * any access to it will trap.
> +        */
> +
> +       ret = get_gicd_base_address(&gicdaddr);
> +       if (ret != 0)
> +               return ret;
> +
> +       /* enable the GIC distributor */
> +       writel(readl(gicdaddr + GICD_CTLR) | 0x03, gicdaddr + GICD_CTLR);
> +
> +       /* TYPER[4:0] contains an encoded number of available interrupts */
> +       itlinesnr = readl(gicdaddr + GICD_TYPER) & 0x1f;
> +
> +       /* set all bits in the GIC group registers to one to allow access
> +        * from non-secure state
> +        */
> +       for (i = 0; i <= itlinesnr; i++)
>

you can start from i = 1 here, since in _nonsec_init you already have:

       mvn     r1, #0                          @ all bits to 1
       str     r1, [r3, #GICD_IGROUPRn]        @ allow private interrupts

+               writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
> +
> +       /* call the non-sec switching code on this CPU */
> +       _nonsec_init();
> +
> +       return NONSEC_VIRT_SUCCESS;
> +}
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index ab9fa58..e5c0279 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -90,6 +90,16 @@ void v7_outer_cache_flush_range(u32 start, u32 end);
>  void v7_outer_cache_inval_range(u32 start, u32 end);
>
>  #ifdef CONFIG_ARMV7_NONSEC
> +
> +enum nonsec_virt_errors {
> +       NONSEC_VIRT_SUCCESS,
> +       NONSEC_ERR_NO_SEC_EXT,
> +       NONSEC_ERR_NO_GIC_ADDRESS,
> +       NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
> +};
> +
> +enum nonsec_virt_errors armv7_switch_nonsec(void);
> +
>  /* defined in assembly file */
>  unsigned int _nonsec_init(void);
>  #endif /* CONFIG_ARMV7_NONSEC */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 1b6e0ac..7b0619e 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -34,6 +34,10 @@
>  #include <asm/bootm.h>
>  #include <linux/compiler.h>
>
> +#ifdef CONFIG_ARMV7_NONSEC
> +#include <asm/armv7.h>
> +#endif
> +
>  DECLARE_GLOBAL_DATA_PTR;
>
>  static struct tag *params;
> @@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
>
>  __weak void setup_board_tags(struct tag **in_params) {}
>
> +static void do_nonsec_virt_switch(void)
> +{
> +#ifdef CONFIG_ARMV7_NONSEC
> +       int ret;
> +
> +       ret = armv7_switch_nonsec();
> +       switch (ret) {
> +       case NONSEC_VIRT_SUCCESS:
> +               debug("entered non-secure state\n");
> +               break;
> +       case NONSEC_ERR_NO_SEC_EXT:
> +               printf("nonsec: Security extensions not implemented.\n");
> +               break;
> +       case NONSEC_ERR_NO_GIC_ADDRESS:
> +               printf("nonsec: could not determine GIC address.\n");
> +               break;
> +       case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
> +               printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
> +               break;
> +       }
> +#endif
> +}
> +
>  /* Subcommand: PREP */
>  static void boot_prep_linux(bootm_headers_t *images)
>  {
> @@ -222,6 +249,7 @@ static void boot_prep_linux(bootm_headers_t *images)
>                 printf("FDT and ATAGS support not compiled in -
> hanging\n");
>                 hang();
>         }
> +       do_nonsec_virt_switch();
>  }
>
>  /* Subcommand: GO */
> --
> 1.7.12.1
>
>
regards,
Nikolay Nikolaev

[U-Boot] [PATCH v3 2/7] ARM: add secure monitor handler to switch to non-secure state

[Christoffer Dall]

n Wed, Jul 10, 2013 at 01:54:14AM +0200, Andre Przywara wrote:
> A prerequisite for using virtualization is to be in HYP mode, which
> requires the CPU to be in non-secure state first.
> Add new file in arch/arm/cpu/armv7 to hold a monitor handler routine
> which switches the CPU to non-secure state by setting the NS and
> associated bits.
> According to the ARM architecture reference manual this should not be
> done in SVC mode, so we have to setup a SMC handler for this.
> We create a new vector table to avoid interference with other boards.
> The MVBAR register will be programmed later just before the smc call.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  arch/arm/cpu/armv7/Makefile      |  4 +++
>  arch/arm/cpu/armv7/nonsec_virt.S | 54 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 arch/arm/cpu/armv7/nonsec_virt.S
> 
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index 7a8c2d0..5d75077 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -36,6 +36,10 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
>  SOBJS	+= lowlevel_init.o
>  endif
>  
> +ifneq ($(CONFIG_ARMV7_NONSEC),)
> +SOBJS   += nonsec_virt.o
> +endif
> +
>  SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
>  START	:= $(addprefix $(obj),$(START))
> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> new file mode 100644
> index 0000000..68a6b38
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -0,0 +1,54 @@
> +/*
> + * code for switching cores into non-secure state
> + *
> + * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <config.h>
> +
> +/* the vector table for secure state */
> +_monitor_vectors:
> +	.word 0	/* reset */
> +	.word 0 /* undef */
> +	adr pc, _secure_monitor
> +	.word 0
> +	.word 0
> +	.word 0
> +	.word 0
> +	.word 0
> +	.word 0	/* pad */
> +
> +/*
> + * software interrupt aka. secure monitor handler

a software interrupt is not aka. a secure monitor handler, this is
misleading, it's just the smc handler.

> + * This is executed on a "smc" instruction, we use a "smc #0" to switch
> + * to non-secure state.
> + * We use only r0 and r1 here, due to constraints in the caller.
> + */
> +	.align	5
> +_secure_monitor:
> +	mrc	p15, 0, r1, c1, c1, 0		@ read SCR
> +	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
> +	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
> +
> +	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
> +
> +	movs	pc, lr				@ return to non-secure SVC
> +
> -- 
> 1.7.12.1
> 

[U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution

[Christoffer Dall]

On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:

[...]

> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 1b6e0ac..7b0619e 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -34,6 +34,10 @@
>  #include <asm/bootm.h>
>  #include <linux/compiler.h>
>  
> +#ifdef CONFIG_ARMV7_NONSEC
> +#include <asm/armv7.h>
> +#endif
> +
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  static struct tag *params;
> @@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
>  
>  __weak void setup_board_tags(struct tag **in_params) {}
>  
> +static void do_nonsec_virt_switch(void)
> +{
> +#ifdef CONFIG_ARMV7_NONSEC
> +	int ret;
> +
> +	ret = armv7_switch_nonsec();
> +	switch (ret) {
> +	case NONSEC_VIRT_SUCCESS:
> +		debug("entered non-secure state\n");
> +		break;
> +	case NONSEC_ERR_NO_SEC_EXT:
> +		printf("nonsec: Security extensions not implemented.\n");
> +		break;
> +	case NONSEC_ERR_NO_GIC_ADDRESS:
> +		printf("nonsec: could not determine GIC address.\n");
> +		break;
> +	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
> +		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
> +		break;
> +	}
> +#endif
> +}

I still don't get why you just don't make armv7_switch_nonsec a void and
print the error when they occur... ???

-Christoffer

[U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch

[Christoffer Dall]

On Wed, Jul 10, 2013 at 01:54:17AM +0200, Andre Przywara wrote:
> Currently the non-secure switch is only done for the boot processor.
> To enable 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/nonsec_virt.S    | 27 +++++++++++++++++++++++++++
>  arch/arm/cpu/armv7/virt-v7.c        | 19 ++++++++++++++++++-
>  arch/arm/include/asm/armv7.h        |  1 +
>  arch/arm/include/asm/gic.h          |  2 ++
>  include/configs/vexpress_ca15_tc2.h |  3 +++
>  5 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> index e9ee831..f9b6b39 100644
> --- a/arch/arm/cpu/armv7/nonsec_virt.S
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -58,6 +58,33 @@ _secure_monitor:
>  	movs	pc, lr				@ return to non-secure SVC
>  
>  /*
> + * Secondary CPUs start here and call the code for the core specific parts
> + * of the non-secure and HYP mode transition. The GIC distributor specific
> + * code has already been executed by a C function before.
> + * Then they go back to wfi and wait to be woken up by the kernel again.
> + */
> +ENTRY(_smp_pen)
> +	mrs	r0, cpsr
> +	orr	r0, r0, #0xc0
> +	msr	cpsr, r0			@ disable interrupts
> +	ldr	r1, =_start
> +	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
> +
> +	bl	_nonsec_init
> +
> +	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
> +	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
> +	adr	r1, _smp_pen
> +waitloop:
> +	wfi
> +	ldr	r0, =CONFIG_SYSFLAGS_ADDR	@ load start address

You seem to have ignored my comment about using the sysflags name?

As I understand, the sysflags name is a versatile express specific
register name that just happens to be used for the SMP boot address as
well...

Therefore, this should really be CONFIG_SMP_BOOT_ADDR or something like
that, at the very least.

> +	ldr	r0, [r0]
> +	cmp	r0, r1			@ make sure we dont execute this code
> +	beq	waitloop		@ again (due to a spurious wakeup)
> +	mov	pc, r0
> +ENDPROC(_smp_pen)
> +
> +/*
>   * Switch a core to non-secure state.
>   *
>   *  1. initialize the GIC per-core interface
> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
> index 54f9746..a0d0b34 100644
> --- a/arch/arm/cpu/armv7/virt-v7.c
> +++ b/arch/arm/cpu/armv7/virt-v7.c
> @@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
>  #endif
>  }
>  
> +static void kick_secondary_cpus(unsigned int gicdaddr)
> +{
> +	unsigned int *secondary_boot_addr;
> +
> +	secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
> +#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
> +	secondary_boot_addr[1] = (unsigned)-1;
> +#endif

again, if you disagreed with my previous comment, please comment on it
and rationalize your choice.

I still feel you're wrapping board specific logic into generic code, and
that you should call out to a more generic function.  Imagine an SOC
that uses an implementation defined control register for this instead of
a memory address...

perhaps what you need is:

void set_board_smp_boot_addr(unsigned long addr);
unsigned long get_board_smp_boot_addr(void);

and call these instead of your direct use of sysflags addr here...?

> +	*secondary_boot_addr = (uintptr_t)_smp_pen;
> +	dmb();
> +
> +	/* now kick all CPUs (except this one) by writing to GICD_SGIR */
> +	writel(1U << 24, gicdaddr + GICD_SGIR);
> +}
> +
>  enum nonsec_virt_errors armv7_switch_nonsec(void)
>  {
>  	unsigned int reg, ret;
> @@ -110,7 +125,9 @@ enum nonsec_virt_errors armv7_switch_nonsec(void)
>  	for (i = 0; i <= itlinesnr; i++)
>  		writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
>  
> -	/* call the non-sec switching code on this CPU */
> +	kick_secondary_cpus(gicdaddr);
> +
> +	/* call the non-sec switching code on this CPU also */
>  	_nonsec_init();
>  
>  	return NONSEC_VIRT_SUCCESS;
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index e5c0279..f6582a1 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -102,6 +102,7 @@ enum nonsec_virt_errors armv7_switch_nonsec(void);
>  
>  /* defined in assembly file */
>  unsigned int _nonsec_init(void);
> +void _smp_pen(void);
>  #endif /* CONFIG_ARMV7_NONSEC */
>  
>  #endif /* ! __ASSEMBLY__ */
> diff --git a/arch/arm/include/asm/gic.h b/arch/arm/include/asm/gic.h
> index c2b1e28..a0891cc 100644
> --- a/arch/arm/include/asm/gic.h
> +++ b/arch/arm/include/asm/gic.h
> @@ -13,5 +13,7 @@
>  #define GIC_CPU_OFFSET_A15	0x2000
>  #define GICC_CTLR		0x0000
>  #define GICC_PMR		0x0004
> +#define GICC_IAR		0x000C
> +#define GICC_EOIR		0x0010
>  
>  #endif
> diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
> index 4f425ac..ade9e5b 100644
> --- a/include/configs/vexpress_ca15_tc2.h
> +++ b/include/configs/vexpress_ca15_tc2.h
> @@ -31,4 +31,7 @@
>  #include "vexpress_common.h"
>  #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
>  
> +#define CONFIG_SYSFLAGS_ADDR 0x1c010030
> +#define CONFIG_SYSFLAGS_NEED_CLEAR_BITS
> +
>  #endif
> -- 
> 1.7.12.1
> 

[U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode

[Christoffer Dall]

On Wed, Jul 10, 2013 at 01:54:18AM +0200, Andre Przywara wrote:
> For the KVM and XEN hypervisors to be usable, we need to enter the
> kernel in HYP mode. Now that we already are in non-secure state,
> HYP mode switching is within short reach.
> 
> While doing the non-secure switch, we have to enable the HVC
> instruction and setup the HYP mode HVBAR (while still secure).
> 
> The actual switch is done by dropping back from a HYP mode handler
> without actually leaving HYP mode, so we introduce a new handler
> routine in our new secure exception vector table.
> 
> In the assembly switching routine we save and restore the banked LR
> and SP registers around the hypercall to do the actual HYP mode
> switch.
> 
> The C routine first checks whether we are in HYP mode already and
> also whether the virtualization extensions are available. It also
> checks whether the HYP mode switch was finally successful.
> The bootm command part only adds and adjusts some error reporting.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  arch/arm/cpu/armv7/Makefile      |  2 +-
>  arch/arm/cpu/armv7/nonsec_virt.S | 43 +++++++++++++++++++++++++++++++++++-----
>  arch/arm/cpu/armv7/virt-v7.c     | 31 +++++++++++++++++++++++++++++
>  arch/arm/include/asm/armv7.h     |  9 +++++++--
>  arch/arm/lib/bootm.c             | 19 +++++++++++++++---
>  5 files changed, 93 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index b59f59e..e5eaa56 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -36,7 +36,7 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
>  SOBJS	+= lowlevel_init.o
>  endif
>  
> -ifneq ($(CONFIG_ARMV7_NONSEC),)
> +ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),)
>  SOBJS   += nonsec_virt.o
>  COBJS	+= virt-v7.o
>  endif
> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> index f9b6b39..895c3b0 100644
> --- a/arch/arm/cpu/armv7/nonsec_virt.S
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -1,5 +1,5 @@
>  /*
> - * code for switching cores into non-secure state
> + * code for switching cores into non-secure state and into HYP mode
>   *
>   * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
>   *
> @@ -28,15 +28,16 @@
>  #include <asm/armv7.h>
>  
>  .arch_extension sec
> +.arch_extension virt
>  
> -/* the vector table for secure state */
> +/* the vector table for secure state and HYP mode */
>  _monitor_vectors:
>  	.word 0	/* reset */
>  	.word 0 /* undef */
>  	adr pc, _secure_monitor
>  	.word 0
>  	.word 0
> -	.word 0
> +	adr pc, _hyp_trap
>  	.word 0
>  	.word 0
>  	.word 0	/* pad */
> @@ -53,10 +54,27 @@ _secure_monitor:
>  	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
>  	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
>  
> +#ifdef CONFIG_ARMV7_VIRT
> +	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
> +	and	r0, r0, #CPUID_ARM_VIRT_MASK	@ mask virtualization bits
> +	cmp	r0, #(1 << CPUID_ARM_VIRT_SHIFT)
> +	orreq	r1, r1, #0x100			@ allow HVC instruction
> +#endif
> +
>  	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
>  
> +#ifdef CONFIG_ARMV7_VIRT
> +	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
> +	mcreq	p15, 4, r0, c12, c0, 0		@ write HVBAR
> +#endif
> +
>  	movs	pc, lr				@ return to non-secure SVC
>  
> +_hyp_trap:
> +	mrs	lr, elr_hyp	@ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1

this comment just confuses: either make it intelligent to support an
older compiler or just get rid of these byte encodings.  You can always
disassemble the file and lookup the byte code with a modern compiler to
get back to the byte encoding.

> +	mov pc, lr				@ do no switch modes, but
> +						@ return to caller
> +
>  /*
>   * Secondary CPUs start here and call the code for the core specific parts
>   * of the non-secure and HYP mode transition. The GIC distributor specific
> @@ -71,9 +89,13 @@ ENTRY(_smp_pen)
>  	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
>  
>  	bl	_nonsec_init
> +	mov	r12, r0				@ save GICC address
> +#ifdef CONFIG_ARMV7_VIRT
> +	bl	_switch_to_hyp
> +#endif
>  
> -	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
> -	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
> +	ldr	r1, [r12, #GICC_IAR]		@ acknowledge IPI
> +	str	r1, [r12, #GICC_EOIR]		@ signal end of interrupt
>  	adr	r1, _smp_pen
>  waitloop:
>  	wfi
> @@ -164,3 +186,14 @@ ENTRY(_nonsec_init)
>  
>  	bx	lr
>  ENDPROC(_nonsec_init)
> +
> +ENTRY(_switch_to_hyp)
> +	mov	r0, lr
> +	mov	r1, sp				@ save SVC copy of LR and SP
> +	isb

did you find out that this isb is indeed needed? if so, why?

> +	hvc #0			 @ for older asm: .byte 0x70, 0x00, 0x40, 0xe1

same comment as above?

> +	mov	sp, r1
> +	mov	lr, r0				@ restore SVC copy of LR and SP
> +
> +	bx	lr
> +ENDPROC(_switch_to_hyp)
> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
> index a0d0b34..3645572 100644
> --- a/arch/arm/cpu/armv7/virt-v7.c
> +++ b/arch/arm/cpu/armv7/virt-v7.c
> @@ -3,6 +3,7 @@
>   * Andre Przywara, Linaro
>   *
>   * Routines to transition ARMv7 processors from secure into non-secure state
> + * and from non-secure SVC into HYP mode
>   * needed to enable ARMv7 virtualization for current hypervisors
>   *
>   * See file CREDITS for list of people who contributed to this
> @@ -29,6 +30,14 @@
>  #include <asm/gic.h>
>  #include <asm/io.h>
>  
> +static unsigned int read_cpsr(void)
> +{
> +	unsigned int reg;
> +
> +	asm volatile ("mrs %0, cpsr\n" : "=r" (reg));
> +	return reg;
> +}
> +
>  static unsigned int read_id_pfr1(void)
>  {
>  	unsigned int reg;
> @@ -92,6 +101,28 @@ static void kick_secondary_cpus(unsigned int gicdaddr)
>  	writel(1U << 24, gicdaddr + GICD_SGIR);
>  }
>  
> +enum nonsec_virt_errors armv7_switch_hyp(void)
> +{
> +	unsigned int reg;
> +
> +	/* check whether we are in HYP mode already */
> +	if ((read_cpsr() & 0x1f) == 0x1a)
> +		return VIRT_ALREADY_HYP_MODE;
> +
> +	/* check whether the CPU supports the virtualization extensions */
> +	reg = read_id_pfr1();
> +	if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT)
> +		return VIRT_ERR_NO_VIRT_EXT;
> +
> +	/* call the HYP switching code on this CPU also */
> +	_switch_to_hyp();
> +
> +	if ((read_cpsr() & 0x1F) != 0x1a)
> +		return VIRT_ERR_NOT_HYP_MODE;
> +
> +	return NONSEC_VIRT_SUCCESS;
> +}
> +
>  enum nonsec_virt_errors armv7_switch_nonsec(void)
>  {
>  	unsigned int reg, ret;
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index f6582a1..baa22fe 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -89,21 +89,26 @@ 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_NONSEC
> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>  
>  enum nonsec_virt_errors {
>  	NONSEC_VIRT_SUCCESS,
>  	NONSEC_ERR_NO_SEC_EXT,
>  	NONSEC_ERR_NO_GIC_ADDRESS,
>  	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
> +	VIRT_ALREADY_HYP_MODE,
> +	VIRT_ERR_NO_VIRT_EXT,
> +	VIRT_ERR_NOT_HYP_MODE
>  };
>  
>  enum nonsec_virt_errors armv7_switch_nonsec(void);
> +enum nonsec_virt_errors armv7_switch_hyp(void);
>  
>  /* defined in assembly file */
>  unsigned int _nonsec_init(void);
>  void _smp_pen(void);
> -#endif /* CONFIG_ARMV7_NONSEC */
> +void _switch_to_hyp(void);
> +#endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
>  
>  #endif /* ! __ASSEMBLY__ */
>  
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 7b0619e..90875b3 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -34,7 +34,7 @@
>  #include <asm/bootm.h>
>  #include <linux/compiler.h>
>  
> -#ifdef CONFIG_ARMV7_NONSEC
> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>  #include <asm/armv7.h>
>  #endif
>  
> @@ -192,13 +192,17 @@ __weak void setup_board_tags(struct tag **in_params) {}
>  
>  static void do_nonsec_virt_switch(void)
>  {
> -#ifdef CONFIG_ARMV7_NONSEC
> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>  	int ret;
>  
>  	ret = armv7_switch_nonsec();
> +#ifdef CONFIG_ARMV7_VIRT
> +	if (ret == NONSEC_VIRT_SUCCESS)
> +		ret = armv7_switch_hyp();
> +#endif
>  	switch (ret) {
>  	case NONSEC_VIRT_SUCCESS:
> -		debug("entered non-secure state\n");
> +		debug("entered non-secure state or HYP mode\n");
>  		break;
>  	case NONSEC_ERR_NO_SEC_EXT:
>  		printf("nonsec: Security extensions not implemented.\n");
> @@ -209,6 +213,15 @@ static void do_nonsec_virt_switch(void)
>  	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
>  		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
>  		break;
> +	case VIRT_ERR_NO_VIRT_EXT:
> +		printf("HYP mode: Virtualization extensions not implemented.\n");
> +		break;
> +	case VIRT_ALREADY_HYP_MODE:
> +		debug("CPU already in HYP mode\n");
> +		break;
> +	case VIRT_ERR_NOT_HYP_MODE:
> +		printf("HYP mode: switch not successful.\n");
> +		break;
>  	}
>  #endif
>  }
> -- 
> 1.7.12.1
> 

[U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution

[Andre Przywara]

On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:
>
> [...]
>
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 1b6e0ac..7b0619e 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -34,6 +34,10 @@
>>   #include <asm/bootm.h>
>>   #include <linux/compiler.h>
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +#include <asm/armv7.h>
>> +#endif
>> +
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>>   static struct tag *params;
>> @@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
>>
>>   __weak void setup_board_tags(struct tag **in_params) {}
>>
>> +static void do_nonsec_virt_switch(void)
>> +{
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +	int ret;
>> +
>> +	ret = armv7_switch_nonsec();
>> +	switch (ret) {
>> +	case NONSEC_VIRT_SUCCESS:
>> +		debug("entered non-secure state\n");
>> +		break;
>> +	case NONSEC_ERR_NO_SEC_EXT:
>> +		printf("nonsec: Security extensions not implemented.\n");
>> +		break;
>> +	case NONSEC_ERR_NO_GIC_ADDRESS:
>> +		printf("nonsec: could not determine GIC address.\n");
>> +		break;
>> +	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
>> +		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
>> +		break;
>> +	}
>> +#endif
>> +}
>
> I still don't get why you just don't make armv7_switch_nonsec a void and
> print the error when they occur... ???

My apologies for not elaborating on these comments I didn't incorporate:

So, I don't like the idea of marrying a low-level routine with high 
level output. I don't want to constraint the usage of the routine by 
requiring an output channel. Also some parts may not be fatal for all 
users - someone could just try to switch and then behave differently if 
that failed - without bothering the user.
May seem a bit over-engineered, but I like it better this way ;-)

If that is a show-stopper for you, I can change it, of course.

Regards,
Andre.

[U-Boot] [PATCH v3 2/7] ARM: add secure monitor handler to switch to non-secure state

[Andre Przywara]

On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> n Wed, Jul 10, 2013 at 01:54:14AM +0200, Andre Przywara wrote:
>> A prerequisite for using virtualization is to be in HYP mode, which
>> requires the CPU to be in non-secure state first.
>> Add new file in arch/arm/cpu/armv7 to hold a monitor handler routine
>> which switches the CPU to non-secure state by setting the NS and
>> associated bits.
>> According to the ARM architecture reference manual this should not be
>> done in SVC mode, so we have to setup a SMC handler for this.
>> We create a new vector table to avoid interference with other boards.
>> The MVBAR register will be programmed later just before the smc call.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   arch/arm/cpu/armv7/Makefile      |  4 +++
>>   arch/arm/cpu/armv7/nonsec_virt.S | 54 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+)
>>   create mode 100644 arch/arm/cpu/armv7/nonsec_virt.S
>>
>> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
>> index 7a8c2d0..5d75077 100644
>> --- a/arch/arm/cpu/armv7/Makefile
>> +++ b/arch/arm/cpu/armv7/Makefile
>> @@ -36,6 +36,10 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
>>   SOBJS	+= lowlevel_init.o
>>   endif
>>
>> +ifneq ($(CONFIG_ARMV7_NONSEC),)
>> +SOBJS   += nonsec_virt.o
>> +endif
>> +
>>   SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
>>   OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
>>   START	:= $(addprefix $(obj),$(START))
>> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
>> new file mode 100644
>> index 0000000..68a6b38
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
>> @@ -0,0 +1,54 @@
>> +/*
>> + * code for switching cores into non-secure state
>> + *
>> + * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#include <config.h>
>> +
>> +/* the vector table for secure state */
>> +_monitor_vectors:
>> +	.word 0	/* reset */
>> +	.word 0 /* undef */
>> +	adr pc, _secure_monitor
>> +	.word 0
>> +	.word 0
>> +	.word 0
>> +	.word 0
>> +	.word 0
>> +	.word 0	/* pad */
>> +
>> +/*
>> + * software interrupt aka. secure monitor handler
>
> a software interrupt is not aka. a secure monitor handler, this is
> misleading, it's just the smc handler.

I agree, but I wanted to stick to the u-boot nomenclature which uses 
"software interrupt" for that exception in arch/arm/cpu/armv7/start.S.
So I used both names to make it more clear to the u-boot reader.
If I make a newer version, I will fix it in there.

Regards,
Andre.

>
>> + * This is executed on a "smc" instruction, we use a "smc #0" to switch
>> + * to non-secure state.
>> + * We use only r0 and r1 here, due to constraints in the caller.
>> + */
>> +	.align	5
>> +_secure_monitor:
>> +	mrc	p15, 0, r1, c1, c1, 0		@ read SCR
>> +	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
>> +	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
>> +
>> +	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
>> +
>> +	movs	pc, lr				@ return to non-secure SVC
>> +
>> --
>> 1.7.12.1
>>

[U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch

[Andre Przywara]

On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> On Wed, Jul 10, 2013 at 01:54:17AM +0200, Andre Przywara wrote:
>> Currently the non-secure switch is only done for the boot processor.
>> To enable 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/nonsec_virt.S    | 27 +++++++++++++++++++++++++++
>>   arch/arm/cpu/armv7/virt-v7.c        | 19 ++++++++++++++++++-
>>   arch/arm/include/asm/armv7.h        |  1 +
>>   arch/arm/include/asm/gic.h          |  2 ++
>>   include/configs/vexpress_ca15_tc2.h |  3 +++
>>   5 files changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
>> index e9ee831..f9b6b39 100644
>> --- a/arch/arm/cpu/armv7/nonsec_virt.S
>> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
>> @@ -58,6 +58,33 @@ _secure_monitor:
>>   	movs	pc, lr				@ return to non-secure SVC
>>
>>   /*
>> + * Secondary CPUs start here and call the code for the core specific parts
>> + * of the non-secure and HYP mode transition. The GIC distributor specific
>> + * code has already been executed by a C function before.
>> + * Then they go back to wfi and wait to be woken up by the kernel again.
>> + */
>> +ENTRY(_smp_pen)
>> +	mrs	r0, cpsr
>> +	orr	r0, r0, #0xc0
>> +	msr	cpsr, r0			@ disable interrupts
>> +	ldr	r1, =_start
>> +	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
>> +
>> +	bl	_nonsec_init
>> +
>> +	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
>> +	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
>> +	adr	r1, _smp_pen
>> +waitloop:
>> +	wfi
>> +	ldr	r0, =CONFIG_SYSFLAGS_ADDR	@ load start address
>
> You seem to have ignored my comment about using the sysflags name?
>
> As I understand, the sysflags name is a versatile express specific
> register name that just happens to be used for the SMP boot address as
> well...
>
> Therefore, this should really be CONFIG_SMP_BOOT_ADDR or something like
> that, at the very least.

>
>> +	ldr	r0, [r0]
>> +	cmp	r0, r1			@ make sure we dont execute this code
>> +	beq	waitloop		@ again (due to a spurious wakeup)
>> +	mov	pc, r0
>> +ENDPROC(_smp_pen)
>> +
>> +/*
>>    * Switch a core to non-secure state.
>>    *
>>    *  1. initialize the GIC per-core interface
>> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
>> index 54f9746..a0d0b34 100644
>> --- a/arch/arm/cpu/armv7/virt-v7.c
>> +++ b/arch/arm/cpu/armv7/virt-v7.c
>> @@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
>>   #endif
>>   }
>>
>> +static void kick_secondary_cpus(unsigned int gicdaddr)
>> +{
>> +	unsigned int *secondary_boot_addr;
>> +
>> +	secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
>> +#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
>> +	secondary_boot_addr[1] = (unsigned)-1;
>> +#endif
>
> again, if you disagreed with my previous comment, please comment on it
> and rationalize your choice.

I am sorry, I thought I reasoned that already in an earlier mail:
I don't want to introduce abstraction prematurely, so I wanted to 
refactor the code as soon as a second board gets supported. Which will 
probably be the Arndale, and also probably done by me ...

> I still feel you're wrapping board specific logic into generic code, and
> that you should call out to a more generic function.  Imagine an SOC
> that uses an implementation defined control register for this instead of
> a memory address...

Well, I can imagine quite some ways to do this, but in fact I'd like to 
focus on things that really exist ;-)
My fear is that any abstraction I introduce now will not suffice for a 
future board and needs to be refactored then anyway.

Regards,
Andre.

>
> perhaps what you need is:
>
> void set_board_smp_boot_addr(unsigned long addr);
> unsigned long get_board_smp_boot_addr(void);
>
> and call these instead of your direct use of sysflags addr here...?
>
>> +	*secondary_boot_addr = (uintptr_t)_smp_pen;
>> +	dmb();
>> +
>> +	/* now kick all CPUs (except this one) by writing to GICD_SGIR */
>> +	writel(1U << 24, gicdaddr + GICD_SGIR);
>> +}
>> +
>>   enum nonsec_virt_errors armv7_switch_nonsec(void)
>>   {
>>   	unsigned int reg, ret;
>> @@ -110,7 +125,9 @@ enum nonsec_virt_errors armv7_switch_nonsec(void)
>>   	for (i = 0; i <= itlinesnr; i++)
>>   		writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
>>
>> -	/* call the non-sec switching code on this CPU */
>> +	kick_secondary_cpus(gicdaddr);
>> +
>> +	/* call the non-sec switching code on this CPU also */
>>   	_nonsec_init();
>>
>>   	return NONSEC_VIRT_SUCCESS;
>> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
>> index e5c0279..f6582a1 100644
>> --- a/arch/arm/include/asm/armv7.h
>> +++ b/arch/arm/include/asm/armv7.h
>> @@ -102,6 +102,7 @@ enum nonsec_virt_errors armv7_switch_nonsec(void);
>>
>>   /* defined in assembly file */
>>   unsigned int _nonsec_init(void);
>> +void _smp_pen(void);
>>   #endif /* CONFIG_ARMV7_NONSEC */
>>
>>   #endif /* ! __ASSEMBLY__ */
>> diff --git a/arch/arm/include/asm/gic.h b/arch/arm/include/asm/gic.h
>> index c2b1e28..a0891cc 100644
>> --- a/arch/arm/include/asm/gic.h
>> +++ b/arch/arm/include/asm/gic.h
>> @@ -13,5 +13,7 @@
>>   #define GIC_CPU_OFFSET_A15	0x2000
>>   #define GICC_CTLR		0x0000
>>   #define GICC_PMR		0x0004
>> +#define GICC_IAR		0x000C
>> +#define GICC_EOIR		0x0010
>>
>>   #endif
>> diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h
>> index 4f425ac..ade9e5b 100644
>> --- a/include/configs/vexpress_ca15_tc2.h
>> +++ b/include/configs/vexpress_ca15_tc2.h
>> @@ -31,4 +31,7 @@
>>   #include "vexpress_common.h"
>>   #define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca15x2_tc2"
>>
>> +#define CONFIG_SYSFLAGS_ADDR 0x1c010030
>> +#define CONFIG_SYSFLAGS_NEED_CLEAR_BITS
>> +
>>   #endif
>> --
>> 1.7.12.1
>>

[U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode

[Andre Przywara]

On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> On Wed, Jul 10, 2013 at 01:54:18AM +0200, Andre Przywara wrote:
>> For the KVM and XEN hypervisors to be usable, we need to enter the
>> kernel in HYP mode. Now that we already are in non-secure state,
>> HYP mode switching is within short reach.
>>
>> While doing the non-secure switch, we have to enable the HVC
>> instruction and setup the HYP mode HVBAR (while still secure).
>>
>> The actual switch is done by dropping back from a HYP mode handler
>> without actually leaving HYP mode, so we introduce a new handler
>> routine in our new secure exception vector table.
>>
>> In the assembly switching routine we save and restore the banked LR
>> and SP registers around the hypercall to do the actual HYP mode
>> switch.
>>
>> The C routine first checks whether we are in HYP mode already and
>> also whether the virtualization extensions are available. It also
>> checks whether the HYP mode switch was finally successful.
>> The bootm command part only adds and adjusts some error reporting.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   arch/arm/cpu/armv7/Makefile      |  2 +-
>>   arch/arm/cpu/armv7/nonsec_virt.S | 43 +++++++++++++++++++++++++++++++++++-----
>>   arch/arm/cpu/armv7/virt-v7.c     | 31 +++++++++++++++++++++++++++++
>>   arch/arm/include/asm/armv7.h     |  9 +++++++--
>>   arch/arm/lib/bootm.c             | 19 +++++++++++++++---
>>   5 files changed, 93 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
>> index b59f59e..e5eaa56 100644
>> --- a/arch/arm/cpu/armv7/Makefile
>> +++ b/arch/arm/cpu/armv7/Makefile
>> @@ -36,7 +36,7 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF
>>   SOBJS	+= lowlevel_init.o
>>   endif
>>
>> -ifneq ($(CONFIG_ARMV7_NONSEC),)
>> +ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),)
>>   SOBJS   += nonsec_virt.o
>>   COBJS	+= virt-v7.o
>>   endif
>> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
>> index f9b6b39..895c3b0 100644
>> --- a/arch/arm/cpu/armv7/nonsec_virt.S
>> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
>> @@ -1,5 +1,5 @@
>>   /*
>> - * code for switching cores into non-secure state
>> + * code for switching cores into non-secure state and into HYP mode
>>    *
>>    * Copyright (c) 2013	Andre Przywara <andre.przywara@linaro.org>
>>    *
>> @@ -28,15 +28,16 @@
>>   #include <asm/armv7.h>
>>
>>   .arch_extension sec
>> +.arch_extension virt
>>
>> -/* the vector table for secure state */
>> +/* the vector table for secure state and HYP mode */
>>   _monitor_vectors:
>>   	.word 0	/* reset */
>>   	.word 0 /* undef */
>>   	adr pc, _secure_monitor
>>   	.word 0
>>   	.word 0
>> -	.word 0
>> +	adr pc, _hyp_trap
>>   	.word 0
>>   	.word 0
>>   	.word 0	/* pad */
>> @@ -53,10 +54,27 @@ _secure_monitor:
>>   	bic	r1, r1, #0x4e			@ clear IRQ, FIQ, EA, nET bits
>>   	orr	r1, r1, #0x31			@ enable NS, AW, FW bits
>>
>> +#ifdef CONFIG_ARMV7_VIRT
>> +	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
>> +	and	r0, r0, #CPUID_ARM_VIRT_MASK	@ mask virtualization bits
>> +	cmp	r0, #(1 << CPUID_ARM_VIRT_SHIFT)
>> +	orreq	r1, r1, #0x100			@ allow HVC instruction
>> +#endif
>> +
>>   	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
>>
>> +#ifdef CONFIG_ARMV7_VIRT
>> +	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
>> +	mcreq	p15, 4, r0, c12, c0, 0		@ write HVBAR
>> +#endif
>> +
>>   	movs	pc, lr				@ return to non-secure SVC
>>
>> +_hyp_trap:
>> +	mrs	lr, elr_hyp	@ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1
>
> this comment just confuses: either make it intelligent to support an
> older compiler or just get rid of these byte encodings.  You can always
> disassemble the file and lookup the byte code with a modern compiler to
> get back to the byte encoding.

Well, I used a Debian 6 cross compiler before, which didn't support 
these instructions. After your remark I updated the system to Debian 7, 
but found it not appropriate to ask any user to do the same just to use 
a fixed, non-parametrized assembly instruction. I have the feeling that 
there are quite some users out there who cannot and don't want to easily 
update their compiler.
So I decided to leave the workaround in the comment to give a hint to a 
quick fix.

By "making it intelligent" you mean a macro which does some version 
checking and inserts the .byte sequence if needed? Are there any 
archetypes of such code?

Regards,
Andre.

>> +	mov pc, lr				@ do no switch modes, but
>> +						@ return to caller
>> +
>>   /*
>>    * Secondary CPUs start here and call the code for the core specific parts
>>    * of the non-secure and HYP mode transition. The GIC distributor specific
>> @@ -71,9 +89,13 @@ ENTRY(_smp_pen)
>>   	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
>>
>>   	bl	_nonsec_init
>> +	mov	r12, r0				@ save GICC address
>> +#ifdef CONFIG_ARMV7_VIRT
>> +	bl	_switch_to_hyp
>> +#endif
>>
>> -	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
>> -	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
>> +	ldr	r1, [r12, #GICC_IAR]		@ acknowledge IPI
>> +	str	r1, [r12, #GICC_EOIR]		@ signal end of interrupt
>>   	adr	r1, _smp_pen
>>   waitloop:
>>   	wfi
>> @@ -164,3 +186,14 @@ ENTRY(_nonsec_init)
>>
>>   	bx	lr
>>   ENDPROC(_nonsec_init)
>> +
>> +ENTRY(_switch_to_hyp)
>> +	mov	r0, lr
>> +	mov	r1, sp				@ save SVC copy of LR and SP
>> +	isb
>
> did you find out that this isb is indeed needed? if so, why?
>
>> +	hvc #0			 @ for older asm: .byte 0x70, 0x00, 0x40, 0xe1
>
> same comment as above?
>
>> +	mov	sp, r1
>> +	mov	lr, r0				@ restore SVC copy of LR and SP
>> +
>> +	bx	lr
>> +ENDPROC(_switch_to_hyp)
>> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
>> index a0d0b34..3645572 100644
>> --- a/arch/arm/cpu/armv7/virt-v7.c
>> +++ b/arch/arm/cpu/armv7/virt-v7.c
>> @@ -3,6 +3,7 @@
>>    * Andre Przywara, Linaro
>>    *
>>    * Routines to transition ARMv7 processors from secure into non-secure state
>> + * and from non-secure SVC into HYP mode
>>    * needed to enable ARMv7 virtualization for current hypervisors
>>    *
>>    * See file CREDITS for list of people who contributed to this
>> @@ -29,6 +30,14 @@
>>   #include <asm/gic.h>
>>   #include <asm/io.h>
>>
>> +static unsigned int read_cpsr(void)
>> +{
>> +	unsigned int reg;
>> +
>> +	asm volatile ("mrs %0, cpsr\n" : "=r" (reg));
>> +	return reg;
>> +}
>> +
>>   static unsigned int read_id_pfr1(void)
>>   {
>>   	unsigned int reg;
>> @@ -92,6 +101,28 @@ static void kick_secondary_cpus(unsigned int gicdaddr)
>>   	writel(1U << 24, gicdaddr + GICD_SGIR);
>>   }
>>
>> +enum nonsec_virt_errors armv7_switch_hyp(void)
>> +{
>> +	unsigned int reg;
>> +
>> +	/* check whether we are in HYP mode already */
>> +	if ((read_cpsr() & 0x1f) == 0x1a)
>> +		return VIRT_ALREADY_HYP_MODE;
>> +
>> +	/* check whether the CPU supports the virtualization extensions */
>> +	reg = read_id_pfr1();
>> +	if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT)
>> +		return VIRT_ERR_NO_VIRT_EXT;
>> +
>> +	/* call the HYP switching code on this CPU also */
>> +	_switch_to_hyp();
>> +
>> +	if ((read_cpsr() & 0x1F) != 0x1a)
>> +		return VIRT_ERR_NOT_HYP_MODE;
>> +
>> +	return NONSEC_VIRT_SUCCESS;
>> +}
>> +
>>   enum nonsec_virt_errors armv7_switch_nonsec(void)
>>   {
>>   	unsigned int reg, ret;
>> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
>> index f6582a1..baa22fe 100644
>> --- a/arch/arm/include/asm/armv7.h
>> +++ b/arch/arm/include/asm/armv7.h
>> @@ -89,21 +89,26 @@ 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_NONSEC
>> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>>
>>   enum nonsec_virt_errors {
>>   	NONSEC_VIRT_SUCCESS,
>>   	NONSEC_ERR_NO_SEC_EXT,
>>   	NONSEC_ERR_NO_GIC_ADDRESS,
>>   	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
>> +	VIRT_ALREADY_HYP_MODE,
>> +	VIRT_ERR_NO_VIRT_EXT,
>> +	VIRT_ERR_NOT_HYP_MODE
>>   };
>>
>>   enum nonsec_virt_errors armv7_switch_nonsec(void);
>> +enum nonsec_virt_errors armv7_switch_hyp(void);
>>
>>   /* defined in assembly file */
>>   unsigned int _nonsec_init(void);
>>   void _smp_pen(void);
>> -#endif /* CONFIG_ARMV7_NONSEC */
>> +void _switch_to_hyp(void);
>> +#endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
>>
>>   #endif /* ! __ASSEMBLY__ */
>>
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 7b0619e..90875b3 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -34,7 +34,7 @@
>>   #include <asm/bootm.h>
>>   #include <linux/compiler.h>
>>
>> -#ifdef CONFIG_ARMV7_NONSEC
>> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>>   #include <asm/armv7.h>
>>   #endif
>>
>> @@ -192,13 +192,17 @@ __weak void setup_board_tags(struct tag **in_params) {}
>>
>>   static void do_nonsec_virt_switch(void)
>>   {
>> -#ifdef CONFIG_ARMV7_NONSEC
>> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>>   	int ret;
>>
>>   	ret = armv7_switch_nonsec();
>> +#ifdef CONFIG_ARMV7_VIRT
>> +	if (ret == NONSEC_VIRT_SUCCESS)
>> +		ret = armv7_switch_hyp();
>> +#endif
>>   	switch (ret) {
>>   	case NONSEC_VIRT_SUCCESS:
>> -		debug("entered non-secure state\n");
>> +		debug("entered non-secure state or HYP mode\n");
>>   		break;
>>   	case NONSEC_ERR_NO_SEC_EXT:
>>   		printf("nonsec: Security extensions not implemented.\n");
>> @@ -209,6 +213,15 @@ static void do_nonsec_virt_switch(void)
>>   	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
>>   		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
>>   		break;
>> +	case VIRT_ERR_NO_VIRT_EXT:
>> +		printf("HYP mode: Virtualization extensions not implemented.\n");
>> +		break;
>> +	case VIRT_ALREADY_HYP_MODE:
>> +		debug("CPU already in HYP mode\n");
>> +		break;
>> +	case VIRT_ERR_NOT_HYP_MODE:
>> +		printf("HYP mode: switch not successful.\n");
>> +		break;
>>   	}
>>   #endif
>>   }
>> --
>> 1.7.12.1
>>

[U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution

[Christoffer Dall]

On Tue, Jul 30, 2013 at 01:32:14PM +0200, Andre Przywara wrote:
> On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> >On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:
> >
> >[...]
> >
> >>diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> >>index 1b6e0ac..7b0619e 100644
> >>--- a/arch/arm/lib/bootm.c
> >>+++ b/arch/arm/lib/bootm.c
> >>@@ -34,6 +34,10 @@
> >>  #include <asm/bootm.h>
> >>  #include <linux/compiler.h>
> >>
> >>+#ifdef CONFIG_ARMV7_NONSEC
> >>+#include <asm/armv7.h>
> >>+#endif
> >>+
> >>  DECLARE_GLOBAL_DATA_PTR;
> >>
> >>  static struct tag *params;
> >>@@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
> >>
> >>  __weak void setup_board_tags(struct tag **in_params) {}
> >>
> >>+static void do_nonsec_virt_switch(void)
> >>+{
> >>+#ifdef CONFIG_ARMV7_NONSEC
> >>+	int ret;
> >>+
> >>+	ret = armv7_switch_nonsec();
> >>+	switch (ret) {
> >>+	case NONSEC_VIRT_SUCCESS:
> >>+		debug("entered non-secure state\n");
> >>+		break;
> >>+	case NONSEC_ERR_NO_SEC_EXT:
> >>+		printf("nonsec: Security extensions not implemented.\n");
> >>+		break;
> >>+	case NONSEC_ERR_NO_GIC_ADDRESS:
> >>+		printf("nonsec: could not determine GIC address.\n");
> >>+		break;
> >>+	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
> >>+		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
> >>+		break;
> >>+	}
> >>+#endif
> >>+}
> >
> >I still don't get why you just don't make armv7_switch_nonsec a void and
> >print the error when they occur... ???
> 
> My apologies for not elaborating on these comments I didn't incorporate:
> 
> So, I don't like the idea of marrying a low-level routine with high
> level output. I don't want to constraint the usage of the routine by
> requiring an output channel. Also some parts may not be fatal for
> all users - someone could just try to switch and then behave
> differently if that failed - without bothering the user.
> May seem a bit over-engineered, but I like it better this way ;-)
> 
> If that is a show-stopper for you, I can change it, of course.
> 
I won't hold back my ack for the patch series based on this, but I do
think it's over-engineered.  I think at least just returning -1 for
error and 0 for success (or even make it a bool) and just printing a
generic error message is cleaner - the level of details as to why the
switch to hyp/nonsec didn't work could then be debug statements that a
board developer could enable with a "#define DEBUG 1" in the
corresponding file.

But ok, we've had the conversation, if you still feel this is better and
necessary, then I'll let it be.

-Christoffer

[U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch

[Christoffer Dall]

On Tue, Jul 30, 2013 at 01:51:33PM +0200, Andre Przywara wrote:
> On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> >On Wed, Jul 10, 2013 at 01:54:17AM +0200, Andre Przywara wrote:
> >>Currently the non-secure switch is only done for the boot processor.
> >>To enable 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/nonsec_virt.S    | 27 +++++++++++++++++++++++++++
> >>  arch/arm/cpu/armv7/virt-v7.c        | 19 ++++++++++++++++++-
> >>  arch/arm/include/asm/armv7.h        |  1 +
> >>  arch/arm/include/asm/gic.h          |  2 ++
> >>  include/configs/vexpress_ca15_tc2.h |  3 +++
> >>  5 files changed, 51 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> >>index e9ee831..f9b6b39 100644
> >>--- a/arch/arm/cpu/armv7/nonsec_virt.S
> >>+++ b/arch/arm/cpu/armv7/nonsec_virt.S
> >>@@ -58,6 +58,33 @@ _secure_monitor:
> >>  	movs	pc, lr				@ return to non-secure SVC
> >>
> >>  /*
> >>+ * Secondary CPUs start here and call the code for the core specific parts
> >>+ * of the non-secure and HYP mode transition. The GIC distributor specific
> >>+ * code has already been executed by a C function before.
> >>+ * Then they go back to wfi and wait to be woken up by the kernel again.
> >>+ */
> >>+ENTRY(_smp_pen)
> >>+	mrs	r0, cpsr
> >>+	orr	r0, r0, #0xc0
> >>+	msr	cpsr, r0			@ disable interrupts
> >>+	ldr	r1, =_start
> >>+	mcr	p15, 0, r1, c12, c0, 0		@ set VBAR
> >>+
> >>+	bl	_nonsec_init
> >>+
> >>+	ldr	r1, [r0, #GICC_IAR]		@ acknowledge IPI
> >>+	str	r1, [r0, #GICC_EOIR]		@ signal end of interrupt
> >>+	adr	r1, _smp_pen
> >>+waitloop:
> >>+	wfi
> >>+	ldr	r0, =CONFIG_SYSFLAGS_ADDR	@ load start address
> >
> >You seem to have ignored my comment about using the sysflags name?
> >
> >As I understand, the sysflags name is a versatile express specific
> >register name that just happens to be used for the SMP boot address as
> >well...
> >
> >Therefore, this should really be CONFIG_SMP_BOOT_ADDR or something like
> >that, at the very least.
> 
> >
> >>+	ldr	r0, [r0]
> >>+	cmp	r0, r1			@ make sure we dont execute this code
> >>+	beq	waitloop		@ again (due to a spurious wakeup)
> >>+	mov	pc, r0
> >>+ENDPROC(_smp_pen)
> >>+
> >>+/*
> >>   * Switch a core to non-secure state.
> >>   *
> >>   *  1. initialize the GIC per-core interface
> >>diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
> >>index 54f9746..a0d0b34 100644
> >>--- a/arch/arm/cpu/armv7/virt-v7.c
> >>+++ b/arch/arm/cpu/armv7/virt-v7.c
> >>@@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
> >>  #endif
> >>  }
> >>
> >>+static void kick_secondary_cpus(unsigned int gicdaddr)
> >>+{
> >>+	unsigned int *secondary_boot_addr;
> >>+
> >>+	secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
> >>+#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
> >>+	secondary_boot_addr[1] = (unsigned)-1;
> >>+#endif
> >
> >again, if you disagreed with my previous comment, please comment on it
> >and rationalize your choice.
> 
> I am sorry, I thought I reasoned that already in an earlier mail:
> I don't want to introduce abstraction prematurely, so I wanted to
> refactor the code as soon as a second board gets supported. Which
> will probably be the Arndale, and also probably done by me ...

Hmm, a big motivation for doing this is to make it clear what new boards
need to do to support Hyp mode, and make it as easy as possible.  We
have already made the choice to make this code generic in virt-v7.c as
opposed to vexpress.c (or whatever that file would be called), so I
don't really understand your argument about being premature, to be
completely honest.

> 
> >I still feel you're wrapping board specific logic into generic code, and
> >that you should call out to a more generic function.  Imagine an SOC
> >that uses an implementation defined control register for this instead of
> >a memory address...
> 
> Well, I can imagine quite some ways to do this, but in fact I'd like
> to focus on things that really exist ;-)
> My fear is that any abstraction I introduce now will not suffice for
> a future board and needs to be refactored then anyway.
> 

Sorry, but I don't see this.  You need to do three very simple things:

get_smp_boot_addr
set_smp_boot_addr
kick_other_cpus

We can't hold back abstractions until we have a concrete example as a
general principle, because we'd never make anything generic.  We can
only try to find the right balance between some abstraction and
something concrete enough that it makes sense to users.

-Christoffer

[U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode

[Christoffer Dall]

On Tue, Jul 30, 2013 at 01:59:29PM +0200, Andre Przywara wrote:
> On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> >On Wed, Jul 10, 2013 at 01:54:18AM +0200, Andre Przywara wrote:

[...]

> >>
> >>+_hyp_trap:
> >>+	mrs	lr, elr_hyp	@ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1
> >
> >this comment just confuses: either make it intelligent to support an
> >older compiler or just get rid of these byte encodings.  You can always
> >disassemble the file and lookup the byte code with a modern compiler to
> >get back to the byte encoding.
> 
> Well, I used a Debian 6 cross compiler before, which didn't support
> these instructions. After your remark I updated the system to Debian
> 7, but found it not appropriate to ask any user to do the same just
> to use a fixed, non-parametrized assembly instruction. I have the
> feeling that there are quite some users out there who cannot and
> don't want to easily update their compiler.
> So I decided to leave the workaround in the comment to give a hint
> to a quick fix.

ok, so if Debian's built-in cross compilers are indeed that old and we
want to support those (that's ok with me), then let's fix it properly.

> 
> By "making it intelligent" you mean a macro which does some version
> checking and inserts the .byte sequence if needed? Are there any
> archetypes of such code?
> 
Yes, see arch/arm/include/asm/opcodes-*.h

-Christoffer

[U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode

[Masahiro Yamada]

Hello Andre,



> -#ifdef CONFIG_ARMV7_NONSEC
> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>  
>  enum nonsec_virt_errors {
>  	NONSEC_VIRT_SUCCESS,
>  	NONSEC_ERR_NO_SEC_EXT,
>  	NONSEC_ERR_NO_GIC_ADDRESS,
>  	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
> +	VIRT_ALREADY_HYP_MODE,
> +	VIRT_ERR_NO_VIRT_EXT,
> +	VIRT_ERR_NOT_HYP_MODE
>  };

This looks weird to me.
If you want to use enum, why don't you separate it like follows? 

enum nonsec_errors {
	NONSEC_SUCCESS,
	NONSEC_ERR_NO_SEC_EXT,
	NONSEC_ERR_NO_GIC_ADDRESS,
	NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB,
};

enum virt_errors {
	VIRT_SUCCESS,
	VIRT_ALREADY_HYP_MODE,
	VIRT_ERR_NO_VIRT_EXT,
	VIRT_ERR_NOT_HYP_MODE
};


>  enum nonsec_virt_errors armv7_switch_nonsec(void);
> +enum nonsec_virt_errors armv7_switch_hyp(void);

enum nonsec_errors armv7_switch_nonsec(void);
+enum virt_errors armv7_switch_hyp(void);


But in the first place,
I like better to fix do_nonsec_virt_switch().



>  static void do_nonsec_virt_switch(void)
>  {
> -#ifdef CONFIG_ARMV7_NONSEC
> +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
>  	int ret;
>  
>  	ret = armv7_switch_nonsec();
> +#ifdef CONFIG_ARMV7_VIRT
> +	if (ret == NONSEC_VIRT_SUCCESS)
> +		ret = armv7_switch_hyp();
> +#endif
>  	switch (ret) {
>  	case NONSEC_VIRT_SUCCESS:
> -		debug("entered non-secure state\n");
> +		debug("entered non-secure state or HYP mode\n");
>  		break;
>  	case NONSEC_ERR_NO_SEC_EXT:
>  		printf("nonsec: Security extensions not implemented.\n");
> @@ -209,6 +213,15 @@ static void do_nonsec_virt_switch(void)
>  	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
>  		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
>  		break;
> +	case VIRT_ERR_NO_VIRT_EXT:
> +		printf("HYP mode: Virtualization extensions not implemented.\n");
> +		break;
> +	case VIRT_ALREADY_HYP_MODE:
> +		debug("CPU already in HYP mode\n");
> +		break;
> +	case VIRT_ERR_NOT_HYP_MODE:
> +		printf("HYP mode: switch not successful.\n");
> +		break;
>  	}
>  #endif


As for this part, I agreed on Christoffer's opinion:


On Tue, 30 Jul 2013 07:23:58 -0700
Christoffer Dall <christoffer.dall@linaro.org> wrote:
> I won't hold back my ack for the patch series based on this, but I do
> think it's over-engineered.  I think at least just returning -1 for
> error and 0 for success (or even make it a bool) and just printing a
> generic error message is cleaner - the level of details as to why the
> switch to hyp/nonsec didn't work could then be debug statements that a
> board developer could enable with a "#define DEBUG 1" in the
> corresponding file.




Best Regards
Masahiro Yamada