Skip to contentSkip to conversation

ARMv7: add PSCI support to U-Boot

PATCH Superseded Marc Zyngier 6 participants 34 messages Apr 26, 2014 – Jun 10, 2014 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code that just calls into PSCI.

More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.

The main idea here is to turn some of the existing U-Boot code into a separate section that can live in secure RAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when U-Boot is long gone, hence the need for a piece of memory that will not be touched by the OS.

This patch series contains 3 parts:
- the first four patches are just bug fixes
- the next two refactor the HYP/non-secure code to allow relocation in secure memory
- the last four contain the generic PSCI code and DT infrastructure

This implements the original 0.1 spec, as nobody implements the new 0.2 version so far. I plan to update this support to 0.2 once there is an official binding available (and support in the kernel).

Most of the development has been done on an Allwinner A20 SoC, which is the main user of this code at the moment. I hope new SoCs will be using this method in the future (my primary goal for this series being to avoid more stupid SMP code from creeping up in the Linux kernel). As instructed, I've removed the A20 support code and made it a separate series, as there is now an effort to mainline this code (see Ian Campbell patch series).

With these three series applied, the A20 now boots in HYP mode, Linux finds the secondary CPU without any SMP code present in the kernel, and runs KVM out of the box. The Xen/ARM guys managed to do the same fairly easily, as did at least one XVizor user.

This code has also been tested on a VExpress TC2, running KVM with all 5 CPUs, in order to make sure there was no obvious regression.

The code is also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/u-boot.git wip/psci-v4

A fully merged branch with the A20 support is in the wip/psci-v4-a20 branch of the same repo.

Cheers,

        M.
#1 lore raw

PATCH v4 1/10 ARM: HYP/non-sec: move switch to non-sec to the last boot phase

Having the switch to non-secure in the "prep" phase is causing all kind of troubles, as that stage can be called multiple times.

Instead, move the switch to non-secure to the last possible phase, when there is no turning back anymore.

---
 arch/arm/lib/bootm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/lib/bootm.c +3 -2
index 47ee070..10634a4 100644
@@ -242,7 +242,6 @@ 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 */
@@ -287,8 +286,10 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
 	else
 		r2 = gd->bd->bi_boot_params;
 
-	if (!fake)
+	if (!fake) {
+		do_nonsec_virt_switch();
 		kernel_entry(0, machid, r2);
+	}
 #endif
 }
 
Signature · 1 line
1.9.2
#2 lore raw

PATCH v4 2/10 ARM: HYP/non-sec: add a barrier after setting SCR.NS==1

In reply to Marc Zyngier

A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 1 +
 1 file changed, 1 insertion(+)
arch/arm/cpu/armv7/nonsec_virt.S +1 -0
index 6367e09..12de5c2 100644
@@ -46,6 +46,7 @@ _secure_monitor:
 #endif
 
 	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
+	isb
 
 #ifdef CONFIG_ARMV7_VIRT
 	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
Signature · 1 line
1.9.2
#3 lore raw

PATCH v4 3/10 ARM: non-sec: reset CNTVOFF to zero

In reply to Marc Zyngier

Before switching to non-secure, make sure that CNTVOFF is set to zero on all CPUs. Otherwise, kernel running in non-secure without HYP enabled (hence using virtual timers) may observe timers that are not synchronized, effectively seeing time going backward...

---
 arch/arm/cpu/armv7/nonsec_virt.S | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/arm/cpu/armv7/nonsec_virt.S +8 -1
index 12de5c2..b5c946f 100644
@@ -38,10 +38,10 @@ _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)
+#ifdef CONFIG_ARMV7_VIRT
 	orreq	r1, r1, #0x100			@ allow HVC instruction
 #endif
 
@@ -52,7 +52,14 @@ _secure_monitor:
 	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
 	mcreq	p15, 4, r0, c12, c0, 0		@ write HVBAR
 #endif
+	bne	1f
 
+	@ Reset CNTVOFF to 0 before leaving monitor mode
+	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
+	ands	r0, r0, #CPUID_ARM_GENTIMER_MASK	@ test arch timer bits
+	movne	r0, #0
+	mcrrne	p15, 4, r0, r0, c14		@ Reset CNTVOFF to zero
+1:
 	movs	pc, lr				@ return to non-secure SVC
 
 _hyp_trap:
Signature · 1 line
1.9.2
#4 lore raw

PATCH v4 4/10 ARM: add missing HYP mode constant

In reply to Marc Zyngier

In order to be able to use the various mode constants (far more readable than random hex values), add the missing HYP and A values.

Also update arm/lib/interrupts.c to display HYP instead of an unknown value.

---
 arch/arm/include/asm/proc-armv/ptrace.h | 2 ++
 arch/arm/lib/interrupts.c               | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
arch/arm/include/asm/proc-armv/ptrace.h +2 -0
index 21aef58..71df5a9 100644
@@ -38,12 +38,14 @@ struct pt_regs {
 #define IRQ_MODE	0x12
 #define SVC_MODE	0x13
 #define ABT_MODE	0x17
+#define HYP_MODE	0x1a
 #define UND_MODE	0x1b
 #define SYSTEM_MODE	0x1f
 #define MODE_MASK	0x1f
 #define T_BIT		0x20
 #define F_BIT		0x40
 #define I_BIT		0x80
+#define A_BIT		0x100
 #define CC_V_BIT	(1 << 28)
 #define CC_C_BIT	(1 << 29)
 #define CC_Z_BIT	(1 << 30)
arch/arm/lib/interrupts.c +1 -1
index 758b013..f6b7c03 100644
@@ -103,7 +103,7 @@ void show_regs (struct pt_regs *regs)
 	"UK12_26",	"UK13_26",	"UK14_26",	"UK15_26",
 	"USER_32",	"FIQ_32",	"IRQ_32",	"SVC_32",
 	"UK4_32",	"UK5_32",	"UK6_32",	"ABT_32",
-	"UK8_32",	"UK9_32",	"UK10_32",	"UND_32",
+	"UK8_32",	"UK9_32",	"HYP_32",	"UND_32",
 	"UK12_32",	"UK13_32",	"UK14_32",	"SYS_32",
 	};
 
Signature · 1 line
1.9.2
#5 lore raw

PATCH v4 5/10 ARM: HYP/non-sec: add separate section for secure code

In reply to Marc Zyngier

In anticipation of refactoring the HYP/non-secure code to run from secure RAM, add a new linker section that will contain that code.

Nothing is using it just yet.

---
 arch/arm/config.mk      |  2 +-
 arch/arm/cpu/u-boot.lds | 30 ++++++++++++++++++++++++++++++
 arch/arm/lib/sections.c |  2 ++
 3 files changed, 33 insertions(+), 1 deletion(-)
arch/arm/config.mk +1 -1
index 66ecc2e..a729ed2 100644
@@ -113,7 +113,7 @@ endif
 ifdef CONFIG_ARM64
 OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
 else
-OBJCOPYFLAGS += -j .text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
+OBJCFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
 endif
 
 ifneq ($(CONFIG_IMX_CONFIG),)
arch/arm/cpu/u-boot.lds +30 -0
index 33c1f99..f45885d 100644
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <config.h>
+
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
 ENTRY(_start)
@@ -22,6 +24,34 @@ SECTIONS
 		*(.text*)
 	}
 
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT) || defined(CONFIG_ARMV7_PSCI)
+
+#ifndef CONFIG_ARMV7_SECURE_BASE
+#define CONFIG_ARMV7_SECURE_BASE
+#endif
+
+	.__secure_start : {
+		. = ALIGN(0x1000);
+		*(.__secure_start)
+	}
+
+	.secure_text CONFIG_ARMV7_SECURE_BASE :
+		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
+	{
+		*(._secure.text)
+	}
+
+	. = LOADADDR(.__secure_start) +
+		SIZEOF(.__secure_start) +
+		SIZEOF(.secure_text);
+
+	__secure_end_lma = .;
+	.__secure_end : AT(__secure_end_lma) {
+		*(.__secure_end)
+		LONG(0x1d1071c);	/* Must output something to reset LMA */
+	}
+#endif
+
 	. = ALIGN(4);
 	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
 
arch/arm/lib/sections.c +2 -0
index 5b30bcb..a1205c3 100644
@@ -25,4 +25,6 @@ char __image_copy_start[0] __attribute__((section(".__image_copy_start")));
 char __image_copy_end[0] __attribute__((section(".__image_copy_end")));
 char __rel_dyn_start[0] __attribute__((section(".__rel_dyn_start")));
 char __rel_dyn_end[0] __attribute__((section(".__rel_dyn_end")));
+char __secure_start[0] __attribute__((section(".__secure_start")));
+char __secure_end[0] __attribute__((section(".__secure_end")));
 char _end[0] __attribute__((section(".__end")));
Signature · 1 line
1.9.2
#6 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

In reply to Marc Zyngier

The current non-sec switching code suffers from one major issue:
it cannot run in secure RAM, as a large part of u-boot still needs to be run while we're switched to non-secure.

This patch reworks the whole HYP/non-secure strategy by:
- making sure the secure code is the *last* thing u-boot executes before entering the payload
- performing an exception return from secure mode directly into the payload
- allowing the code to be dynamically relocated to secure RAM before switching to non-secure.

This involves quite a bit of horrible code, specially as u-boot relocation is quite primitive.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 161 +++++++++++++++++++--------------------
 arch/arm/cpu/armv7/virt-v7.c     |  59 +++++---------
 arch/arm/include/asm/armv7.h     |  10 ++-
 arch/arm/include/asm/secure.h    |  26 +++++++
 arch/arm/lib/bootm.c             |  22 +++---
 5 files changed, 138 insertions(+), 140 deletions(-)
 create mode 100644 arch/arm/include/asm/secure.h
arch/arm/cpu/armv7/nonsec_virt.S +77 -84
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+	.pushsection ._secure.text, "ax"
+
 	.align	5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
 	adr pc, _secure_monitor
 	.word 0
 	.word 0
-	adr pc, _hyp_trap
+	.word 0
 	.word 0
 	.word 0
 
+.macro is_cpu_virt_capable	tmp
+	mrc	p15, 0, \tmp, c0, c1, 1		@ read ID_PFR1
+	and	\tmp, \tmp, #CPUID_ARM_VIRT_MASK	@ mask virtualization bits
+	cmp	\tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm
+
 /*
  * secure monitor handler
  * U-boot calls this "software interrupt" in start.S
  * 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.
+ * r0, r1, r2: passed to the callee
+ * ip: target PC
  */
 _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
+	mrc	p15, 0, r5, c1, c1, 0		@ read SCR
+	bic	r5, r5, #0x4e			@ clear IRQ, FIQ, EA, nET bits
+	orr	r5, r5, #0x31			@ enable NS, AW, FW bits
 
-	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)
+	mov	r6, #SVC_MODE			@ default mode is SVC
+	is_cpu_virt_capable r4
 #ifdef CONFIG_ARMV7_VIRT
-	orreq	r1, r1, #0x100			@ allow HVC instruction
+	orreq	r5, r5, #0x100			@ allow HVC instruction
+	moveq	r6, #HYP_MODE			@ Enter the kernel as HYP
 #endif
 
-	mcr	p15, 0, r1, c1, c1, 0		@ write SCR (with NS bit set)
+	mcr	p15, 0, r5, c1, c1, 0		@ write SCR (with NS bit set)
 	isb
 
-#ifdef CONFIG_ARMV7_VIRT
-	mrceq	p15, 0, r0, c12, c0, 1		@ get MVBAR value
-	mcreq	p15, 4, r0, c12, c0, 0		@ write HVBAR
-#endif
 	bne	1f
 
 	@ Reset CNTVOFF to 0 before leaving monitor mode
-	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
-	ands	r0, r0, #CPUID_ARM_GENTIMER_MASK	@ test arch timer bits
-	movne	r0, #0
-	mcrrne	p15, 4, r0, r0, c14		@ Reset CNTVOFF to zero
+	mrc	p15, 0, r4, c0, c1, 1		@ read ID_PFR1
+	ands	r4, r4, #CPUID_ARM_GENTIMER_MASK	@ test arch timer bits
+	movne	r4, #0
+	mcrrne	p15, 4, r4, r4, c14		@ Reset CNTVOFF to zero
 1:
