* [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
@ 2014-10-22 13:45 Hans de Goede
2014-10-22 18:55 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2014-10-22 13:45 UTC (permalink / raw)
To: u-boot
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
}
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
2014-10-22 13:45 [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems Hans de Goede
@ 2014-10-22 18:55 ` Ian Campbell
2014-10-23 8:22 ` Hans de Goede
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2014-10-22 18:55 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
2014-10-22 18:55 ` Ian Campbell
@ 2014-10-23 8:22 ` Hans de Goede
2014-10-23 9:30 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2014-10-23 8:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
2014-10-23 8:22 ` Hans de Goede
@ 2014-10-23 9:30 ` Ian Campbell
2014-10-23 9:37 ` Hans de Goede
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2014-10-23 9:30 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems
2014-10-23 9:30 ` Ian Campbell
@ 2014-10-23 9:37 ` Hans de Goede
0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2014-10-23 9:37 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-23 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-22 13:45 [U-Boot] [PATCH v2] ARM: bootm: Allow booting in secure mode on hyp capable systems Hans de Goede
2014-10-22 18:55 ` Ian Campbell
2014-10-23 8:22 ` Hans de Goede
2014-10-23 9:30 ` Ian Campbell
2014-10-23 9:37 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox