[U-Boot] [PATCH v2 2/3] arm: relocate the exception vectors

[Georges Savoundararadj]

Hi Albert,

Le 15/10/2014 00:11, Albert ARIBAUD a écrit :
> Hi Georges,
>
> On Tue, 14 Oct 2014 22:02:00 +0200, Georges Savoundararadj
> <savoundg at gmail.com> wrote:
>
>> Hi Albert,
>>
>> Hi Masahiro,
> (putting Masahiro in Cc: just in case)
>
>> As my issue is related to Kconfig, I would like you to give me your
>> opinions.
>>
>>
>> Le 11/10/2014 12:47, Albert ARIBAUD a écrit :
>>> Hi Georges,
>>>
>>> On Sat, 27 Sep 2014 21:48:10 +0200, Georges Savoundararadj
>>> <savoundg at gmail.com> wrote:
>>>
>>>> This commit relocates the exception vectors.
>>>> As ARM1176 and ARMv7 have the security extensions, it uses VBAR.  For
>>>> the other ARM processors, it copies the relocated exception vectors to
>>>> the correct address: 0x00000000 or 0xFFFF0000.
>>>>
>>>> Signed-off-by: Georges Savoundararadj <savoundg at gmail.com>
>>>> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
>>>> Cc: Tom Warren <twarren at nvidia.com>
>>>>
>>>> ---
>>>> This patch needs some tests because it impacts many boards. I have
>>>> tested it with my raspberry pi in the two cases: using VBAR and
>>>> using the copied exception vectors.
>>>>
>>>> Changes in v2:
>>>> - Relocate exception vectors also on processors which do not support
>>>>     security extensions
>>>> - Reword the commit message
>>>>
>>>>    arch/arm/cpu/armv7/start.S |  6 ------
>>>>    arch/arm/lib/relocate.S    | 30 ++++++++++++++++++++++++++++++
>>>>    2 files changed, 30 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>>> index fedd7c8..fdc05b9 100644
>>>> --- a/arch/arm/cpu/armv7/start.S
>>>> +++ b/arch/arm/cpu/armv7/start.S
>>>> @@ -81,12 +81,6 @@ ENTRY(c_runtime_cpu_setup)
>>>>    	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>>>>    	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>>>>    #endif
>>>> -/*
>>>> - * Move vector table
>>>> - */
>>>> -	/* Set vector address in CP15 VBAR register */
>>>> -	ldr     r0, =_start
>>>> -	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>>>>    
>>>>    	bx	lr
>>>>    
>>>> diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S
>>>> index 8035251..88a478e 100644
>>>> --- a/arch/arm/lib/relocate.S
>>>> +++ b/arch/arm/lib/relocate.S
>>>> @@ -6,6 +6,8 @@
>>>>     * SPDX-License-Identifier:	GPL-2.0+
>>>>     */
>>>>    
>>>> +#include <asm-offsets.h>
>>>> +#include <config.h>
>>>>    #include <linux/linkage.h>
>>>>    
>>>>    /*
>>>> @@ -52,6 +54,34 @@ fixnext:
>>>>    	cmp	r2, r3
>>>>    	blo	fixloop
>>>>    
>>>> +	/*
>>>> +	 * Relocate the exception vectors
>>>> +	 */
>>>> +#if (defined(CONFIG_ARM1176) || defined(CONFIG_ARMV7))
>>> I would prefer a single CONFIG_HAS_VBAR symbol defined through
>>> Kconfig.
>> 1)
>> Actually, there is no Kconfig entry such as "config ARM1176" nor "config
>> ARMV7" in U-Boot,
>> unlike in Linux (arch/arm/mm/Kconfig).
>>
>> If there were such entries, we would simply do like the following (in
>> arch/arm/Kconfig):
>>
>> config HAS_VBAR
>>       bool
>>
>> config ARM1176
>>       select HAS_VBAR
>>
>> config ARMV7
>>       select HAS_VBAR
>>
>> Should we go in this direction?
>> It is the cleanest way to use Kconfig but it requires some work in order
>> to convert all
>> "#define CONFIG_<cpu>" into Kconfig entries.
>>
>> 2)
>> Otherwise, we can insert a "select HAS_VBAR" in all boards that have a
>> ARM1176 or a ARMv7
>> processor in arch/arm/Kconfig. It is not logical but this is what has
>> been done with the Kconfig
>> entry ARM64. And, it does not require much change.
>>
>> 3)
>> The last thing we can do is as follows:
>>
>> config HAS_VBAR
>>           bool
>>           depends on SYS_CPU = "arm1176" || SYS_CPU = "armv7"
>>           default y
>>
>> CONFIG_HAS_VBAR will be defined if SYS_CPU are arm1176 or armv7. It does
>> not require much
>> change as well but, I think, it is bad code.
>>
>> What do you think is the best way to introduce CONFIG_HAS_VBAR symbol?
>> (1, 2 or 3)
> I believe you have already sorted the options in order of decreasing
> 'quality' -- 1 being the best option, and 3 being the worst... Indeed
> option 1 would be the best and cleanest, and it could possibly open the
> way for other per-CPU options.
>
> We could try and limit the effort to converting only ARM1176 and ARMV7
> and leaving other CONFIG_<cpu> #define'd until some later point in the
> future, but experience shows that such half-hearted attempts are never
> completed.
>
> Amicalement,

I am currently trying to implement solution 1. only for ARM1176 and 
ARMV7 but I wonder
if this work worth the effort just for one CPU feature.
Do you expect more CPU feature like HAS_VBAR coming in the future?

I add the following lines in arch/arm/Kconfig:
config HAS_VBAR
        bool

config ARM1176
        bool
        select HAS_VBAR

config ARMV7
         bool
         select HAS_VBAR

config SYS_CPU
        default "arm1176" if ARM1176
        default "armv7" if ARMV7

Then, in the same file, under each "config TARGET_<board>",  I add 
"select ARM1176" or "select ARMV7".
Also, I delete the Kconfig entries "config SYS_CPU" in all Kconfig of 
*all* boards that use ARM1176 and ARMV7.

Actually, I find the change quite big. What do you think about this 
implementation?
Should I continue in this direction?

Regards,

Georges