-	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
-
+	mov	lr, ip
+	mov	ip, #(F_BIT | I_BIT | A_BIT)	@ Set A, I and F
+	tst	lr, #1				@ Check for Thumb PC
+	orrne	ip, ip, #T_BIT			@ Set T if Thumb
+	orr	ip, ip, r6			@ Slot target mode in
+	msr	spsr_cxfs, ip			@ Set full SPSR
+	movs	pc, lr				@ ERET to non-secure
+
+ENTRY(_do_nonsec_entry)
+	mov	ip, r0
+	mov	r0, r1
+	mov	r1, r2
+	mov	r2, r3
+	smc	#0
+ENDPROC(_do_nonsec_entry)
+
+.macro get_cbar_addr	addr
+#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
+	ldr	\addr, =CONFIG_ARM_GIC_BASE_ADDRESS
+#else
+	mrc	p15, 4, \addr, c15, c0, 0	@ read CBAR
+	bfc	\addr, #0, #15			@ clear reserved bits
+#endif
+.endm
+
+.macro get_gicd_addr	addr
+	get_cbar_addr	\addr
+	add	\addr, \addr, #GIC_DIST_OFFSET	@ GIC dist i/f offset
+.endm
+
+.macro get_gicc_addr	addr, tmp
+	get_cbar_addr	\addr
+	is_cpu_virt_capable \tmp
+	movne	\tmp, #GIC_CPU_OFFSET_A9	@ GIC CPU offset for A9
+	moveq	\tmp, #GIC_CPU_OFFSET_A15	@ GIC CPU offset for A15/A7
+	add	\addr, \addr, \tmp
+.endm
+
+#ifndef CONFIG_ARMV7_PSCI
 /*
  * 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
@@ -74,31 +112,21 @@ _hyp_trap:
  * 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
+	cpsid	i
+	cpsid	f
 
 	bl	_nonsec_init
-	mov	r12, r0				@ save GICC address
-#ifdef CONFIG_ARMV7_VIRT
-	bl	_switch_to_hyp
-#endif
-
-	ldr	r1, [r12, #GICC_IAR]		@ acknowledge IPI
-	str	r1, [r12, #GICC_EOIR]		@ signal end of interrupt
 
 	adr	r0, _smp_pen			@ do not use this address again
 	b	smp_waitloop			@ wait for IPIs, board specific
 ENDPROC(_smp_pen)
+#endif
 
 /*
  * 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
@@ -108,38 +136,23 @@ ENDPROC(_smp_pen)
  * 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
-	bfc	r2, #0, #15			@ clear reserved bits
-#endif
-	add	r3, r2, #GIC_DIST_OFFSET	@ GIC dist i/f offset
+	get_gicd_addr	r3
+
 	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
+	get_gicc_addr	r3, r1
 
-	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]
+	mov	r1, #3				@ Enable both groups
 	str	r1, [r3, #GICC_CTLR]		@ and clear all other bits
 	mov	r1, #0xff
 	str	r1, [r3, #GICC_PMR]		@ set priority mask register
 
+	mrc	p15, 0, r0, c1, c1, 2
 	movw	r1, #0x3fff
-	movt	r1, #0x0006
-	mcr	p15, 0, r1, c1, c1, 2		@ NSACR = all copros to non-sec
+	movt	r1, #0x0004
+	orr	r0, r0, r1
+	mcr	p15, 0, r0, 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
@@ -157,21 +170,9 @@ ENTRY(_nonsec_init)
 
 	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)
 
@@ -183,18 +184,10 @@ ENTRY(smp_waitloop)
 	ldr	r1, [r1]
 	cmp	r0, r1			@ make sure we dont execute this code
 	beq	smp_waitloop		@ again (due to a spurious wakeup)
-	mov	pc, r1
+	mov	r0, r1
+	b	_do_nonsec_entry
 ENDPROC(smp_waitloop)
 .weak smp_waitloop
 #endif
 
-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)
+	.popsection
arch/arm/cpu/armv7/virt-v7.c +19 -40

88 lines not on this page · raw message

arch/arm/include/asm/armv7.h +7 -3
index 395444e..11476dd 100644
@@ -78,13 +78,17 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 
-int armv7_switch_nonsec(void);
-int armv7_switch_hyp(void);
+int armv7_init_nonsec(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
+void _do_nonsec_entry(void *target_pc, unsigned long r0,
+		      unsigned long r1, unsigned long r2);
 void _smp_pen(void);
-void _switch_to_hyp(void);
+
+extern char __secure_start[];
+extern char __secure_end[];
+
 #endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
 
 #endif /* ! __ASSEMBLY__ */
arch/arm/include/asm/secure.h new file +26 -0
new file mode 100644
index 0000000..effdb18
@@ -0,0 +1,26 @@
+#ifndef __ASM_SECURE_H
+#define __ASM_SECURE_H
+
+#include <config.h>
+
+#ifdef CONFIG_ARMV7_SECURE_BASE
+/*
+ * Warning, horror ahead.
+ *
+ * The target code lives in our "secure ram", but u-boot doesn't know
+ * that, and has blindly added reloc_off to every relocation
+ * entry. Gahh. Do the opposite conversion. This hack also prevents
+ * GCC from generating code veeners, which u-boot doesn't relocate at
+ * all...
+ */
+#define secure_ram_addr(_fn) ({						\
+			DECLARE_GLOBAL_DATA_PTR;			\
+			void *__fn = _fn;				\
+			typeof(_fn) *__tmp = (__fn - gd->reloc_off);	\
+			__tmp;						\
+		})
+#else
+#define secure_ram_addr(_fn)	(_fn)
+#endif
+
+#endif
arch/arm/lib/bootm.c +9 -13
index 10634a4..61aa14e 100644
@@ -20,6 +20,7 @@
 #include <libfdt.h>
 #include <fdt_support.h>
 #include <asm/bootm.h>
+#include <asm/secure.h>
 #include <linux/compiler.h>
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
@@ -184,27 +185,17 @@ static void setup_end_tag(bd_t *bd)
 
 __weak void setup_board_tags(struct tag **in_params) {}
 
+#ifdef CONFIG_ARM64
 static void do_nonsec_virt_switch(void)
 {
-#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
-	if (armv7_switch_nonsec() == 0)
-#ifdef CONFIG_ARMV7_VIRT
-		if (armv7_switch_hyp() == 0)
-			debug("entered HYP mode\n");
-#else
-		debug("entered non-secure state\n");
-#endif
-#endif
-
-#ifdef CONFIG_ARM64
 	smp_kick_all_cpus();
 	flush_dcache_all();	/* flush cache before swtiching to EL2 */
 	armv8_switch_to_el2();
 #ifdef CONFIG_ARMV8_SWITCH_TO_EL1
 	armv8_switch_to_el1();
 #endif
-#endif
 }
+#endif
 
 /* Subcommand: PREP */
 static void boot_prep_linux(bootm_headers_t *images)
@@ -287,8 +278,13 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
 		r2 = gd->bd->bi_boot_params;
 
 	if (!fake) {
-		do_nonsec_virt_switch();
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+		armv7_init_nonsec();
+		secure_ram_addr(_do_nonsec_entry)(kernel_entry,
+						  0, machid, r2);
+#else
 		kernel_entry(0, machid, r2);
+#endif
 	}
 #endif
 }
Signature · 1 line
1.9.2
#7 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

Mark,

I finally have all this working for me on an A9 system too!

However, there were a few things that I had to change a bit. For example, by CPUs will always come out of reset at 0x0 and I do not have the ability to set their first-fetch address to anything else. To accommodate this, I need to ensure that the _monitor_vectors are loaded at address 0x0, and that the first entry in the exception vector (for reset) jumped to some notion of "secure_reset code". So I changed this code:

Quoted 22 lines
arch/arm/cpu/armv7/nonsec_virt.S +3 -0
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+       .pushsection ._secure.text, "ax"
+
       .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
       adr pc, _secure_monitor
       .word 0
       .word 0

arch/arm/cpu/armv7/nonsec_virt.S _monitor_vectors: · line 36

Quoted 10 lines
arch/arm/cpu/armv7/nonsec_virt.S +6 -1
-       adr pc, _hyp_trap
+       .word 0
       .word 0
        .word 0
 
+.macro is_cpu_virt_capable     tmp
+       mrc     p15, 0, \tmp, c0, c1, 1         @ read ID_PFR1
+       and     \tmp, \tmp, #CPUID_ARM_VIRT_MASK        @ mask virtualization bits
+       cmp     \tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm

So that it did this too:

@@ -20,15 +20,23 @@
.align 5
/* the vector table for secure state and HYP mode */ _monitor_vectors:

-       .word 0 /* reset */
-       .word 0 /* undef */
-       adr pc, _secure_monitor
+       ldr pc, _secure_reset   /* reset */
+       .word 0                 /* undef */
+       adr pc, _secure_monitor /* SMC */
        .word 0
        .word 0
        .word 0
        .word 0
        .word 0

+
+_secure_reset:
+#ifdef CONFIG_SECURE_MONITOR_RESET_FUNCTION
+ .word CONFIG_SECURE_MONITOR_RESET_FUNCTION
+#else
+ .word 0
+#endif
+
.macro is_cpu_virt_capable tmp

That enabled me to define CONFIG_SECURE_MONITOR_RESET_FUNCTION in my config header file:

/*
* With the Secure Monitor at 0x0, its reset vector must also
* then point off to the correct "out-of-reset entry function." */
#define CONFIG_SECURE_MONITOR_RESET_FUNCTION _myplatform_cpu_entry
#define CONFIG_ARMV7_SECURE_BASE 0x0

That _myplatform_cpu_entry corresponds to your sunxi_cpu_entry code.

So, yeah, I know that isn't a proper patch and all. :-) I'm just sending you more information to ponder for this patch series! If you would like to generalize your patch this way, please feel free to do so. If not, I can send a proper patch after this hits mainline or so.

HTH,
jdl

Quoted 498 lines · Marc Zyngier

The current non-sec switching code suffers from one major issue:
it cannot run in secure RAM, as a large part of u-boot still needs to be run while we're switched to non-secure.

This patch reworks the whole HYP/non-secure strategy by:
- making sure the secure code is the *last* thing u-boot executes before entering the payload
- performing an exception return from secure mode directly into the payload
- allowing the code to be dynamically relocated to secure RAM before switching to non-secure.

This involves quite a bit of horrible code, specially as u-boot relocation is quite primitive.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 161 +++++++++++++++++++--------------------
 arch/arm/cpu/armv7/virt-v7.c     |  59 +++++---------
 arch/arm/include/asm/armv7.h     |  10 ++-
 arch/arm/include/asm/secure.h    |  26 +++++++
 arch/arm/lib/bootm.c             |  22 +++---
 5 files changed, 138 insertions(+), 140 deletions(-)
 create mode 100644 arch/arm/include/asm/secure.h
arch/arm/cpu/armv7/nonsec_virt.S +77 -84
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+       .pushsection ._secure.text, "ax"
+
        .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
        adr pc, _secure_monitor
        .word 0
        .word 0
-       adr pc, _hyp_trap
+       .word 0
        .word 0
        .word 0
 
+.macro is_cpu_virt_capable     tmp
+       mrc     p15, 0, \tmp, c0, c1, 1         @ read ID_PFR1
+       and     \tmp, \tmp, #CPUID_ARM_VIRT_MASK        @ mask virtualization bits
+       cmp     \tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm
+
 /*
  * secure monitor handler
  * U-boot calls this "software interrupt" in start.S
  * 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.
+ * r0, r1, r2: passed to the callee
+ * ip: target PC
  */
 _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
+       mrc     p15, 0, r5, c1, c1, 0           @ read SCR
+       bic     r5, r5, #0x4e                   @ clear IRQ, FIQ, EA, nET bits
+       orr     r5, r5, #0x31                   @ enable NS, AW, FW bits
 
-       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)
+       mov     r6, #SVC_MODE                   @ default mode is SVC
+       is_cpu_virt_capable r4
 #ifdef CONFIG_ARMV7_VIRT
-       orreq   r1, r1, #0x100                  @ allow HVC instruction
+       orreq   r5, r5, #0x100                  @ allow HVC instruction
+       moveq   r6, #HYP_MODE                   @ Enter the kernel as HYP
 #endif
 
-       mcr     p15, 0, r1, c1, c1, 0           @ write SCR (with NS bit set)
+       mcr     p15, 0, r5, c1, c1, 0           @ write SCR (with NS bit set)
        isb
 
-#ifdef CONFIG_ARMV7_VIRT
-       mrceq   p15, 0, r0, c12, c0, 1          @ get MVBAR value
-       mcreq   p15, 4, r0, c12, c0, 0          @ write HVBAR
-#endif
        bne     1f
 
        @ Reset CNTVOFF to 0 before leaving monitor mode
-       mrc     p15, 0, r0, c0, c1, 1           @ read ID_PFR1
-       ands    r0, r0, #CPUID_ARM_GENTIMER_MASK        @ test arch timer bits
-       movne   r0, #0
-       mcrrne  p15, 4, r0, r0, c14             @ Reset CNTVOFF to zero
+       mrc     p15, 0, r4, c0, c1, 1           @ read ID_PFR1
+       ands    r4, r4, #CPUID_ARM_GENTIMER_MASK        @ test arch timer bits
+       movne   r4, #0
+       mcrrne  p15, 4, r4, r4, c14             @ Reset CNTVOFF to zero
 1:
