[U-Boot] [PATCH] ARM: bootm: do not add PSCI to fdt when booting in secure mode.

[U-Boot] [PATCH] ARM: bootm: do not add PSCI to fdt when booting in secure mode.

[Ian Campbell]

Commit 8bc347e2ec17 "ARM: bootm: Allow booting in secure mode on hyp capable
systems" added the capability to select nonsec vs sec mode boot via an
environment var.

There is a subtle gotcha with this functionality, which is that the PSCI nodes
are still created in the fdt (via armv7_update_dt->fdt_psci) even when booting
in secure mode. Which means that if the kernel is PSCI aware then it will fail
to boot because it will try and do PSCI from secure world, which won't work.

This likely didn't get noticed before because the original purpose was to
support booting the legacy linux-sunxi kernels which don't understand PSCI.

To fix expose boot_nonsec (renaming with armv7_ prefix) outside of bootm.c and
use from the virt-dt code.

As well as avoiding the creation of the PSCI nodes we should also avoid
reserving the secure RAM, so do so.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Cc: Tom Rini <trini@ti.com>
---
I think this should go into v2015.01 as a bug fix.
---
 arch/arm/cpu/armv7/virt-dt.c | 2 ++
 arch/arm/include/asm/armv7.h | 1 +
 arch/arm/lib/bootm.c         | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
index 0b0d6a7..ad19e4c 100644
--- a/arch/arm/cpu/armv7/virt-dt.c
+++ b/arch/arm/cpu/armv7/virt-dt.c
@@ -90,6 +90,8 @@ static int fdt_psci(void *fdt)
 
 int armv7_update_dt(void *fdt)
 {
+	if (!armv7_boot_nonsec())
+		return 0;
 #ifndef CONFIG_ARMV7_SECURE_BASE
 	/* secure code lives in RAM, keep it alive */
 	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index 323f282..a13da23 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -80,6 +80,7 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
 
 int armv7_init_nonsec(void);
 int armv7_update_dt(void *fdt);
+bool armv7_boot_nonsec(void);
 
 /* defined in assembly file */
 unsigned int _nonsec_init(void);
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index a7f7c67..0c1298a 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -238,7 +238,7 @@ static void boot_prep_linux(bootm_headers_t *images)
 }
 
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
-static bool boot_nonsec(void)
+bool armv7_boot_nonsec(void)
 {
 	char *s = getenv("bootm_boot_mode");
 #ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT
@@ -305,7 +305,7 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
 
 	if (!fake) {
 #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
-		if (boot_nonsec()) {
+		if (armv7_boot_nonsec()) {
 			armv7_init_nonsec();
 			secure_ram_addr(_do_nonsec_entry)(kernel_entry,
 							  0, machid, r2);
-- 
2.1.3

[U-Boot] [PATCH] ARM: bootm: do not add PSCI to fdt when booting in secure mode.

[Hans de Goede]

Hi,

On 21-12-14 10:45, Ian Campbell wrote:
> Commit 8bc347e2ec17 "ARM: bootm: Allow booting in secure mode on hyp capable
> systems" added the capability to select nonsec vs sec mode boot via an
> environment var.
>
> There is a subtle gotcha with this functionality, which is that the PSCI nodes
> are still created in the fdt (via armv7_update_dt->fdt_psci) even when booting
> in secure mode. Which means that if the kernel is PSCI aware then it will fail
> to boot because it will try and do PSCI from secure world, which won't work.
>
> This likely didn't get noticed before because the original purpose was to
> support booting the legacy linux-sunxi kernels which don't understand PSCI.
>
> To fix expose boot_nonsec (renaming with armv7_ prefix) outside of bootm.c and
> use from the virt-dt code.
>
> As well as avoiding the creation of the PSCI nodes we should also avoid
> reserving the secure RAM, so do so.
>
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@ti.com>

Looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

> ---
> I think this should go into v2015.01 as a bug fix.

Agreed.

Regards,

Hans

> ---
>   arch/arm/cpu/armv7/virt-dt.c | 2 ++
>   arch/arm/include/asm/armv7.h | 1 +
>   arch/arm/lib/bootm.c         | 4 ++--
>   3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
> index 0b0d6a7..ad19e4c 100644
> --- a/arch/arm/cpu/armv7/virt-dt.c
> +++ b/arch/arm/cpu/armv7/virt-dt.c
> @@ -90,6 +90,8 @@ static int fdt_psci(void *fdt)
>
>   int armv7_update_dt(void *fdt)
>   {
> +	if (!armv7_boot_nonsec())
> +		return 0;
>   #ifndef CONFIG_ARMV7_SECURE_BASE
>   	/* secure code lives in RAM, keep it alive */
>   	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index 323f282..a13da23 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -80,6 +80,7 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
>
>   int armv7_init_nonsec(void);
>   int armv7_update_dt(void *fdt);
> +bool armv7_boot_nonsec(void);
>
>   /* defined in assembly file */
>   unsigned int _nonsec_init(void);
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index a7f7c67..0c1298a 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -238,7 +238,7 @@ static void boot_prep_linux(bootm_headers_t *images)
>   }
>
>   #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
> -static bool boot_nonsec(void)
> +bool armv7_boot_nonsec(void)
>   {
>   	char *s = getenv("bootm_boot_mode");
>   #ifdef CONFIG_ARMV7_BOOT_SEC_DEFAULT
> @@ -305,7 +305,7 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
>
>   	if (!fake) {
>   #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
> -		if (boot_nonsec()) {
> +		if (armv7_boot_nonsec()) {
>   			armv7_init_nonsec();
>   			secure_ram_addr(_do_nonsec_entry)(kernel_entry,
>   							  0, machid, r2);
>

[U-Boot] ARM: bootm: do not add PSCI to fdt when booting in secure mode.

[Tom Rini]

On Sun, Dec 21, 2014 at 09:45:11AM +0000, Ian Campbell wrote:

> Commit 8bc347e2ec17 "ARM: bootm: Allow booting in secure mode on hyp capable
> systems" added the capability to select nonsec vs sec mode boot via an
> environment var.
> 
> There is a subtle gotcha with this functionality, which is that the PSCI nodes
> are still created in the fdt (via armv7_update_dt->fdt_psci) even when booting
> in secure mode. Which means that if the kernel is PSCI aware then it will fail
> to boot because it will try and do PSCI from secure world, which won't work.
> 
> This likely didn't get noticed before because the original purpose was to
> support booting the legacy linux-sunxi kernels which don't understand PSCI.
> 
> To fix expose boot_nonsec (renaming with armv7_ prefix) outside of bootm.c and
> use from the virt-dt code.
> 
> As well as avoiding the creation of the PSCI nodes we should also avoid
> reserving the secure RAM, so do so.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@ti.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150107/161f547a/attachment.pgp>