From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andre Przywara <andre.przywara@linaro.org>
Date: Tue, 30 Jul 2013 13:32:14 +0200
Subject: [U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during
	bootm execution
In-Reply-To: <20130729220224.GB26768@cbox>
References: <1373414059-22779-1-git-send-email-andre.przywara@linaro.org>
	<1373414059-22779-5-git-send-email-andre.przywara@linaro.org>
	<20130729220224.GB26768@cbox>
Message-ID: <51F7A43E.4020903@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 07/30/2013 12:02 AM, Christoffer Dall wrote:
> On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:
>
> [...]
>
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 1b6e0ac..7b0619e 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -34,6 +34,10 @@
>>   #include <asm/bootm.h>
>>   #include <linux/compiler.h>
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +#include <asm/armv7.h>
>> +#endif
>> +
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>>   static struct tag *params;
>> @@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
>>
>>   __weak void setup_board_tags(struct tag **in_params) {}
>>
>> +static void do_nonsec_virt_switch(void)
>> +{
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +	int ret;
>> +
>> +	ret = armv7_switch_nonsec();
>> +	switch (ret) {
>> +	case NONSEC_VIRT_SUCCESS:
>> +		debug("entered non-secure state\n");
>> +		break;
>> +	case NONSEC_ERR_NO_SEC_EXT:
>> +		printf("nonsec: Security extensions not implemented.\n");
>> +		break;
>> +	case NONSEC_ERR_NO_GIC_ADDRESS:
>> +		printf("nonsec: could not determine GIC address.\n");
>> +		break;
>> +	case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
>> +		printf("nonsec: PERIPHBASE is above 4 GB, no access.\n");
>> +		break;
>> +	}
>> +#endif
>> +}
>
> I still don't get why you just don't make armv7_switch_nonsec a void and
> print the error when they occur... ???

My apologies for not elaborating on these comments I didn't incorporate:

So, I don't like the idea of marrying a low-level routine with high 
level output. I don't want to constraint the usage of the routine by 
requiring an output channel. Also some parts may not be fatal for all 
users - someone could just try to switch and then behave differently if 
that failed - without bothering the user.
May seem a bit over-engineered, but I like it better this way ;-)

If that is a show-stopper for you, I can change it, of course.

Regards,
Andre.