-       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
-
+       mov     lr, ip
+       mov     ip, #(F_BIT | I_BIT | A_BIT)    @ Set A, I and F
+       tst     lr, #1                          @ Check for Thumb PC
+       orrne   ip, ip, #T_BIT                  @ Set T if Thumb
+       orr     ip, ip, r6                      @ Slot target mode in
+       msr     spsr_cxfs, ip                   @ Set full SPSR
+       movs    pc, lr                          @ ERET to non-secure
+
+ENTRY(_do_nonsec_entry)
+       mov     ip, r0
+       mov     r0, r1
+       mov     r1, r2
+       mov     r2, r3
+       smc     #0
+ENDPROC(_do_nonsec_entry)
+
+.macro get_cbar_addr   addr
+#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
+       ldr     \addr, =CONFIG_ARM_GIC_BASE_ADDRESS
+#else
+       mrc     p15, 4, \addr, c15, c0, 0       @ read CBAR
+       bfc     \addr, #0, #15                  @ clear reserved bits
+#endif
+.endm
+
+.macro get_gicd_addr   addr
+       get_cbar_addr   \addr
+       add     \addr, \addr, #GIC_DIST_OFFSET  @ GIC dist i/f offset
+.endm
+
+.macro get_gicc_addr   addr, tmp
+       get_cbar_addr   \addr
+       is_cpu_virt_capable \tmp
+       movne   \tmp, #GIC_CPU_OFFSET_A9        @ GIC CPU offset for A9
+       moveq   \tmp, #GIC_CPU_OFFSET_A15       @ GIC CPU offset for A15/A7
+       add     \addr, \addr, \tmp
+.endm
+
+#ifndef CONFIG_ARMV7_PSCI
 /*
  * 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
@@ -74,31 +112,21 @@ _hyp_trap:
  * 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
+       cpsid   i
+       cpsid   f
 
        bl      _nonsec_init
-       mov     r12, r0                         @ save GICC address
-#ifdef CONFIG_ARMV7_VIRT
-       bl      _switch_to_hyp
-#endif
-
-       ldr     r1, [r12, #GICC_IAR]            @ acknowledge IPI
-       str     r1, [r12, #GICC_EOIR]           @ signal end of interrupt
 
        adr     r0, _smp_pen                    @ do not use this address again
        b       smp_waitloop                    @ wait for IPIs, board specific
 ENDPROC(_smp_pen)
+#endif
 
 /*
  * 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
@@ -108,38 +136,23 @@ ENDPROC(_smp_pen)
  * 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
-       bfc     r2, #0, #15                     @ clear reserved bits
-#endif
-       add     r3, r2, #GIC_DIST_OFFSET        @ GIC dist i/f offset
+       get_gicd_addr   r3
+
        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
+       get_gicc_addr   r3, r1
 
-       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]
+       mov     r1, #3                          @ Enable both groups
        str     r1, [r3, #GICC_CTLR]            @ and clear all other bits
        mov     r1, #0xff
        str     r1, [r3, #GICC_PMR]             @ set priority mask register
 
+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, 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
@@ -157,21 +170,9 @@ ENTRY(_nonsec_init)
 
        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)
 
@@ -183,18 +184,10 @@ ENTRY(smp_waitloop)
        ldr     r1, [r1]
        cmp     r0, r1                  @ make sure we dont execute this code
        beq     smp_waitloop            @ again (due to a spurious wakeup)
-       mov     pc, r1
+       mov     r0, r1
+       b       _do_nonsec_entry
 ENDPROC(smp_waitloop)
 .weak smp_waitloop
 #endif
 
-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)
+       .popsection
arch/arm/cpu/armv7/virt-v7.c +19 -40
index 2cd604f..6500030 100644
@@ -13,17 +13,10 @@
 #include <asm/armv7.h>
 #include <asm/gic.h>
 #include <asm/io.h>
+#include <asm/secure.h>
 
 unsigned long gic_dist_addr;
 
-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;
@@ -72,6 +65,18 @@ static unsigned long get_gicd_base_address(void)
 #endif
 }
 
+static void relocate_secure_section(void)
+{
+#ifdef CONFIG_ARMV7_SECURE_BASE
+       size_t sz = __secure_end - __secure_start;
+
+       memcpy((void *)CONFIG_ARMV7_SECURE_BASE, __secure_start, sz);
+       flush_dcache_range(CONFIG_ARMV7_SECURE_BASE,
+                          CONFIG_ARMV7_SECURE_BASE + sz + 1);
+       invalidate_icache_all();
+#endif
+}
+
 static void kick_secondary_cpus_gic(unsigned long gicdaddr)
 {
        /* kick all CPUs (except this one) by writing to GICD_SGIR */
@@ -83,35 +88,7 @@ void __weak smp_kick_all_cpus(void)
        kick_secondary_cpus_gic(gic_dist_addr);
 }
 
-int armv7_switch_hyp(void)
-{
-       unsigned int reg;
-
-       /* check whether we are in HYP mode already */
-       if ((read_cpsr() & 0x1f) == 0x1a) {
-               debug("CPU already in HYP mode\n");
-               return 0;
-       }
-
-       /* check whether the CPU supports the virtualization extensions */
-       reg = read_id_pfr1();
-       if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT) {
-               printf("HYP mode: Virtualization extensions not implemented.\n");
-               return -1;
-       }
-
-       /* call the HYP switching code on this CPU also */
-       _switch_to_hyp();
-
-       if ((read_cpsr() & 0x1F) != 0x1a) {
-               printf("HYP mode: switch not successful.\n");
-               return -1;
-       }
-
-       return 0;
-}
-
-int armv7_switch_nonsec(void)
+int armv7_init_nonsec(void)
 {
        unsigned int reg;
        unsigned itlinesnr, i;
@@ -147,11 +124,13 @@ int armv7_switch_nonsec(void)
        for (i = 1; i <= itlinesnr; i++)
                writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
 
-       smp_set_core_boot_addr((unsigned long)_smp_pen, -1);
+#ifndef CONFIG_ARMV7_PSCI
+       smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen), -1);
        smp_kick_all_cpus();
+#endif
 
        /* call the non-sec switching code on this CPU also */
-       _nonsec_init();
-
+       relocate_secure_section();
+       secure_ram_addr(_nonsec_init)();
        return 0;
 }
arch/arm/include/asm/armv7.h +7 -3
index 395444e..11476dd 100644
@@ -78,13 +78,17 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 
-int armv7_switch_nonsec(void);
-int armv7_switch_hyp(void);
+int armv7_init_nonsec(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
+void _do_nonsec_entry(void *target_pc, unsigned long r0,
+                     unsigned long r1, unsigned long r2);
 void _smp_pen(void);
-void _switch_to_hyp(void);
+
+extern char __secure_start[];
+extern char __secure_end[];
+
 #endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
 
 #endif /* ! __ASSEMBLY__ */
arch/arm/include/asm/secure.h new file +26 -0
new file mode 100644
index 0000000..effdb18
@@ -0,0 +1,26 @@
+#ifndef __ASM_SECURE_H
+#define __ASM_SECURE_H
+
+#include <config.h>
+
+#ifdef CONFIG_ARMV7_SECURE_BASE
+/*
+ * Warning, horror ahead.
+ *
+ * The target code lives in our "secure ram", but u-boot doesn't know
+ * that, and has blindly added reloc_off to every relocation
+ * entry. Gahh. Do the opposite conversion. This hack also prevents
+ * GCC from generating code veeners, which u-boot doesn't relocate at
+ * all...
+ */
+#define secure_ram_addr(_fn) ({                                                \
+                       DECLARE_GLOBAL_DATA_PTR;                        \
+                       void *__fn = _fn;                               \
+                       typeof(_fn) *__tmp = (__fn - gd->reloc_off);    \
+                       __tmp;                                          \
+               })
+#else
+#define secure_ram_addr(_fn)   (_fn)
+#endif
+
+#endif
arch/arm/lib/bootm.c +9 -14
index 10634a4..61aa14e 100644
@@ -20,6 +20,7 @@
 #include <libfdt.h>
 #include <fdt_support.h>
 #include <asm/bootm.h>
+#include <asm/secure.h>
 #include <linux/compiler.h>
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
@@ -184,27 +185,17 @@ static void setup_end_tag(bd_t *bd)
 
 __weak void setup_board_tags(struct tag **in_params) {}
 
+#ifdef CONFIG_ARM64
 static void do_nonsec_virt_switch(void)
 {
-#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
-       if (armv7_switch_nonsec() == 0)
-#ifdef CONFIG_ARMV7_VIRT
-               if (armv7_switch_hyp() == 0)
-                       debug("entered HYP mode\n");
-#else
-               debug("entered non-secure state\n");
-#endif
-#endif
-
-#ifdef CONFIG_ARM64
        smp_kick_all_cpus();
        flush_dcache_all();     /* flush cache before swtiching to EL2 */
        armv8_switch_to_el2();
 #ifdef CONFIG_ARMV8_SWITCH_TO_EL1
        armv8_switch_to_el1();
 #endif
-#endif
 }
+#endif
 
 /* Subcommand: PREP */
 static void boot_prep_linux(bootm_headers_t *images)
@@ -287,8 +278,13 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
                r2 = gd->bd->bi_boot_params;
 
        if (!fake) {
-               do_nonsec_virt_switch();
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+               armv7_init_nonsec();
+               secure_ram_addr(_do_nonsec_entry)(kernel_entry,
+                                                 0, machid, r2);
+#else
                kernel_entry(0, machid, r2);
+#endif
        }
 #endif
 }
--

1.9.2

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#8 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

Mark,

In your nonsec_init code, you suggest this change:

+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

Leaving:

       mrc     p15, 0, r0, c1, c1, 2
       movw    r1, #0x3fff
       movt    r1, #0x0004
       orr     r0, r0, r1
       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

That sets all the co-processor bits, but the man page suggests that only copros with bits 10 and 11 should be modified. It also seems that if the PLE is enabled, we should mark it NS-enabled at bit 16 also:. Perhaps:

    mrc    p15, 0, r0, c1, c1, 2
    movw    r1, #0x0c00
    movt    r1, #0x0005
    orr    r0, r0, r1
    mcr    p15, 0, r0, c1, c1, 2        @ NSACR = all copros to non-sec

HTH,
jdl

Quoted 593 lines · Jon Loeliger

Mark,

I finally have all this working for me on an A9 system too!

However, there were a few things that I had to change a bit. For example, by CPUs will always come out of reset at 0x0 and I do not have the ability to set their first-fetch address to anything else. To accommodate this, I need to ensure that the _monitor_vectors are loaded at address 0x0, and that the first entry in the exception vector (for reset) jumped to some notion of "secure_reset code". So I changed this code:

Quoted 22 lines
arch/arm/cpu/armv7/nonsec_virt.S +3 -0
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+       .pushsection ._secure.text, "ax"
+
       .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
       adr pc, _secure_monitor
       .word 0
       .word 0

arch/arm/cpu/armv7/nonsec_virt.S _monitor_vectors: · line 36

Quoted 10 lines
arch/arm/cpu/armv7/nonsec_virt.S +6 -1
-       adr pc, _hyp_trap
+       .word 0
       .word 0
        .word 0
 
+.macro is_cpu_virt_capable     tmp
+       mrc     p15, 0, \tmp, c0, c1, 1         @ read ID_PFR1
+       and     \tmp, \tmp, #CPUID_ARM_VIRT_MASK        @ mask virtualization bits
+       cmp     \tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm

So that it did this too:

(quoted diff) +11 -3
@@ -20,15 +20,23 @@
        .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
-       .word 0 /* reset */
-       .word 0 /* undef */
-       adr pc, _secure_monitor
+       ldr pc, _secure_reset   /* reset */
+       .word 0                 /* undef */
+       adr pc, _secure_monitor /* SMC */
        .word 0
        .word 0
        .word 0
        .word 0
        .word 0
 
+
+_secure_reset:
+#ifdef CONFIG_SECURE_MONITOR_RESET_FUNCTION
+       .word CONFIG_SECURE_MONITOR_RESET_FUNCTION
+#else
+       .word 0
+#endif
+
 .macro is_cpu_virt_capable     tmp
 

That enabled me to define CONFIG_SECURE_MONITOR_RESET_FUNCTION in my config header file:

/*
* With the Secure Monitor at 0x0, its reset vector must also
* then point off to the correct "out-of-reset entry function." */
#define CONFIG_SECURE_MONITOR_RESET_FUNCTION _myplatform_cpu_entry
#define CONFIG_ARMV7_SECURE_BASE 0x0

That _myplatform_cpu_entry corresponds to your sunxi_cpu_entry code.

So, yeah, I know that isn't a proper patch and all. :-) I'm just sending you more information to ponder for this patch series! If you would like to generalize your patch this way, please feel free to do so. If not, I can send a proper patch after this hits mainline or so.

HTH,
jdl

Quoted 498 lines · Marc Zyngier

The current non-sec switching code suffers from one major issue:
it cannot run in secure RAM, as a large part of u-boot still needs to be run while we're switched to non-secure.

This patch reworks the whole HYP/non-secure strategy by:
- making sure the secure code is the *last* thing u-boot executes before entering the payload
- performing an exception return from secure mode directly into the payload
- allowing the code to be dynamically relocated to secure RAM before switching to non-secure.

This involves quite a bit of horrible code, specially as u-boot relocation is quite primitive.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 161 +++++++++++++++++++--------------------
 arch/arm/cpu/armv7/virt-v7.c     |  59 +++++---------
 arch/arm/include/asm/armv7.h     |  10 ++-
 arch/arm/include/asm/secure.h    |  26 +++++++
 arch/arm/lib/bootm.c             |  22 +++---
 5 files changed, 138 insertions(+), 140 deletions(-)
 create mode 100644 arch/arm/include/asm/secure.h
