[U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
Older Linux kernels will not properly boot in hype mode, add support for a bootm_boot_mode environment variable, which when set to "sec" will cause u-boot to boot in secure mode even when build with non-sec (and hyp) support.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Marc Zyngier marc.zyngier@arm.com Acked-by: Siarhei Siamashka siarhei.siamashka@gmail.com
-- Changes in v2: -Allow changing the default boot mode to secure through defining CONFIG_ARMV7_SEC_BY_DEFAULT, this is useful for archs which have a Kconfig option for compatibility with older kernels --- arch/arm/lib/bootm.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 39fe7a1..ff0170a 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -235,6 +235,26 @@ static void boot_prep_linux(bootm_headers_t *images) } }
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT) +static bool boot_nonsec(void) +{ + char *s = getenv("bootm_boot_mode"); +#ifdef CONFIG_ARMV7_SEC_BY_DEFAULT + bool nonsec = false; +#else + bool nonsec = true; +#endif + + if (s && !strcmp(s, "sec")) + nonsec = false; + + if (s && !strcmp(s, "nonsec")) + nonsec = true; + + return nonsec; +} +#endif + /* Subcommand: GO */ static void boot_jump_linux(bootm_headers_t *images, int flag) { @@ -283,12 +303,13 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
if (!fake) { #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); + if (boot_nonsec()) { + armv7_init_nonsec(); + secure_ram_addr(_do_nonsec_entry)(kernel_entry, + 0, machid, r2); + } #endif + kernel_entry(0, machid, r2); } #endif }
On Wed, 2014-10-22 at 15:45 +0200, Hans de Goede wrote:
if (!fake) { #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);
if (boot_nonsec()) {armv7_init_nonsec();secure_ram_addr(_do_nonsec_entry)(kernel_entry,0, machid, r2);}#endif
kernel_entry(0, machid, r2);
There's a subtle different here, which is that this final kernel_entry call used to be in the #else clause, and so emitted for the NONSEC || VIRT case. So if the _do_nonsec_entry call were to fail (not currently possible) and return you'd end up trying again via the sec path.
I'm not sure that's a bad thing, but it is a difference so it'd be good to know it was a deliberate choice (or not).
Ian.
Hi Ian,
On 10/22/2014 08:55 PM, Ian Campbell wrote:
On Wed, 2014-10-22 at 15:45 +0200, Hans de Goede wrote:
if (!fake) { #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);
if (boot_nonsec()) {armv7_init_nonsec();secure_ram_addr(_do_nonsec_entry)(kernel_entry,0, machid, r2);}#endif
kernel_entry(0, machid, r2);There's a subtle different here, which is that this final kernel_entry call used to be in the #else clause, and so emitted for the NONSEC || VIRT case. So if the _do_nonsec_entry call were to fail (not currently possible) and return you'd end up trying again via the sec path.
I'm not sure that's a bad thing, but it is a difference so it'd be good to know it was a deliberate choice (or not).
I was under the assumption that do_nonsec_entry would never fail, and would not return, which is why I wrote this code the way I wrote it. I'm not sure if retrying in secure mode meets the principle of least surprise, so I guess the #if .. #endif block should probably get an "else" added before the #endif, do you agree?
Regards,
Hans
On Thu, 2014-10-23 at 10:22 +0200, Hans de Goede wrote:
Hi Ian,
On 10/22/2014 08:55 PM, Ian Campbell wrote:
On Wed, 2014-10-22 at 15:45 +0200, Hans de Goede wrote:
if (!fake) { #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);
if (boot_nonsec()) {armv7_init_nonsec();secure_ram_addr(_do_nonsec_entry)(kernel_entry,0, machid, r2);}#endif
kernel_entry(0, machid, r2);There's a subtle different here, which is that this final kernel_entry call used to be in the #else clause, and so emitted for the NONSEC || VIRT case. So if the _do_nonsec_entry call were to fail (not currently possible) and return you'd end up trying again via the sec path.
I'm not sure that's a bad thing, but it is a difference so it'd be good to know it was a deliberate choice (or not).
I was under the assumption that do_nonsec_entry would never fail, and would not return, which is why I wrote this code the way I wrote it.
AFAICT in practice it can't fail today, but if it were somehow modified in the future to do so this would expose some slightly surprising behaviour.
I'm not sure if retrying in secure mode meets the principle of least surprise, so I guess the #if .. #endif block should probably get an "else" added before the #endif, do you agree?
Yes.
BTW, if you put the #ifdef around boot_nonsec() instead and make the #else case #define boot_nonsec() (0) then does that end up looking cleaner here at the caller? Maybe that causes knockons with the prototypes for the unused functions in that case, in which case I doubt it is worth it.
Ian.
Hi,
On 10/23/2014 11:30 AM, Ian Campbell wrote:
On Thu, 2014-10-23 at 10:22 +0200, Hans de Goede wrote:
Hi Ian,
On 10/22/2014 08:55 PM, Ian Campbell wrote:
On Wed, 2014-10-22 at 15:45 +0200, Hans de Goede wrote:
if (!fake) { #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);
if (boot_nonsec()) {armv7_init_nonsec();secure_ram_addr(_do_nonsec_entry)(kernel_entry,0, machid, r2);}#endif
kernel_entry(0, machid, r2);There's a subtle different here, which is that this final kernel_entry call used to be in the #else clause, and so emitted for the NONSEC || VIRT case. So if the _do_nonsec_entry call were to fail (not currently possible) and return you'd end up trying again via the sec path.
I'm not sure that's a bad thing, but it is a difference so it'd be good to know it was a deliberate choice (or not).
I was under the assumption that do_nonsec_entry would never fail, and would not return, which is why I wrote this code the way I wrote it.
AFAICT in practice it can't fail today, but if it were somehow modified in the future to do so this would expose some slightly surprising behaviour.
I'm not sure if retrying in secure mode meets the principle of least surprise, so I guess the #if .. #endif block should probably get an "else" added before the #endif, do you agree?
Yes.
BTW, if you put the #ifdef around boot_nonsec() instead and make the #else case #define boot_nonsec() (0) then does that end up looking cleaner here at the caller? Maybe that causes knockons with the prototypes for the unused functions in that case, in which case I doubt it is worth it.
The problem there is that do_nonsec_entry is not defined in that case, so we really need an #ifdef there.
Regards,
Hans
participants (2)
-
Hans de Goede -
Ian Campbell