[U-Boot] [PATCH] ARM: vexpress: Remove virt and nonsec support for TC2
When CPU's come out of reset they are in secure state supervisor mode, so this is the state Linux kernel entry point is called in when it brings up secondary CPU cores or the primary CPU restarts after power management has sent it through an off/on transition.
As U-Boot starts the kernel in hypervisor mode and the kernel expects and checks that CPUs start in the same state as initial boot this results in a dead system. Specifically, it crashes early in boot when the primary CPU runs the MCPM test [1] and even if power management features are disabled it will still refuse to bring up any secondary CPUs.
Fix this problem by removing U-Boot support for virt and nonsec support on TC2.
[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=35...
Signed-off-by: Jon Medhurst tixy@linaro.org --- arch/arm/Kconfig | 2 -- board/armltd/vexpress/vexpress_common.c | 15 --------------- 2 files changed, 17 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e9d2fc9..2e48568 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -293,8 +293,6 @@ config ARCH_BCM283X config TARGET_VEXPRESS_CA15_TC2 bool "Support vexpress_ca15_tc2" select CPU_V7 - select CPU_V7_HAS_NONSEC - select CPU_V7_HAS_VIRT
config TARGET_VEXPRESS_CA5X2 bool "Support vexpress_ca5x2" diff --git a/board/armltd/vexpress/vexpress_common.c b/board/armltd/vexpress/vexpress_common.c index d3b3b31..fe5d163 100644 --- a/board/armltd/vexpress/vexpress_common.c +++ b/board/armltd/vexpress/vexpress_common.c @@ -180,18 +180,3 @@ void lowlevel_init(void) ulong get_board_rev(void){ return readl((u32 *)SYS_ID); } - -#ifdef CONFIG_ARMV7_NONSEC -/* Setting the address at which secondary cores start from. - * Versatile Express uses one address for all cores, so ignore corenr - */ -void smp_set_core_boot_addr(unsigned long addr, int corenr) -{ - /* The SYSFLAGS register on VExpress needs to be cleared first - * by writing to the next address, since any writes to the address - * at offset 0 will only be ORed in - */ - writel(~0, CONFIG_SYSFLAGS_ADDR + 4); - writel(addr, CONFIG_SYSFLAGS_ADDR); -} -#endif
Hi,
On 22/06/16 15:34, Jon Medhurst (Tixy) wrote:
When CPU's come out of reset they are in secure state supervisor mode, so this is the state Linux kernel entry point is called in when it brings up secondary CPU cores or the primary CPU restarts after power management has sent it through an off/on transition.
As U-Boot starts the kernel in hypervisor mode and the kernel expects and checks that CPUs start in the same state as initial boot this results in a dead system. Specifically, it crashes early in boot when the primary CPU runs the MCPM test [1] and even if power management features are disabled it will still refuse to bring up any secondary CPUs.
Fix this problem by removing U-Boot support for virt and nonsec support on TC2.
So this disables any kind of virtualization support for TC2, which is a bit unfortunate. If I get this (and Sudeep's explanations) correctly, this new behaviour comes from the per-CPU-mailboxes setting in SCC 0x700, which is a setting in boot.txt on the uSD card. I wonder if we could make this virt/secure support a runtime decision then based on that register? So that a user can select whether she wants virtualisation or power management by changing the boot.txt setting and U-Boot transparently adapts to it, entering the kernel in either secure SVC or HYP.
Does that make sense? I can look into a fix if this approach is fine.
Cheers, Andre.
[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=35...
Signed-off-by: Jon Medhurst tixy@linaro.org
arch/arm/Kconfig | 2 -- board/armltd/vexpress/vexpress_common.c | 15 --------------- 2 files changed, 17 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e9d2fc9..2e48568 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -293,8 +293,6 @@ config ARCH_BCM283X config TARGET_VEXPRESS_CA15_TC2 bool "Support vexpress_ca15_tc2" select CPU_V7
- select CPU_V7_HAS_NONSEC
- select CPU_V7_HAS_VIRT
config TARGET_VEXPRESS_CA5X2 bool "Support vexpress_ca5x2" diff --git a/board/armltd/vexpress/vexpress_common.c b/board/armltd/vexpress/vexpress_common.c index d3b3b31..fe5d163 100644 --- a/board/armltd/vexpress/vexpress_common.c +++ b/board/armltd/vexpress/vexpress_common.c @@ -180,18 +180,3 @@ void lowlevel_init(void) ulong get_board_rev(void){ return readl((u32 *)SYS_ID); }
-#ifdef CONFIG_ARMV7_NONSEC -/* Setting the address at which secondary cores start from.
- Versatile Express uses one address for all cores, so ignore corenr
- */
-void smp_set_core_boot_addr(unsigned long addr, int corenr) -{
- /* The SYSFLAGS register on VExpress needs to be cleared first
* by writing to the next address, since any writes to the address* at offset 0 will only be ORed in*/- writel(~0, CONFIG_SYSFLAGS_ADDR + 4);
- writel(addr, CONFIG_SYSFLAGS_ADDR);
-} -#endif
On Wed, 2016-06-22 at 15:53 +0100, Andre Przywara wrote:
Hi,
On 22/06/16 15:34, Jon Medhurst (Tixy) wrote:
When CPU's come out of reset they are in secure state supervisor mode, so this is the state Linux kernel entry point is called in when it brings up secondary CPU cores or the primary CPU restarts after power management has sent it through an off/on transition.
As U-Boot starts the kernel in hypervisor mode and the kernel expects and checks that CPUs start in the same state as initial boot this results in a dead system. Specifically, it crashes early in boot when the primary CPU runs the MCPM test [1] and even if power management features are disabled it will still refuse to bring up any secondary CPUs.
Fix this problem by removing U-Boot support for virt and nonsec support on TC2.
So this disables any kind of virtualization support for TC2, which is a bit unfortunate. If I get this (and Sudeep's explanations) correctly, this new behaviour comes from the per-CPU-mailboxes setting in SCC 0x700, which is a setting in boot.txt on the uSD card.
Right, so I got the current U-Boot to boot Linux OK with all CPU's coming up in hyp mode by:
- Clearing bits 12 and 13 in SCC 0x700 to disable per-cpu mailboxes and keep the secondary CPU powered up.
- Removing kernel configs CONFIG_ARCH_VEXPRESS_TC2_PM CONFIG_MCPM
I believe this works because the SCC change means that all CPU will be running and waiting in the bootmonitor holding pen, from which they are released when U-Boot calls smp_set_core_boot_addr. These secondary CPUs then execute U-Boot code to switch to hypervisor mode, then enter a new holding pen. (Running in RAM that can't get overwritten by Linux I hope! )
Then when kernel boots and releases cores from holding pen they are in hyp mode, like the main CPU. As we've also disabled power management in the kernel, CPU's aren't ever powered down again and so stay in hyp mode.
Actually, I just tried hotplugging a CPU which obviously the kernel didn't like, so there is probably other kernel configs that need tweaking to make things work.
I wonder if we could make this virt/secure support a runtime decision then based on that register? So that a user can select whether she wants virtualisation or power management by changing the boot.txt setting and U-Boot transparently adapts to it, entering the kernel in either secure SVC or HYP.
Does that make sense? I can look into a fix if this approach is fine.
Anything that gets U-Boot in it's default config working with vexpress firmware and Linux kernel in their default config would be good start :-)
I'm not sure that having an automatic selection in U-Boot is worth it, given that anyone booting in hyp mode also needs to modify the firmware and kernel configs to get things working. But I wouldn't object to such an automatic selection either. After all, it's not like anyone really uses or cares about this stuff, and if they are, they probably have there own customised setup rather than using upstream defaults.
The firmware on TC2 needs to be configured appropriately before booting in nonsec mode will work as expected, so test for this and fall back to sec mode if required.
Signed-off-by: Jon Medhurst tixy@linaro.org ---
This is an implementation of Andre's suggestion in http://lists.denx.de/pipermail/u-boot/2016-June/258873.html
Possibly the change to bootm.c should be in a separate patch?
arch/arm/lib/bootm.c | 15 ++++++++++----- board/armltd/vexpress/Makefile | 1 + board/armltd/vexpress/vexpress_tc2.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 board/armltd/vexpress/vexpress_tc2.c
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 0838d89..766a0c8 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -248,15 +248,20 @@ static void boot_prep_linux(bootm_headers_t *images) } }
-#ifdef CONFIG_ARMV7_NONSEC -bool armv7_boot_nonsec(void) +__weak bool armv7_boot_nonsec_default(void) { - char *s = getenv("bootm_boot_mode"); #ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT - bool nonsec = false; + return false; #else - bool nonsec = true; + return true; #endif +} + +#ifdef CONFIG_ARMV7_NONSEC +bool armv7_boot_nonsec(void) +{ + char *s = getenv("bootm_boot_mode"); + bool nonsec = armv7_boot_nonsec_default();
if (s && !strcmp(s, "sec")) nonsec = false; diff --git a/board/armltd/vexpress/Makefile b/board/armltd/vexpress/Makefile index 1dd6780..95f4ec0 100644 --- a/board/armltd/vexpress/Makefile +++ b/board/armltd/vexpress/Makefile @@ -6,3 +6,4 @@ #
obj-y := vexpress_common.o +obj-$(CONFIG_TARGET_VEXPRESS_CA15_TC2) += vexpress_tc2.o diff --git a/board/armltd/vexpress/vexpress_tc2.c b/board/armltd/vexpress/vexpress_tc2.c new file mode 100644 index 0000000..f00a83c --- /dev/null +++ b/board/armltd/vexpress/vexpress_tc2.c @@ -0,0 +1,33 @@ +/* + * (C) Copyright 2016 Linaro + * Jon Medhurst tixy@linaro.org + * + * TC2 specific code for Versatile Express. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <asm/io.h> + +#define SCC_BASE 0x7fff0000 + +bool armv7_boot_nonsec_default(void) +{ +#ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT + return false +#else + /* + * The Serial Configuration Controller (SCC) register at address 0x700 + * contains flags for configuring the behaviour of the Boot Monitor + * (which CPUs execute from reset). Two of these bits are of interest: + * + * bit 12 = Use per-cpu mailboxes for power management + * bit 13 = Power down the non-boot cluster + * + * It is only when both of these are false that U-Boot's current + * implementation of 'nonsec' mode can work as expected because we + * rely on getting all CPUs to execute _nonsec_init, so let's check that. + */ + return (readl((u32 *)(SCC_BASE + 0x700)) & ((1 << 12) | (1 << 13))) == 0; +#endif +}
On 23 June 2016 at 13:37, Jon Medhurst (Tixy) tixy@linaro.org wrote:
The firmware on TC2 needs to be configured appropriately before booting in nonsec mode will work as expected, so test for this and fall back to sec mode if required.
Signed-off-by: Jon Medhurst tixy@linaro.org
Reviewed-by: Ryan Harkin ryan.harkin@linaro.org Tested-by: Ryan Harkin ryan.harkin@linaro.org
I tested with a "regular" linux kernel only. I didn't test with hyp mode enabled.
This is an implementation of Andre's suggestion in http://lists.denx.de/pipermail/u-boot/2016-June/258873.html
Possibly the change to bootm.c should be in a separate patch?
arch/arm/lib/bootm.c | 15 ++++++++++----- board/armltd/vexpress/Makefile | 1 + board/armltd/vexpress/vexpress_tc2.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 board/armltd/vexpress/vexpress_tc2.c
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 0838d89..766a0c8 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -248,15 +248,20 @@ static void boot_prep_linux(bootm_headers_t *images) } }
-#ifdef CONFIG_ARMV7_NONSEC -bool armv7_boot_nonsec(void) +__weak bool armv7_boot_nonsec_default(void) {
char *s = getenv("bootm_boot_mode");#ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT
bool nonsec = false;
return false;#else
bool nonsec = true;
return true;#endif +}
+#ifdef CONFIG_ARMV7_NONSEC +bool armv7_boot_nonsec(void) +{
char *s = getenv("bootm_boot_mode");bool nonsec = armv7_boot_nonsec_default(); if (s && !strcmp(s, "sec")) nonsec = false;diff --git a/board/armltd/vexpress/Makefile b/board/armltd/vexpress/Makefile index 1dd6780..95f4ec0 100644 --- a/board/armltd/vexpress/Makefile +++ b/board/armltd/vexpress/Makefile @@ -6,3 +6,4 @@ #
obj-y := vexpress_common.o +obj-$(CONFIG_TARGET_VEXPRESS_CA15_TC2) += vexpress_tc2.o diff --git a/board/armltd/vexpress/vexpress_tc2.c b/board/armltd/vexpress/vexpress_tc2.c new file mode 100644 index 0000000..f00a83c --- /dev/null +++ b/board/armltd/vexpress/vexpress_tc2.c @@ -0,0 +1,33 @@ +/*
- (C) Copyright 2016 Linaro
- Jon Medhurst tixy@linaro.org
- TC2 specific code for Versatile Express.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <asm/io.h>
+#define SCC_BASE 0x7fff0000
+bool armv7_boot_nonsec_default(void) +{ +#ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT
return false+#else
/** The Serial Configuration Controller (SCC) register at address 0x700* contains flags for configuring the behaviour of the Boot Monitor* (which CPUs execute from reset). Two of these bits are of interest:** bit 12 = Use per-cpu mailboxes for power management* bit 13 = Power down the non-boot cluster** It is only when both of these are false that U-Boot's current* implementation of 'nonsec' mode can work as expected because we* rely on getting all CPUs to execute _nonsec_init, so let's check that.*/return (readl((u32 *)(SCC_BASE + 0x700)) & ((1 << 12) | (1 << 13))) == 0;+#endif
+}
2.1.4
On Thu, Jun 23, 2016 at 01:37:32PM +0100, Jon Medhurst (Tixy) wrote:
The firmware on TC2 needs to be configured appropriately before booting in nonsec mode will work as expected, so test for this and fall back to sec mode if required.
Signed-off-by: Jon Medhurst tixy@linaro.org Reviewed-by: Ryan Harkin ryan.harkin@linaro.org Tested-by: Ryan Harkin ryan.harkin@linaro.org
This is an implementation of Andre's suggestion in http://lists.denx.de/pipermail/u-boot/2016-June/258873.html
Possibly the change to bootm.c should be in a separate patch?
arch/arm/lib/bootm.c | 15 ++++++++++----- board/armltd/vexpress/Makefile | 1 + board/armltd/vexpress/vexpress_tc2.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 board/armltd/vexpress/vexpress_tc2.c
So, this supersedes https://patchwork.ozlabs.org/patch/639232/ right? Thanks!
On Sun, 2016-08-14 at 16:05 -0400, Tom Rini wrote:
On Thu, Jun 23, 2016 at 01:37:32PM +0100, Jon Medhurst (Tixy) wrote:
The firmware on TC2 needs to be configured appropriately before booting in nonsec mode will work as expected, so test for this and fall back to sec mode if required.
Signed-off-by: Jon Medhurst tixy@linaro.org Reviewed-by: Ryan Harkin ryan.harkin@linaro.org Tested-by: Ryan Harkin ryan.harkin@linaro.org
This is an implementation of Andre's suggestion in http://lists.denx.de/pipermail/u-boot/2016-June/258873.html
Possibly the change to bootm.c should be in a separate patch?
arch/arm/lib/bootm.c | 15 ++++++++++----- board/armltd/vexpress/Makefile | 1 + board/armltd/vexpress/vexpress_tc2.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 board/armltd/vexpress/vexpress_tc2.c
So, this supersedes https://patchwork.ozlabs.org/patch/639232/ right?
Yes, it does.
Thanks
On Thu, Jun 23, 2016 at 01:37:32PM +0100, Jon Medhurst (Tixy) wrote:
The firmware on TC2 needs to be configured appropriately before booting in nonsec mode will work as expected, so test for this and fall back to sec mode if required.
Signed-off-by: Jon Medhurst tixy@linaro.org Reviewed-by: Ryan Harkin ryan.harkin@linaro.org Tested-by: Ryan Harkin ryan.harkin@linaro.org
Applied to u-boot/master, thanks!
participants (4)
-
Andre Przywara -
Jon Medhurst (Tixy) -
Ryan Harkin -
Tom Rini