arch/arm/cpu/armv7/nonsec_virt.S +77 -84
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+       .pushsection ._secure.text, "ax"
+
        .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
        adr pc, _secure_monitor
        .word 0
        .word 0
-       adr pc, _hyp_trap
+       .word 0
        .word 0
        .word 0
 
+.macro is_cpu_virt_capable     tmp
+       mrc     p15, 0, \tmp, c0, c1, 1         @ read ID_PFR1
+       and     \tmp, \tmp, #CPUID_ARM_VIRT_MASK        @ mask virtualization bits
+       cmp     \tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm
+
 /*
  * secure monitor handler
  * U-boot calls this "software interrupt" in start.S
  * 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.
+ * r0, r1, r2: passed to the callee
+ * ip: target PC
  */
 _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
+       mrc     p15, 0, r5, c1, c1, 0           @ read SCR
+       bic     r5, r5, #0x4e                   @ clear IRQ, FIQ, EA, nET bits
+       orr     r5, r5, #0x31                   @ enable NS, AW, FW bits
 
-       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)
+       mov     r6, #SVC_MODE                   @ default mode is SVC
+       is_cpu_virt_capable r4
 #ifdef CONFIG_ARMV7_VIRT
-       orreq   r1, r1, #0x100                  @ allow HVC instruction
+       orreq   r5, r5, #0x100                  @ allow HVC instruction
+       moveq   r6, #HYP_MODE                   @ Enter the kernel as HYP
 #endif
 
-       mcr     p15, 0, r1, c1, c1, 0           @ write SCR (with NS bit set)
+       mcr     p15, 0, r5, c1, c1, 0           @ write SCR (with NS bit set)
        isb
 
-#ifdef CONFIG_ARMV7_VIRT
-       mrceq   p15, 0, r0, c12, c0, 1          @ get MVBAR value
-       mcreq   p15, 4, r0, c12, c0, 0          @ write HVBAR
-#endif
        bne     1f
 
        @ Reset CNTVOFF to 0 before leaving monitor mode
-       mrc     p15, 0, r0, c0, c1, 1           @ read ID_PFR1
-       ands    r0, r0, #CPUID_ARM_GENTIMER_MASK        @ test arch timer bits
-       movne   r0, #0
-       mcrrne  p15, 4, r0, r0, c14             @ Reset CNTVOFF to zero
+       mrc     p15, 0, r4, c0, c1, 1           @ read ID_PFR1
+       ands    r4, r4, #CPUID_ARM_GENTIMER_MASK        @ test arch timer bits
+       movne   r4, #0
+       mcrrne  p15, 4, r4, r4, c14             @ Reset CNTVOFF to zero
 1:
-       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
-
+       mov     lr, ip
+       mov     ip, #(F_BIT | I_BIT | A_BIT)    @ Set A, I and F
+       tst     lr, #1                          @ Check for Thumb PC
+       orrne   ip, ip, #T_BIT                  @ Set T if Thumb
+       orr     ip, ip, r6                      @ Slot target mode in
+       msr     spsr_cxfs, ip                   @ Set full SPSR
+       movs    pc, lr                          @ ERET to non-secure
+
+ENTRY(_do_nonsec_entry)
+       mov     ip, r0
+       mov     r0, r1
+       mov     r1, r2
+       mov     r2, r3
+       smc     #0
+ENDPROC(_do_nonsec_entry)
+
+.macro get_cbar_addr   addr
+#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
+       ldr     \addr, =CONFIG_ARM_GIC_BASE_ADDRESS
+#else
+       mrc     p15, 4, \addr, c15, c0, 0       @ read CBAR
+       bfc     \addr, #0, #15                  @ clear reserved bits
+#endif
+.endm
+
+.macro get_gicd_addr   addr
+       get_cbar_addr   \addr
+       add     \addr, \addr, #GIC_DIST_OFFSET  @ GIC dist i/f offset
+.endm
+
+.macro get_gicc_addr   addr, tmp
+       get_cbar_addr   \addr
+       is_cpu_virt_capable \tmp
+       movne   \tmp, #GIC_CPU_OFFSET_A9        @ GIC CPU offset for A9
+       moveq   \tmp, #GIC_CPU_OFFSET_A15       @ GIC CPU offset for A15/A7
+       add     \addr, \addr, \tmp
+.endm
+
+#ifndef CONFIG_ARMV7_PSCI
 /*
  * 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
@@ -74,31 +112,21 @@ _hyp_trap:
  * 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
+       cpsid   i
+       cpsid   f
 
        bl      _nonsec_init
-       mov     r12, r0                         @ save GICC address
-#ifdef CONFIG_ARMV7_VIRT
-       bl      _switch_to_hyp
-#endif
-
-       ldr     r1, [r12, #GICC_IAR]            @ acknowledge IPI
-       str     r1, [r12, #GICC_EOIR]           @ signal end of interrupt
 
        adr     r0, _smp_pen                    @ do not use this address again
        b       smp_waitloop                    @ wait for IPIs, board specific
 ENDPROC(_smp_pen)
+#endif
 
 /*
  * 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
@@ -108,38 +136,23 @@ ENDPROC(_smp_pen)
  * 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
-       bfc     r2, #0, #15                     @ clear reserved bits
-#endif
-       add     r3, r2, #GIC_DIST_OFFSET        @ GIC dist i/f offset
+       get_gicd_addr   r3
+
        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
+       get_gicc_addr   r3, r1
 
-       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]
+       mov     r1, #3                          @ Enable both groups
        str     r1, [r3, #GICC_CTLR]            @ and clear all other bits
        mov     r1, #0xff
        str     r1, [r3, #GICC_PMR]             @ set priority mask register
 
+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, 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
@@ -157,21 +170,9 @@ ENTRY(_nonsec_init)
 
        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)
 
@@ -183,18 +184,10 @@ ENTRY(smp_waitloop)
        ldr     r1, [r1]
        cmp     r0, r1                  @ make sure we dont execute this code
        beq     smp_waitloop            @ again (due to a spurious wakeup)
-       mov     pc, r1
+       mov     r0, r1
+       b       _do_nonsec_entry
 ENDPROC(smp_waitloop)
 .weak smp_waitloop
 #endif
 
-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)
+       .popsection
arch/arm/cpu/armv7/virt-v7.c +19 -40
index 2cd604f..6500030 100644
@@ -13,17 +13,10 @@
 #include <asm/armv7.h>
 #include <asm/gic.h>
 #include <asm/io.h>
+#include <asm/secure.h>
 
 unsigned long gic_dist_addr;
 
-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;
@@ -72,6 +65,18 @@ static unsigned long get_gicd_base_address(void)
 #endif
 }
 
+static void relocate_secure_section(void)
+{
+#ifdef CONFIG_ARMV7_SECURE_BASE
+       size_t sz = __secure_end - __secure_start;
+
+       memcpy((void *)CONFIG_ARMV7_SECURE_BASE, __secure_start, sz);
+       flush_dcache_range(CONFIG_ARMV7_SECURE_BASE,
+                          CONFIG_ARMV7_SECURE_BASE + sz + 1);
+       invalidate_icache_all();
+#endif
+}
+
 static void kick_secondary_cpus_gic(unsigned long gicdaddr)
 {
        /* kick all CPUs (except this one) by writing to GICD_SGIR */
@@ -83,35 +88,7 @@ void __weak smp_kick_all_cpus(void)
        kick_secondary_cpus_gic(gic_dist_addr);
 }
 
-int armv7_switch_hyp(void)
-{
-       unsigned int reg;
-
-       /* check whether we are in HYP mode already */
-       if ((read_cpsr() & 0x1f) == 0x1a) {
-               debug("CPU already in HYP mode\n");
-               return 0;
-       }
-
-       /* check whether the CPU supports the virtualization extensions */
-       reg = read_id_pfr1();
-       if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT) {
-               printf("HYP mode: Virtualization extensions not implemented.\n");
-               return -1;
-       }
-
-       /* call the HYP switching code on this CPU also */
-       _switch_to_hyp();
-
-       if ((read_cpsr() & 0x1F) != 0x1a) {
-               printf("HYP mode: switch not successful.\n");
-               return -1;
-       }
-
-       return 0;
-}
-
-int armv7_switch_nonsec(void)
+int armv7_init_nonsec(void)
 {
        unsigned int reg;
        unsigned itlinesnr, i;
@@ -147,11 +124,13 @@ int armv7_switch_nonsec(void)
        for (i = 1; i <= itlinesnr; i++)
                writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
 
-       smp_set_core_boot_addr((unsigned long)_smp_pen, -1);
+#ifndef CONFIG_ARMV7_PSCI
+       smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen), -1);
        smp_kick_all_cpus();
+#endif
 
        /* call the non-sec switching code on this CPU also */
-       _nonsec_init();
-
+       relocate_secure_section();
+       secure_ram_addr(_nonsec_init)();
        return 0;
 }
arch/arm/include/asm/armv7.h +7 -3
index 395444e..11476dd 100644
@@ -78,13 +78,17 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 
-int armv7_switch_nonsec(void);
-int armv7_switch_hyp(void);
+int armv7_init_nonsec(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
+void _do_nonsec_entry(void *target_pc, unsigned long r0,
+                     unsigned long r1, unsigned long r2);
 void _smp_pen(void);
-void _switch_to_hyp(void);
+
+extern char __secure_start[];
+extern char __secure_end[];
+
 #endif /* CONFIG_ARMV7_NONSEC || CONFIG_ARMV7_VIRT */
 
 #endif /* ! __ASSEMBLY__ */
arch/arm/include/asm/secure.h new file +26 -0
new file mode 100644
index 0000000..effdb18
@@ -0,0 +1,26 @@
+#ifndef __ASM_SECURE_H
+#define __ASM_SECURE_H
+
+#include <config.h>
+
+#ifdef CONFIG_ARMV7_SECURE_BASE
+/*
+ * Warning, horror ahead.
+ *
+ * The target code lives in our "secure ram", but u-boot doesn't know
+ * that, and has blindly added reloc_off to every relocation
+ * entry. Gahh. Do the opposite conversion. This hack also prevents
+ * GCC from generating code veeners, which u-boot doesn't relocate at
+ * all...
+ */
+#define secure_ram_addr(_fn) ({                                                \
+                       DECLARE_GLOBAL_DATA_PTR;                        \
+                       void *__fn = _fn;                               \
+                       typeof(_fn) *__tmp = (__fn - gd->reloc_off);    \
+                       __tmp;                                          \
+               })
+#else
+#define secure_ram_addr(_fn)   (_fn)
+#endif
+
+#endif
arch/arm/lib/bootm.c +9 -14
index 10634a4..61aa14e 100644
@@ -20,6 +20,7 @@
 #include <libfdt.h>
 #include <fdt_support.h>
 #include <asm/bootm.h>
+#include <asm/secure.h>
 #include <linux/compiler.h>
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
@@ -184,27 +185,17 @@ static void setup_end_tag(bd_t *bd)
 
 __weak void setup_board_tags(struct tag **in_params) {}
 
+#ifdef CONFIG_ARM64
 static void do_nonsec_virt_switch(void)
 {
-#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
-       if (armv7_switch_nonsec() == 0)
-#ifdef CONFIG_ARMV7_VIRT
-               if (armv7_switch_hyp() == 0)
-                       debug("entered HYP mode\n");
-#else
-               debug("entered non-secure state\n");
-#endif
-#endif
-
-#ifdef CONFIG_ARM64
        smp_kick_all_cpus();
        flush_dcache_all();     /* flush cache before swtiching to EL2 */
        armv8_switch_to_el2();
 #ifdef CONFIG_ARMV8_SWITCH_TO_EL1
        armv8_switch_to_el1();
 #endif
-#endif
 }
+#endif
 
 /* Subcommand: PREP */
 static void boot_prep_linux(bootm_headers_t *images)
@@ -287,8 +278,13 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
                r2 = gd->bd->bi_boot_params;
 
        if (!fake) {
-               do_nonsec_virt_switch();
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+               armv7_init_nonsec();
+               secure_ram_addr(_do_nonsec_entry)(kernel_entry,
+                                                 0, machid, r2);
+#else
                kernel_entry(0, machid, r2);
+#endif
        }
 #endif
 }
--

1.9.2

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#9 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

Quoted 14 lines · Jon Loeliger

Mark,

In your nonsec_init code, you suggest this change:

arch/arm/cpu/armv7/nonsec_virt.S +4 -2
+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

Leaving:

Quoted 7 lines
       mrc     p15, 0, r0, c1, c1, 2
       movw    r1, #0x3fff
       movt    r1, #0x0004
       orr     r0, r0, r1
       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

That sets all the co-processor bits, but the man page suggests that only

Just to be clear: which document are you referring to?

Quoted 1 line

copros with bits 10 and 11 should be modified. It also seems that if the

