[U-Boot] [PATCH v2 3/3] ARM: bootm: Allow booting in secure mode on hyp capable systems

[Hans de Goede]

Hi,

On 11/14/2014 08:33 AM, Albert ARIBAUD wrote:
> Hello Hans,
> 
> On Thu, 13 Nov 2014 20:37:42 +0100, Hans de Goede <hdegoede at redhat.com>
> wrote:
>> Older Linux kernels will not properly boot in hyp mode, add support for a
>> bootm_boot_mode environment variable, which can be set to "sec" or "nonsec"
>> to force booting in secure or non-secure mode when build with non-sec support.
>>
>> The default behavior can be selected through CONFIG_ARMV7_BOOT_SEC_DEFAULT,
>> when this is set booting in secure mode is the default. The default setting
>> for this Kconfig option is N, preserving the current behavior of booting in
>> non-secure mode by default when non-secure mode is supported.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> Acked-by: Marc Zyngier <marc.zyngier at arm.com>
>> Acked-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
>> --
>> Changes in v2:
>> -Allow changing the default boot mode to secure through defining
>>  CONFIG_ARMV7_BOOT_SEC_DEFAULT, this is useful for archs which have a Kconfig
>>  option for compatibility with older kernels
>> Changes in v3:
>> -Add an else at the end of the #ifdef NONSEC block so that if do_nonsec_entry
>>  fails we do not end up re-trying in secure mode
>> Changes in v4:
>> -Add a Kconfig option to select to boot in secure or non-secure mode by default
>> ---
>>  arch/arm/cpu/armv7/Kconfig | 11 +++++++++++
>>  arch/arm/lib/bootm.c       | 31 ++++++++++++++++++++++++++-----
>>  2 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
>> index 15c5155..6ee5ff8 100644
>> --- a/arch/arm/cpu/armv7/Kconfig
>> +++ b/arch/arm/cpu/armv7/Kconfig
>> @@ -13,6 +13,17 @@ config ARMV7_NONSEC
>>  	---help---
>>  	Say Y here to enable support for booting in non-secure / SVC mode.
>>  
>> +config ARMV7_BOOT_SEC_DEFAULT
>> +	boolean "Boot in secure mode by default" if EXPERT
>> +	depends on ARMV7_NONSEC
>> +	default n
>> +	---help---
>> +	Say Y here to boot in secure mode by default even if non-secure mode
>> +	is supported. This option is useful to boot kernels which do not
>> +	suppport booting in secure mode. Only set this if you need it.
>> +	This can be overriden at run-time by setting the bootm_boot_mode env.
>> +	variable to "sec" or "nonsec".
> 
> Not sure I'm getting this right, but it seems to me that forcing secure
> boot mode for kernels that don't support secure boot mode is kind of
> contradictory. Did you mean "... for kernels which do not suport
> booting in *non*-secure mode..." ?

Yes, my bad will fix in v5.

> 
>>  config ARMV7_VIRT
>>  	boolean "Enable support for hardware virtualization" if EXPERT
>>  	depends on CPU_V7_HAS_VIRT && ARMV7_NONSEC
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 4949d57..a7f7c67 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -237,6 +237,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_BOOT_SEC_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)
>>  {
>> @@ -285,12 +305,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);
>> +		} else
>>  #endif
>> +			kernel_entry(0, machid, r2);
>>  	}
>>  #endif
>>  }
>> -- 
>> 2.1.0

Regards,

Hans