The ARM ARM says that NSACR[13:0] is either RAZ/WI or writable from secure for unimplemented coprocessors. So I believe the above is safe. If you wanted to be really picky, you'd start by reading CPACR, write either 1 or 3 to all the CPn fields, read it back again, see what sticks, and populate NSACR accordingly. Did I hear someone saying "Boring"? ;-)

Quoted 7 lines

PLE is enabled, we should mark it NS-enabled at bit 16 also:. Perhaps:

    mrc    p15, 0, r0, c1, c1, 2
    movw    r1, #0x0c00
    movt    r1, #0x0005
    orr    r0, r0, r1
    mcr    p15, 0, r0, c1, c1, 2        @ NSACR = all copros to non-sec

We're getting into IMPDEF territory pretty quickly here. PLE only exists on A9, and is optionnal there (and probably doesn't exist on all versions, if memory serves well...).

This could be implemented as a per-platform optional feature, though. What do you think?

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#10 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

Quoted 21 lines · Marc Zyngier
Quoted 20 lines · Jon Loeliger

Mark,

In your nonsec_init code, you suggest this change:

arch/arm/cpu/armv7/nonsec_virt.S +4 -2
+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

Leaving:

       mrc     p15, 0, r0, c1, c1, 2
       movw    r1, #0x3fff
       movt    r1, #0x0004
       orr     r0, r0, r1
       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec
Quoted 3 lines
Quoted 1 line

That sets all the co-processor bits, but the man page suggests that only

Just to be clear: which document are you referring to?

Hmm... Lessee.. Uh, this one:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388i/CIHEAIAJ.html

So, Cortex-A9 TRM 4.3.13 That one happens to be r4p1, but the description is the same for my part rev (r2p9, IIRC). Anyway, those low bits are marked as UNK/SBZP, hence my concern for the apparent extra ON bits.

Quoted 4 lines
Quoted 1 line

copros with bits 10 and 11 should be modified. It also seems that if the

The ARM ARM says that NSACR[13:0] is either RAZ/WI or writable from secure for unimplemented coprocessors.

The ARM ARM uber alles. :-)

Quoted 5 lines

So I believe the above is
safe. If you wanted to be really picky, you'd start by reading CPACR, write either 1 or 3 to all the CPn fields, read it back again, see what sticks, and populate NSACR accordingly. Did I hear someone saying "Boring"? ;-)

I'm sorry, did you say something? Sounded like you said "Waw-waw waw-wah CPn waw ..."

Quoted 11 lines
Quoted 7 lines

PLE is enabled, we should mark it NS-enabled at bit 16 also:. Perhaps:

    mrc    p15, 0, r0, c1, c1, 2
    movw    r1, #0x0c00
    movt    r1, #0x0005
    orr    r0, r0, r1
    mcr    p15, 0, r0, c1, c1, 2        @ NSACR = all copros to non-sec

We're getting into IMPDEF territory pretty quickly here. PLE only exists on A9, and is optionnal there (and probably doesn't exist on all versions, if memory serves well...).

Ah. Gotcha. Blah blah Osprey ah-chew! Gesundheit!

Quoted 2 lines

This could be implemented as a per-platform optional feature, though. What do you think?

I think we should all convert to A57 on a Dickens ring and be done.

In the meantime, it's likely not worth it to be this picky about the darn PLE bits, nor the rest of the NSACR bits. Especially if the ARM ARM says we can let it slide.

Quoted 3 lines
        M.

--
Without deviation from the norm, progress is not possible.

Yes, yes, Everybody got to deviate from the norm.

jdl

Quoted 52 lines · Marc Zyngier
Quoted 14 lines · Jon Loeliger

Mark,

In your nonsec_init code, you suggest this change:

arch/arm/cpu/armv7/nonsec_virt.S +4 -2
+       mrc     p15, 0, r0, c1, c1, 2
        movw    r1, #0x3fff
-       movt    r1, #0x0006
-       mcr     p15, 0, r1, c1, c1, 2           @ NSACR = all copros to non-sec
+       movt    r1, #0x0004
+       orr     r0, r0, r1
+       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

Leaving:

Quoted 7 lines
       mrc     p15, 0, r0, c1, c1, 2
       movw    r1, #0x3fff
       movt    r1, #0x0004
       orr     r0, r0, r1
       mcr     p15, 0, r0, c1, c1, 2           @ NSACR = all copros to non-sec

That sets all the co-processor bits, but the man page suggests that only

Just to be clear: which document are you referring to?

Quoted 1 line

copros with bits 10 and 11 should be modified. It also seems that if the

The ARM ARM says that NSACR[13:0] is either RAZ/WI or writable from secure for unimplemented coprocessors. So I believe the above is safe. If you wanted to be really picky, you'd start by reading CPACR, write either 1 or 3 to all the CPn fields, read it back again, see what sticks, and populate NSACR accordingly. Did I hear someone saying "Boring"? ;-)

Quoted 7 lines

PLE is enabled, we should mark it NS-enabled at bit 16 also:. Perhaps:

    mrc    p15, 0, r0, c1, c1, 2
    movw    r1, #0x0c00
    movt    r1, #0x0005
    orr    r0, r0, r1
    mcr    p15, 0, r0, c1, c1, 2        @ NSACR = all copros to non-sec

We're getting into IMPDEF territory pretty quickly here. PLE only exists on A9, and is optionnal there (and probably doesn't exist on all versions, if memory serves well...).

This could be implemented as a per-platform optional feature, though. What do you think?

        M.

--
Without deviation from the norm, progress is not possible.

#11 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

In reply to Jon Loeliger

On Fri, May 02 2014 at 9:30:05 pm BST, Jon Loeliger <[email protected]> wrote:

Hi Jon,

Quoted 1 line

I finally have all this working for me on an A9 system too!

Awesome! Ship it! ;-)

Quoted 74 lines

However, there were a few things that I had to change a bit. For example, by CPUs will always come out of reset at 0x0 and I do not have the ability to set their first-fetch address to anything else. To accommodate this, I need to ensure that the _monitor_vectors are loaded at address 0x0, and that the first entry in the exception vector (for reset) jumped to some notion of "secure_reset code". So I changed this code:

Quoted 23 lines
arch/arm/cpu/armv7/nonsec_virt.S +3 -0
index b5c946f..2a43e3c 100644
@@ -10,10 +10,13 @@
 #include <linux/linkage.h>
 #include <asm/gic.h>
 #include <asm/armv7.h>
+#include <asm/proc-armv/ptrace.h>
 
 .arch_extension sec
 .arch_extension virt
 
+       .pushsection ._secure.text, "ax"
+
       .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
@@ -22,51 +25,86 @@ _monitor_vectors:
       adr pc, _secure_monitor
       .word 0
       .word 0

arch/arm/cpu/armv7/nonsec_virt.S _monitor_vectors: · line 36

Quoted 10 lines
arch/arm/cpu/armv7/nonsec_virt.S +6 -1
-       adr pc, _hyp_trap
+       .word 0
       .word 0
        .word 0
 
+.macro is_cpu_virt_capable     tmp
+       mrc     p15, 0, \tmp, c0, c1, 1         @ read ID_PFR1
+       and     \tmp, \tmp, #CPUID_ARM_VIRT_MASK        @ mask virtualization bits
+       cmp     \tmp, #(1 << CPUID_ARM_VIRT_SHIFT)
+.endm

So that it did this too:

(quoted diff) +11 -3
@@ -20,15 +20,23 @@
        .align  5
 /* the vector table for secure state and HYP mode */
 _monitor_vectors:
-       .word 0 /* reset */
-       .word 0 /* undef */
-       adr pc, _secure_monitor
+       ldr pc, _secure_reset   /* reset */
+       .word 0                 /* undef */
+       adr pc, _secure_monitor /* SMC */
        .word 0
        .word 0
        .word 0
        .word 0
        .word 0
 
+
+_secure_reset:
+#ifdef CONFIG_SECURE_MONITOR_RESET_FUNCTION
+       .word CONFIG_SECURE_MONITOR_RESET_FUNCTION
+#else
+       .word 0
+#endif
+
 .macro is_cpu_virt_capable     tmp
 

That enabled me to define CONFIG_SECURE_MONITOR_RESET_FUNCTION in my config header file:

Quoted 8 lines

/*
* With the Secure Monitor at 0x0, its reset vector must also
* then point off to the correct "out-of-reset entry function." */
#define CONFIG_SECURE_MONITOR_RESET_FUNCTION _myplatform_cpu_entry
#define CONFIG_ARMV7_SECURE_BASE 0x0

That _myplatform_cpu_entry corresponds to your sunxi_cpu_entry code.

Yup, makes sense. Nit-pick: make the _secure_reset a weak symbol that your platform code will overload, just like the rest of the PSCI stuff. Saves the #ifdef horror; ;-)

Quoted 4 lines

So, yeah, I know that isn't a proper patch and all. :-) I'm just sending you more information to ponder for this patch series! If you would like to generalize your patch this way, please feel free to do so. If not, I can send a proper patch after this hits mainline or so.

My prefered way would be indeed to have a proper patch on top of this to handle the "coming out of reset" case. You'll get proper credit for the idea! :-)

Thanks,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#12 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

Quoted 7 lines
Quoted 7 lines

/*
* With the Secure Monitor at 0x0, its reset vector must also
* then point off to the correct "out-of-reset entry function." */
#define CONFIG_SECURE_MONITOR_RESET_FUNCTION _myplatform_cpu_entry
#define CONFIG_ARMV7_SECURE_BASE 0x0

Quoted 5 lines
Quoted 1 line

That _myplatform_cpu_entry corresponds to your sunxi_cpu_entry code.

Yup, makes sense. Nit-pick: make the _secure_reset a weak symbol that your platform code will overload, just like the rest of the PSCI stuff. Saves the #ifdef horror; ;-)

Oh, good idea. I'll add that bit in. Thanks!

Quoted 8 lines
Quoted 4 lines

So, yeah, I know that isn't a proper patch and all. :-) I'm just sending you more information to ponder for this patch series! If you would like to generalize your patch this way, please feel free to do so. If not, I can send a proper patch after this hits mainline or so.

My prefered way would be indeed to have a proper patch on top of this to handle the "coming out of reset" case. You'll get proper credit for the idea! :-)

Will do.

Thanks,
jdl

#13 lore raw

PATCH v4 6/10 ARM: HYP/non-sec: allow relocation to secure RAM

In reply to Marc Zyngier

Hi Marc,

Quoted 12 lines · Marc Zyngier

The current non-sec switching code suffers from one major issue:
it cannot run in secure RAM, as a large part of u-boot still needs to be run while we're switched to non-secure.

This patch reworks the whole HYP/non-secure strategy by:
- making sure the secure code is the *last* thing u-boot executes before entering the payload
- performing an exception return from secure mode directly into the payload
- allowing the code to be dynamically relocated to secure RAM before switching to non-secure.

Quoted 5 lines

This involves quite a bit of horrible code, specially as u-boot relocation is quite primitive.

---

This one causes a minor warning to appear when building aarch64 board vexpress_aemv8a:

/home/albert.u.boot/src/u-boot-arm/arch/arm/lib/bootm.c:189:13: warning: 'do_nonsec_virt_switch' defined but not used [-Wunused-function]
static void do_nonsec_virt_switch(void)

             ^ 

Can you look into removing this warning? Thanks in advance.

Amicalement,

Signature · 1 line
Albert.
#14 lore raw

PATCH v4 7/10 ARM: HYP/non-sec: add generic ARMv7 PSCI code

In reply to Marc Zyngier

Implement core support for PSCI. As this is generic code, it doesn't implement anything really useful (all the functions are returning Not Implemented).

---
 arch/arm/cpu/armv7/Makefile |   4 ++
 arch/arm/cpu/armv7/psci.S   | 102 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/psci.h |  35 +++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/psci.S
 create mode 100644 arch/arm/include/asm/psci.h
arch/arm/cpu/armv7/Makefile +4 -0
index 119ebb3..de1cc1a 100644
@@ -23,6 +23,10 @@ obj-y	+= nonsec_virt.o
 obj-y	+= virt-v7.o
 endif
 
+ifneq ($(CONFIG_ARMV7_PSCI),)
+obj-y	+= psci.o
+endif
+
 obj-$(CONFIG_KONA) += kona-common/
 obj-$(CONFIG_OMAP_COMMON) += omap-common/
 obj-$(CONFIG_TEGRA) += tegra-common/
arch/arm/cpu/armv7/psci.S new file +102 -0

102 lines not on this page · raw message

arch/arm/include/asm/psci.h new file +35 -0
new file mode 100644
index 0000000..704b4b0
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2013 - ARM Ltd
+ * Author: Marc Zyngier <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARM_PSCI_H__
+#define __ARM_PSCI_H__
+
+/* PSCI interface */
+#define ARM_PSCI_FN_BASE		0x95c1ba5e
+#define ARM_PSCI_FN(n)			(ARM_PSCI_FN_BASE + (n))
+
+#define ARM_PSCI_FN_CPU_SUSPEND		ARM_PSCI_FN(0)
+#define ARM_PSCI_FN_CPU_OFF		ARM_PSCI_FN(1)
+#define ARM_PSCI_FN_CPU_ON		ARM_PSCI_FN(2)
+#define ARM_PSCI_FN_MIGRATE		ARM_PSCI_FN(3)
+
+#define ARM_PSCI_RET_SUCCESS		0
+#define ARM_PSCI_RET_NI			(-1)
+#define ARM_PSCI_RET_INVAL		(-2)
+#define ARM_PSCI_RET_DENIED		(-3)
+
+#endif /* __ARM_PSCI_H__ */
Signature · 1 line
1.9.2
#15 lore raw

PATCH v4 8/10 ARM: HYP/non-sec: add the option for a second-stage monitor

In reply to Marc Zyngier

Allow the switch to a second stage secure monitor just before switching to non-secure.

This allows a resident piece of firmware to be active once the kernel has been entered (the u-boot monitor is dead anyway, its pages being reused).

---
 arch/arm/cpu/armv7/nonsec_virt.S | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
arch/arm/cpu/armv7/nonsec_virt.S +11 -2
index 2a43e3c..745670e 100644
@@ -44,10 +44,19 @@ _monitor_vectors:
  * ip: target PC
  */
 _secure_monitor:
+#ifdef CONFIG_ARMV7_PSCI
+	ldr	r5, =_psci_vectors		@ Switch to the next monitor
+	mcr	p15, 0, r5, c12, c0, 1
+	isb
+
+	@ Obtain a secure stack, and configure the PSCI backend
+	bl	psci_arch_init
+#endif
+
 	mrc	p15, 0, r5, c1, c1, 0		@ read SCR
-	bic	r5, r5, #0x4e			@ clear IRQ, FIQ, EA, nET bits
+	bic	r5, r5, #0x4a			@ clear IRQ, EA, nET bits
 	orr	r5, r5, #0x31			@ enable NS, AW, FW bits
-
+						@ FIQ preserved for secure mode
 	mov	r6, #SVC_MODE			@ default mode is SVC
 	is_cpu_virt_capable r4
 #ifdef CONFIG_ARMV7_VIRT
Signature · 1 line
1.9.2
#16 lore raw

PATCH v4 9/10 ARM: convert arch_fixup_memory_node to a generic FDT fixup function

In reply to Marc Zyngier

From: Ma Haijun <[email protected]>

Some architecture needs extra device tree setup. Instead of adding yet another hook, convert arch_fixup_memory_node to be a generic FDT fixup function.

[maz: collapsed 3 patches into one, rewrote commit message]

---
 arch/arm/lib/bootm-fdt.c | 2 +-
 arch/arm/lib/bootm.c     | 2 +-
 common/image-fdt.c       | 7 +++++--
 include/common.h         | 6 +++---
 4 files changed, 10 insertions(+), 7 deletions(-)
arch/arm/lib/bootm-fdt.c +1 -1
index e40691d..8394e15 100644
@@ -20,7 +20,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int arch_fixup_memory_node(void *blob)
+int arch_fixup_fdt(void *blob)
 {
 	bd_t *bd = gd->bd;
 	int bank;
arch/arm/lib/bootm.c +1 -1
index 61aa14e..4cff6b0 100644
@@ -357,7 +357,7 @@ void boot_prep_vxworks(bootm_headers_t *images)
 	if (images->ft_addr) {
 		off = fdt_path_offset(images->ft_addr, "/memory");
 		if (off < 0) {
-			if (arch_fixup_memory_node(images->ft_addr))
+			if (arch_fixup_fdt(images->ft_addr))
 				puts("## WARNING: fixup memory failed!\n");
 		}
 	}
common/image-fdt.c +5 -2
index a54a919..6f074de 100644
@@ -445,7 +445,7 @@ __weak int ft_verify_fdt(void *fdt)
 	return 1;
 }
 
-__weak int arch_fixup_memory_node(void *blob)
+__weak int arch_fixup_fdt(void *blob)
 {
 	return 0;
 }
@@ -462,7 +462,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
 		puts(" - must RESET the board to recover.\n");
 		return -1;
 	}
-	arch_fixup_memory_node(blob);
+	if (arch_fixup_fdt(blob) < 0) {
+		puts("ERROR: arch specific fdt fixup failed");
+		return -1;
+	}
 	if (IMAGE_OF_BOARD_SETUP)
 		ft_board_setup(blob, gd->bd);
 	fdt_fixup_ethernet(blob);
include/common.h +3 -3
index 5fb0bb2..3f50437 100644
@@ -326,14 +326,14 @@ int arch_early_init_r(void);
 void board_show_dram(ulong size);
 
 /**
- * arch_fixup_memory_node() - Write arch-specific memory information to fdt
+ * arch_fixup_fdt() - Write arch-specific information to fdt
  *
- * Defined in arch/$(ARCH)/lib/bootm.c
+ * Defined in arch/$(ARCH)/lib/bootm-fdt.c
  *
  * @blob:	FDT blob to write to
  * @return 0 if ok, or -ve FDT_ERR_... on failure
  */
-int arch_fixup_memory_node(void *blob);
+int arch_fixup_fdt(void *blob);
 
 /* common/flash.c */
 void flash_perror (int);
Signature · 1 line
1.9.2
#17 lore raw

PATCH v4 10/10 ARM: HYP/non-sec/PSCI: emit DT nodes

In reply to Marc Zyngier

Generate the PSCI node in the device tree.

Also add a reserve section for the "secure" code that lives in in normal RAM, so that the kernel knows it'd better not trip on it.

---
 arch/arm/cpu/armv7/Makefile  |   1 +
 arch/arm/cpu/armv7/virt-dt.c | 100 +++++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/armv7.h |   1 +
 arch/arm/lib/bootm-fdt.c     |  12 +++++-
 4 files changed, 112 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/virt-dt.c
arch/arm/cpu/armv7/Makefile +1 -0
index de1cc1a..44329cd 100644
@@ -21,6 +21,7 @@ endif
 ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),)
 obj-y	+= nonsec_virt.o
 obj-y	+= virt-v7.o
+obj-y	+= virt-dt.o
 endif
 
 ifneq ($(CONFIG_ARMV7_PSCI),)
arch/arm/cpu/armv7/virt-dt.c new file +100 -0

100 lines not on this page · raw message

arch/arm/include/asm/armv7.h +1 -0
index 11476dd..323f282 100644
@@ -79,6 +79,7 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
 
 int armv7_init_nonsec(void);
+int armv7_update_dt(void *fdt);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
arch/arm/lib/bootm-fdt.c +10 -2
index 8394e15..d4f1578 100644
@@ -17,13 +17,14 @@
 
 #include <common.h>
 #include <fdt_support.h>
+#include <asm/armv7.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 int arch_fixup_fdt(void *blob)
 {
 	bd_t *bd = gd->bd;
-	int bank;
+	int bank, ret;
 	u64 start[CONFIG_NR_DRAM_BANKS];
 	u64 size[CONFIG_NR_DRAM_BANKS];
 
@@ -32,5 +33,12 @@ int arch_fixup_fdt(void *blob)
 		size[bank] = bd->bi_dram[bank].size;
 	}
 
-	return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
+	ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+	if (ret)
+		return ret;
+
+	ret = armv7_update_dt(blob);
+#endif
+	return ret;
 }
Signature · 1 line
1.9.2
#18 lore raw

[U-Boot] DA850EVM with USE_NAND config does not pad the AIS file

I'm a U-Boot newbie so please feel free to correct how I'm reporting this issue..

I recently downloaded the 2014.04-rc3 snapshot to build U-Boot for my custom DA850-based board. The only change was to add a new target "dav850evm_nand" in boards.cfg with the added parameter "USE_NAND".

The resulting AIS file was programmed into EVM-compatible NAND using standard sfh_OMAP-L138 method.

The board failed to boot, and stayed in a loop printing the SPL console message repeatedly.

After some debugging with CCS 5.5 and an XDS100v2, I found that incorrect code was being loaded into the 0xc108000 RAM destination. The da850evm.h file defines CONFIG_SYS_NAND_U_BOOT_OFFS as 0x28000, which corresponds to an AIS offset of 0x8000 but the u-boot header did not appear there in the AIS file. A search revealed that the Makefile catenated u-boot immediately after the SPL without any padding.

Further investigation revealed that the target Makefile needs CONFIG_SPL_MAX_SIZE to be defined as 0x8000 in order for the padding to be performed properly; however, this constant was apparently deleted during a series of changes in April, 2013 to accommodate separate code and BSS size limits for another target. In its place,
CONFIG_SPL_MAX_FOOTPRINT was defined as 32768. Unfortunately, the
da850evm Makefile does not refer to this constant.

To solve the problem, I added the following 2 lines in my custom-modified da850evm.h:
#define CONFIG_SPL_PAD_TO 0x8000
#define CONFIG_SPL_MAX_SIZE 0x8000

although the first line may not be strictly required. This solved the problem and allowed the board to boot.

Doesn't this mean that other similar targets may be broken?

How can I assist with contributing the patch to fix this? One problem I have is that I only have an EXP, not an EVM board. My custom target board shares some features with the EVM such as NOR and NAND memory, but other things are different like the use of a fixed PHY. This makes it impossible for me to completely verify a da850evm build.

I would like to see more target configurations available for the DA850EVM but it seems like development for this has stopped. NAND boot was a simple change, but adding USB support required me to copy & paste code from the da830evm target. Are there any future plans to do this, or will I need to do this development for my custom board only?

Tom Taylor

#19 lore raw

[U-Boot] DA850EVM with USE_NAND config does not pad the AIS file

Quoted 30 lines · Tom Taylor

I'm a U-Boot newbie so please feel free to correct how I'm reporting this issue..

I recently downloaded the 2014.04-rc3 snapshot to build U-Boot for my custom DA850-based board. The only change was to add a new target "dav850evm_nand" in boards.cfg with the added parameter "USE_NAND".

The resulting AIS file was programmed into EVM-compatible NAND using standard sfh_OMAP-L138 method.

The board failed to boot, and stayed in a loop printing the SPL console message repeatedly.

After some debugging with CCS 5.5 and an XDS100v2, I found that incorrect code was being loaded into the 0xc108000 RAM destination. The da850evm.h file defines CONFIG_SYS_NAND_U_BOOT_OFFS as 0x28000, which corresponds to an AIS offset of 0x8000 but the u-boot header did not appear there in the AIS file. A search revealed that the Makefile catenated u-boot immediately after the SPL without any padding.

Further investigation revealed that the target Makefile needs CONFIG_SPL_MAX_SIZE to be defined as 0x8000 in order for the padding to be performed properly; however, this constant was apparently deleted during a series of changes in April, 2013 to accommodate separate code and BSS size limits for another target. In its place,
CONFIG_SPL_MAX_FOOTPRINT was defined as 32768. Unfortunately, the
da850evm Makefile does not refer to this constant.

Quoted 4 lines

To solve the problem, I added the following 2 lines in my custom-modified da850evm.h:
#define CONFIG_SPL_PAD_TO 0x8000
#define CONFIG_SPL_MAX_SIZE 0x8000

Thanks for looking into this. I saw a similar issue on my am18xx EVM but was 5 bugs deep and didn't have time to bisect it then.

I think the answer here is to post a patch fixing the boards listed in:

$ git grep -l ARM926EJS include/configs/ | xargs grep -l DAVINCI

As they all share that set of constraints.

Thanks for digging into this!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140505/1063bf1b/attachment.pgp>
#20 lore raw

PATCH v4 10/10 ARM: HYP/non-sec/PSCI: emit DT nodes

In reply to Marc Zyngier
Quoted 60 lines · Marc Zyngier
arch/arm/cpu/armv7/virt-dt.c new file +53 -0
new file mode 100644
index 0000000..0b0d6a7
 
+
+static int fdt_psci(void *fdt)
+{
+#ifdef CONFIG_ARMV7_PSCI
+       int nodeoff;
+       int tmp;
+
+       nodeoff = fdt_path_offset(fdt, "/cpus");
+       if (nodeoff < 0) {
+               printf("couldn't find /cpus\n");
+               return nodeoff;
+       }
+
+       /* add 'enable-method = "psci"' to each cpu node */
+       for (tmp = fdt_first_subnode(fdt, nodeoff);
+            tmp >= 0;
+            tmp = fdt_next_subnode(fdt, tmp)) {
+               const struct fdt_property *prop;
+               int len;
+
+               prop = fdt_get_property(fdt, tmp, "device_type", &len);
+               if (!prop)
+                       continue;
+               if (len < 4)
+                       continue;
+               if (strcmp(prop->data, "cpu"))
+                       continue;
+
+               fdt_setprop_string(fdt, tmp, "enable-method", "psci");
+       }
+
+       nodeoff = fdt_path_offset(fdt, "/psci");
+       if (nodeoff < 0) {
+               nodeoff = fdt_path_offset(fdt, "/");
+               if (nodeoff < 0)
+                       return nodeoff;
+
+               nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
+               if (nodeoff < 0)
+                       return nodeoff;
+       }
+
+       tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci");
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_suspend", ARM_PSCI_FN_CPU_SUSPEND);
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_off", ARM_PSCI_FN_CPU_OFF);
+       if (tmp)

arch/arm/cpu/armv7/virt-dt.c line 89

Quoted 10 lines
arch/arm/cpu/armv7/virt-dt.c new file +10 -0
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_on", ARM_PSCI_FN_CPU_ON);
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "migrate", ARM_PSCI_FN_MIGRATE);
+       if (tmp)
+               return tmp;
+#endif
+       return 0;
+}

So, I wonder if it would be better to be a bit more selective or cautious about adding these nodes and properties. Specifically, if they are already present in the device tree itself, perhaps they should be honored and left alone?

I understand that U-Boot gets to define what it implements, and that if the secure monitor code doesn't actually implement something, or for that matter *does* implement it, it makes sense for U-Boot to be able to state those facts in a device tree. However, the DTS may also be stating what it has implemented or willing to honor on the Linux side as well. So, yeah, there has to be agreement here.

But who gets to make the final adjustment to the device tree? U-boot with this code, or the DTS author who may have hand coded specific wishes and loaded a specific device tree?

HTH,
jdl

#21 lore raw

PATCH v4 10/10 ARM: HYP/non-sec/PSCI: emit DT nodes

Quoted 65 lines · Jon Loeliger
Quoted 63 lines · Marc Zyngier
arch/arm/cpu/armv7/virt-dt.c new file +49 -0
new file mode 100644
index 0000000..0b0d6a7
 
+
+static int fdt_psci(void *fdt)
+{
+#ifdef CONFIG_ARMV7_PSCI
+       int nodeoff;
+       int tmp;
+
+       nodeoff = fdt_path_offset(fdt, "/cpus");
+       if (nodeoff < 0) {
+               printf("couldn't find /cpus\n");
+               return nodeoff;
+       }
+
+       /* add 'enable-method = "psci"' to each cpu node */
+       for (tmp = fdt_first_subnode(fdt, nodeoff);
+            tmp >= 0;
+            tmp = fdt_next_subnode(fdt, tmp)) {
+               const struct fdt_property *prop;
+               int len;
+
+               prop = fdt_get_property(fdt, tmp, "device_type", &len);
+               if (!prop)
+                       continue;
+               if (len < 4)
+                       continue;
+               if (strcmp(prop->data, "cpu"))
+                       continue;
+
+               fdt_setprop_string(fdt, tmp, "enable-method", "psci");
+       }
+
+       nodeoff = fdt_path_offset(fdt, "/psci");
+       if (nodeoff < 0) {
+               nodeoff = fdt_path_offset(fdt, "/");
+               if (nodeoff < 0)
+                       return nodeoff;
+
+               nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
+               if (nodeoff < 0)
+                       return nodeoff;
+       }
+
+       tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci");
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
+       if (tmp)
+               return tmp;
+ tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_suspend",

ARM_PSCI_FN_CPU_SUSPEND);

arch/arm/cpu/armv7/virt-dt.c new file +7 -0
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_off", ARM_PSCI_FN_CPU_OFF);
+       if (tmp)
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_on", ARM_PSCI_FN_CPU_ON);
+       if (tmp)
Quoted 8 lines

arch/arm/cpu/armv7/virt-dt.c line 89

Quoted 8 lines
arch/arm/cpu/armv7/virt-dt.c new file +7 -0
+               return tmp;
+       tmp = fdt_setprop_u32(fdt, nodeoff, "migrate", ARM_PSCI_FN_MIGRATE);
+       if (tmp)
+               return tmp;
+#endif
+       return 0;
+}

Hi Jon,

Quoted 4 lines

So, I wonder if it would be better to be a bit more selective or cautious about adding these nodes and properties. Specifically, if they are already present in the device tree itself, perhaps they should be honored and left alone?

Well, we have exactly two possibilities:
- PSCI is provided by the platform's firmware, and we'd better not touch the DT at all.
- PSCI is provided U-Boot, and we *own* the PSCI related nodes.

I don't think there is an alternative to that, because either U-Boot or the firmware will install its own secure vectors. The DT manipulation code just reflect this situation.

Quoted 10 lines

I understand that U-Boot gets to define what it implements, and that if the secure monitor code doesn't actually implement something, or for that matter *does* implement it, it makes sense for U-Boot to be able to state those facts in a device tree. However, the DTS may also be stating what it has implemented or willing to honor on the Linux side as well. So, yeah, there has to be agreement here.

But who gets to make the final adjustment to the device tree? U-boot with this code, or the DTS author who may have hand coded specific wishes and loaded a specific device tree?

We could have an environment variable named "i_know_this_looks_silly_but_nevertheless_please_pretty_please_leave_my_cpu_nodes_alone"?

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#22 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

In reply to Marc Zyngier
Quoted 117 lines · On Sat, 26 Apr 2014 13:17:01 +0100 Marc Zyngier <[email protected]> wrote:

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code that just calls into PSCI.

More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.

The main idea here is to turn some of the existing U-Boot code into a separate section that can live in secure RAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when U-Boot is long gone, hence the need for a piece of memory that will not be touched by the OS.

This patch series contains 3 parts:
- the first four patches are just bug fixes
- the next two refactor the HYP/non-secure code to allow relocation in secure memory
- the last four contain the generic PSCI code and DT infrastructure

This implements the original 0.1 spec, as nobody implements the new 0.2 version so far. I plan to update this support to 0.2 once there is an official binding available (and support in the kernel).

Most of the development has been done on an Allwinner A20 SoC, which is the main user of this code at the moment. I hope new SoCs will be using this method in the future (my primary goal for this series being to avoid more stupid SMP code from creeping up in the Linux kernel). As instructed, I've removed the A20 support code and made it a separate series, as there is now an effort to mainline this code (see Ian Campbell patch series).

With these three series applied, the A20 now boots in HYP mode, Linux finds the secondary CPU without any SMP code present in the kernel, and runs KVM out of the box. The Xen/ARM guys managed to do the same fairly easily, as did at least one XVizor user.

This code has also been tested on a VExpress TC2, running KVM with all 5 CPUs, in order to make sure there was no obvious regression.

The code is also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/u-boot.git
wip/psci-v4

A fully merged branch with the A20 support is in the wip/psci-v4-a20 branch of the same repo.

Cheers,

        M.

From v3:
- Return ARM_PSCI_RET_INVAL instead of ARM_PSCI_RET_NI when PSCI is entered with an invalid function code.
- Fix !NONSEC compilation
- Rebased on top of adcdeac

From v2:
- Dropped the secure stack allocation from the generic PSCI code. There was too little space there for it to be really useful, and the arch code knows a lot better about its requirements anyway. It is now the responsibility of the arch code to provide a stack. This allows it to get rid of the silly game with the thread registers that was confusing everyone...
- Added provision for FIQ handling in secure mode. Allwinner A20 is going to require this for CPU_OFF.
- Better integration of the FDT injection code with the rest of the code, fixing the truncated FDT issue that people have been reporting (courtesy of Ma Haijun).
- Cleanup of the AW-specific code (stack allocation, timer macro).
- Rebased on mainline U-Boot (on top of 22a240c32c13).

From v1:
- Complete rewrite, now directly relocating the secure code withing U-Boot, instead of having a separate psci blob.

Ma Haijun (1):
ARM: convert arch_fixup_memory_node to a generic FDT fixup function

Marc Zyngier (9):
ARM: HYP/non-sec: move switch to non-sec to the last boot phase
ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
ARM: non-sec: reset CNTVOFF to zero
ARM: add missing HYP mode constant
ARM: HYP/non-sec: add separate section for secure code
ARM: HYP/non-sec: allow relocation to secure RAM
ARM: HYP/non-sec: add generic ARMv7 PSCI code
ARM: HYP/non-sec: add the option for a second-stage monitor
ARM: HYP/non-sec/PSCI: emit DT nodes

 arch/arm/config.mk                      |   2 +-
 arch/arm/cpu/armv7/Makefile             |   5 +
 arch/arm/cpu/armv7/nonsec_virt.S        | 168
+++++++++++++++++---------------
arch/arm/cpu/armv7/psci.S               | 102 +++++++++++++++++++
arch/arm/cpu/armv7/virt-dt.c            | 100 +++++++++++++++++++
arch/arm/cpu/armv7/virt-v7.c            |  59 ++++-------
arch/arm/cpu/u-boot.lds                 |  30 ++++++
arch/arm/include/asm/armv7.h            |  11 ++-
arch/arm/include/asm/proc-armv/ptrace.h |   2 +
arch/arm/include/asm/psci.h             |  35 +++++++
arch/arm/include/asm/secure.h           |  26 +++++
arch/arm/lib/bootm-fdt.c                |  14 ++-
arch/arm/lib/bootm.c                    |  27 +++--
arch/arm/lib/interrupts.c               |   2 +-
arch/arm/lib/sections.c                 |   2 +
common/image-fdt.c                      |   7 +-
include/common.h                        |   6 +- 17 files changed,
Quoted 6 lines

451 insertions(+), 147 deletions(-) create mode 100644 arch/arm/cpu/armv7/psci.S create mode 100644 arch/arm/cpu/armv7/virt-dt.c create mode 100644 arch/arm/include/asm/psci.h create mode 100644 arch/arm/include/asm/secure.h

this series fails to compile for me
CC spl/arch/arm/cpu/armv7/virt-v7.o
arch/arm/cpu/armv7/virt-v7.c: In function ?armv7_init_nonsec?:
arch/arm/cpu/armv7/virt-v7.c:128:41: error: ?_smp_pen? undeclared (first use in this function)

  smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen), -1);
                                         ^

arch/arm/cpu/armv7/virt-v7.c:128:41: note: each undeclared identifier is reported only once for each function it appears in arch/arm/cpu/armv7/virt-v7.c:134:3: error: ?_nonsec_init? undeclared (first use in this function)
secure_ram_addr(_nonsec_init)();
^

is there a series I am missing?

Dennis

#23 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Quoted 7 lines · Dennis Gilmore
Quoted 4 lines · On Sat, 26 Apr 2014 13:17:01 +0100 Marc Zyngier <[email protected]> wrote:

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management

[...]

Quoted 9 lines

this series fails to compile for me
CC spl/arch/arm/cpu/armv7/virt-v7.o
arch/arm/cpu/armv7/virt-v7.c: In function ?armv7_init_nonsec?:
arch/arm/cpu/armv7/virt-v7.c:128:41: error: ?_smp_pen? undeclared (first use in this function)
smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen),

-1);
                                         ^
Quoted 8 lines

arch/arm/cpu/armv7/virt-v7.c:128:41: note: each undeclared identifier is reported only once for each function it appears in arch/arm/cpu/armv7/virt-v7.c:134:3: error: ?_nonsec_init? undeclared (first use in this function)

  secure_ram_addr(_nonsec_init)();
   ^

is there a series I am missing?

Interesting. What are you compiling for? Having the non-virt part built in the SPL is unexpected though..

Thanks,

         M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#24 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Quoted 27 lines · On Sat, 26 Apr 2014 17:31:03 +0100 Marc Zyngier <[email protected]> wrote:
Quoted 7 lines · Dennis Gilmore
Quoted 4 lines · On Sat, 26 Apr 2014 13:17:01 +0100 Marc Zyngier <[email protected]> wrote:

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management

[...]

Quoted 16 lines

this series fails to compile for me
CC spl/arch/arm/cpu/armv7/virt-v7.o
arch/arm/cpu/armv7/virt-v7.c: In function ?armv7_init_nonsec?:
arch/arm/cpu/armv7/virt-v7.c:128:41: error: ?_smp_pen? undeclared (first use in this function)
smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen),

-1);
                                         ^

arch/arm/cpu/armv7/virt-v7.c:128:41: note: each undeclared identifier is reported only once for each function it appears in arch/arm/cpu/armv7/virt-v7.c:134:3: error: ?_nonsec_init? undeclared (first use in this function)

  secure_ram_addr(_nonsec_init)();
   ^
Quoted 4 lines
Quoted 1 line

is there a series I am missing?

Interesting. What are you compiling for? Having the non-virt part built in the SPL is unexpected though..

Cubietruck trying to pull in the patches so we can support it in Fedora, ended up being user error. I did a make mrproper and reconfigured then it built fine.

Dennis

#25 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

In reply to Marc Zyngier

Mark,

Not sure on which patch of your series to base the following comment, but thought it was worth bringing up for discussion nevertheless.

So, over in U-Boot ARMv7's arch/arm/cpu/armv7/start.S code, there is a bit of code under the enty-point named cpu_init_cp15 that does some I-Cache/MMU/state and Errata fi-xup for the boot CPU. As it should. That's all splendid.

However, any secondary CPU coming out of reset *after* the U-Boot is gone and only the secure monitor remains will still need to execute code that is substantially similar if not exactly the same.

That means the same code block (that cpu_init_cp15 code) needs to be assembled into the original out-of-reset sequence for the boot CPU, and also needs to be assembled into the secure monitor code for secondary CPU out-of-reset sequences.

We clearly can not directly refactor that function, extracting it and reusing it wiith the same entry-point as it needs to be linked into two different sections. We could put it in some header file and #include it into both places. We could define a macro and instantiate it in each place.
We could do some hacky assemble it twice with different names thing.

Thoughts?

Thanks,
jdl

Quoted 128 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code that just calls into PSCI.

More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.

The main idea here is to turn some of the existing U-Boot code into a separate section that can live in secure RAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when U-Boot is long gone, hence the need for a piece of memory that will not be touched by the OS.

This patch series contains 3 parts:
- the first four patches are just bug fixes
- the next two refactor the HYP/non-secure code to allow relocation in secure memory
- the last four contain the generic PSCI code and DT infrastructure

This implements the original 0.1 spec, as nobody implements the new 0.2 version so far. I plan to update this support to 0.2 once there is an official binding available (and support in the kernel).

Most of the development has been done on an Allwinner A20 SoC, which is the main user of this code at the moment. I hope new SoCs will be using this method in the future (my primary goal for this series being to avoid more stupid SMP code from creeping up in the Linux kernel). As instructed, I've removed the A20 support code and made it a separate series, as there is now an effort to mainline this code (see Ian Campbell patch series).

With these three series applied, the A20 now boots in HYP mode, Linux finds the secondary CPU without any SMP code present in the kernel, and runs KVM out of the box. The Xen/ARM guys managed to do the same fairly easily, as did at least one XVizor user.

This code has also been tested on a VExpress TC2, running KVM with all 5 CPUs, in order to make sure there was no obvious regression.

The code is also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/u-boot.git wip/psci-v4

A fully merged branch with the A20 support is in the wip/psci-v4-a20 branch of the same repo.

Cheers,

        M.

From v3:
- Return ARM_PSCI_RET_INVAL instead of ARM_PSCI_RET_NI when PSCI is entered with an invalid function code.
- Fix !NONSEC compilation
- Rebased on top of adcdeac

From v2:
- Dropped the secure stack allocation from the generic PSCI code. There was too little space there for it to be really useful, and the arch code knows a lot better about its requirements anyway. It is now the responsibility of the arch code to provide a stack. This allows it to get rid of the silly game with the thread registers that was confusing everyone...
- Added provision for FIQ handling in secure mode. Allwinner A20 is going to require this for CPU_OFF.
- Better integration of the FDT injection code with the rest of the code, fixing the truncated FDT issue that people have been reporting (courtesy of Ma Haijun).
- Cleanup of the AW-specific code (stack allocation, timer macro).
- Rebased on mainline U-Boot (on top of 22a240c32c13).

From v1:
- Complete rewrite, now directly relocating the secure code withing U-Boot, instead of having a separate psci blob.

Ma Haijun (1):
ARM: convert arch_fixup_memory_node to a generic FDT fixup function

Marc Zyngier (9):
ARM: HYP/non-sec: move switch to non-sec to the last boot phase
ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
ARM: non-sec: reset CNTVOFF to zero
ARM: add missing HYP mode constant
ARM: HYP/non-sec: add separate section for secure code
ARM: HYP/non-sec: allow relocation to secure RAM
ARM: HYP/non-sec: add generic ARMv7 PSCI code
ARM: HYP/non-sec: add the option for a second-stage monitor
ARM: HYP/non-sec/PSCI: emit DT nodes

 arch/arm/config.mk                      |   2 +-
 arch/arm/cpu/armv7/Makefile             |   5 +
 arch/arm/cpu/armv7/nonsec_virt.S        | 168 +++++++++++++++++---------------
 arch/arm/cpu/armv7/psci.S               | 102 +++++++++++++++++++
 arch/arm/cpu/armv7/virt-dt.c            | 100 +++++++++++++++++++
 arch/arm/cpu/armv7/virt-v7.c            |  59 ++++-------
 arch/arm/cpu/u-boot.lds                 |  30 ++++++
 arch/arm/include/asm/armv7.h            |  11 ++-
 arch/arm/include/asm/proc-armv/ptrace.h |   2 +
 arch/arm/include/asm/psci.h             |  35 +++++++
 arch/arm/include/asm/secure.h           |  26 +++++
 arch/arm/lib/bootm-fdt.c                |  14 ++-
 arch/arm/lib/bootm.c                    |  27 +++--
 arch/arm/lib/interrupts.c               |   2 +-
 arch/arm/lib/sections.c                 |   2 +
 common/image-fdt.c                      |   7 +-
 include/common.h                        |   6 +-
 17 files changed, 451 insertions(+), 147 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/psci.S
 create mode 100644 arch/arm/cpu/armv7/virt-dt.c
 create mode 100644 arch/arm/include/asm/psci.h
 create mode 100644 arch/arm/include/asm/secure.h

--
1.9.2

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#26 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Quoted 19 lines · Jon Loeliger

Mark,

Not sure on which patch of your series to base the following comment, but thought it was worth bringing up for discussion nevertheless.

So, over in U-Boot ARMv7's arch/arm/cpu/armv7/start.S code, there is a bit of code under the enty-point named cpu_init_cp15 that does some I-Cache/MMU/state and Errata fi-xup for the boot CPU. As it should. That's all splendid.

However, any secondary CPU coming out of reset *after* the U-Boot is gone and only the secure monitor remains will still need to execute code that is substantially similar if not exactly the same.

That means the same code block (that cpu_init_cp15 code) needs to be assembled into the original out-of-reset sequence for the boot CPU, and also needs to be assembled into the secure monitor code for secondary CPU out-of-reset sequences.

Quoted 8 lines

We clearly can not directly refactor that function, extracting it and reusing it wiith the same entry-point as it needs to be linked into two different sections. We could put it in some header file and #include it into both places. We could define a macro and instantiate it in each place. We could do some hacky assemble it twice with different names thing.

Thoughts?

Yes, that makes perfect sense. Can we put this into a separate object file and play some link-time hackery instead? Otherwise, the include thingy seems fine.

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#27 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

In reply to Marc Zyngier

Hi Marc,

Quoted 7 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

Do the discussions on 06/10 and 10/10 mean there will be a v5 of this series, or can/should I apply v4 as-is (and expect later changes in the form of later patches) ?

Amicalement,

Signature · 1 line
Albert.
#28 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Quoted 13 lines · Albert ARIBAUD

Hi Marc,

Quoted 7 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

Quoted 3 lines

Do the discussions on 06/10 and 10/10 mean there will be a v5 of this series, or can/should I apply v4 as-is (and expect later changes in the form of later patches) ?

Ping.

Amicalement,

Signature · 1 line
Albert.
#29 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Quoted 16 lines · Albert ARIBAUD
Quoted 13 lines · Albert ARIBAUD

Hi Marc,

Quoted 7 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

Quoted 5 lines
Quoted 3 lines

Do the discussions on 06/10 and 10/10 mean there will be a v5 of this series, or can/should I apply v4 as-is (and expect later changes in the form of later patches) ?

Ping.

Arghhh. Drowning under the load, sorry about the delay.

I don't expect much changes in this series, as it is good enough a base for future developments (PSCI 0.2, among other things).

If the series doesn't apply directly, I'll rebase it onto HEAD.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#30 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

Hi Marc,

Quoted 17 lines · Marc Zyngier
Quoted 16 lines · Albert ARIBAUD
Quoted 13 lines · Albert ARIBAUD

Hi Marc,

Quoted 7 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Do the discussions on 06/10 and 10/10 mean there will be a v5 of this series, or can/should I apply v4 as-is (and expect later changes in the form of later patches) ?

Ping.

Arghhh. Drowning under the load, sorry about the delay.

No problem.

Quoted 4 lines

I don't expect much changes in this series, as it is good enough a base for future developments (PSCI 0.2, among other things).

If the series doesn't apply directly, I'll rebase it onto HEAD.

It did apply using pwclient git-am, except for 07/10 which required a pwclient get + git am -3. Building and testing underway.

Quoted 3 lines

Thanks,

	M.

Amicalement,

Signature · 1 line
Albert.
#31 lore raw

v4 cover ARMv7: add PSCI support to U-Boot

In reply to Marc Zyngier

Hi Marc,

Quoted 112 lines · Marc Zyngier

PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset

It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code that just calls into PSCI.

More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.

The main idea here is to turn some of the existing U-Boot code into a separate section that can live in secure RAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when U-Boot is long gone, hence the need for a piece of memory that will not be touched by the OS.

This patch series contains 3 parts:
- the first four patches are just bug fixes
- the next two refactor the HYP/non-secure code to allow relocation in secure memory
- the last four contain the generic PSCI code and DT infrastructure

This implements the original 0.1 spec, as nobody implements the new 0.2 version so far. I plan to update this support to 0.2 once there is an official binding available (and support in the kernel).

Most of the development has been done on an Allwinner A20 SoC, which is the main user of this code at the moment. I hope new SoCs will be using this method in the future (my primary goal for this series being to avoid more stupid SMP code from creeping up in the Linux kernel). As instructed, I've removed the A20 support code and made it a separate series, as there is now an effort to mainline this code (see Ian Campbell patch series).

With these three series applied, the A20 now boots in HYP mode, Linux finds the secondary CPU without any SMP code present in the kernel, and runs KVM out of the box. The Xen/ARM guys managed to do the same fairly easily, as did at least one XVizor user.

This code has also been tested on a VExpress TC2, running KVM with all 5 CPUs, in order to make sure there was no obvious regression.

The code is also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/u-boot.git wip/psci-v4

A fully merged branch with the A20 support is in the wip/psci-v4-a20 branch of the same repo.

Cheers,

        M.
Quoted 1 line

From v3:

- Return ARM_PSCI_RET_INVAL instead of ARM_PSCI_RET_NI when PSCI is entered with an invalid function code.
- Fix !NONSEC compilation
- Rebased on top of adcdeac

Quoted 1 line

From v2:

- Dropped the secure stack allocation from the generic PSCI code. There was too little space there for it to be really useful, and the arch code knows a lot better about its requirements anyway. It is now the responsibility of the arch code to provide a stack. This allows it to get rid of the silly game with the thread registers that was confusing everyone...
- Added provision for FIQ handling in secure mode. Allwinner A20 is going to require this for CPU_OFF.
- Better integration of the FDT injection code with the rest of the code, fixing the truncated FDT issue that people have been reporting (courtesy of Ma Haijun).
- Cleanup of the AW-specific code (stack allocation, timer macro).
- Rebased on mainline U-Boot (on top of 22a240c32c13).

Quoted 1 line

From v1:

- Complete rewrite, now directly relocating the secure code withing U-Boot, instead of having a separate psci blob.

Ma Haijun (1):
ARM: convert arch_fixup_memory_node to a generic FDT fixup function

Marc Zyngier (9):
ARM: HYP/non-sec: move switch to non-sec to the last boot phase
ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
ARM: non-sec: reset CNTVOFF to zero
ARM: add missing HYP mode constant
ARM: HYP/non-sec: add separate section for secure code
ARM: HYP/non-sec: allow relocation to secure RAM
ARM: HYP/non-sec: add generic ARMv7 PSCI code
ARM: HYP/non-sec: add the option for a second-stage monitor
ARM: HYP/non-sec/PSCI: emit DT nodes

 arch/arm/config.mk                      |   2 +-
 arch/arm/cpu/armv7/Makefile             |   5 +
 arch/arm/cpu/armv7/nonsec_virt.S        | 168 +++++++++++++++++---------------
 arch/arm/cpu/armv7/psci.S               | 102 +++++++++++++++++++
 arch/arm/cpu/armv7/virt-dt.c            | 100 +++++++++++++++++++
 arch/arm/cpu/armv7/virt-v7.c            |  59 ++++-------
 arch/arm/cpu/u-boot.lds                 |  30 ++++++
 arch/arm/include/asm/armv7.h            |  11 ++-
 arch/arm/include/asm/proc-armv/ptrace.h |   2 +
 arch/arm/include/asm/psci.h             |  35 +++++++
 arch/arm/include/asm/secure.h           |  26 +++++
 arch/arm/lib/bootm-fdt.c                |  14 ++-
 arch/arm/lib/bootm.c                    |  27 +++--
 arch/arm/lib/interrupts.c               |   2 +-
Quoted 9 lines
 arch/arm/lib/sections.c                 |   2 +
 common/image-fdt.c                      |   7 +-
 include/common.h                        |   6 +-
 17 files changed, 451 insertions(+), 147 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/psci.S
 create mode 100644 arch/arm/cpu/armv7/virt-dt.c
 create mode 100644 arch/arm/include/asm/psci.h
 create mode 100644 arch/arm/include/asm/secure.h

Applied to u-boot-arm/master, thanks!

Amicalement,

Signature · 1 line
Albert.
#32 lore raw

v4 cover ARMv7: add PSCI support to U-Boot - *not* applied

Quoted 3 lines · Albert ARIBAUD

Hi Marc,

Applied to u-boot-arm/master, thanks!

Apologies -- *not* applied until 08/10 warning is fixed.

Amicalement,

Signature · 1 line
Albert.
#33 lore raw

v4 cover ARMv7: add PSCI support to U-Boot - *not* applied

Quoted 8 lines · Albert ARIBAUD
Quoted 3 lines · Albert ARIBAUD

Hi Marc,

Applied to u-boot-arm/master, thanks!

Apologies -- *not* applied until 08/10 warning is fixed.

I meant 06/10, not 08/10.

Amicalement,

Signature · 1 line
Albert.