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

[Georges Savoundararadj <savoundg@gmail.com>]

Hi Albert and Masahiro,

Le 22/10/2014 11:54, Masahiro Yamada a ?crit :
> Hi Albert,
>
>
>
> On Tue, 21 Oct 2014 15:54:51 +0200
> Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
>
>> Hi Georges,
>>
>> On Mon, 20 Oct 2014 23:08:30 +0200, Georges Savoundararadj
>> <savoundg@gmail.com> wrote:
>>
>>> Hi Albert,
>>>
>>> Le 15/10/2014 00:11, Albert ARIBAUD a ecrit :
>>>> Hi Georges,
>>>>
>>>> On Tue, 14 Oct 2014 22:02:00 +0200, Georges Savoundararadj
>>>> <savoundg@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 ecrit :
>>>>>> Hi Georges,
>>>>>>
>>>>>> On Sat, 27 Sep 2014 21:48:10 +0200, Georges Savoundararadj
>>>>>> <savoundg@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@gmail.com>
>>>>>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>>>>>>> Cc: Tom Warren <twarren@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?
>> This looks like the right way to me
OK, that's what I wanted to know.
>> (even if ideally I would prefer
>> that SYS_CPU be deduced from the SYS_SOC defined in the boards' Kconfig
>> files rather than added to them).
OK.

>> Hopefully you can devise a sed, awk o perl script to do the change
>> without too much manual effort?
Yes, I can try to script.
>> Incidentally, this raises a question which Masahiro can probably
>> answer. In arch/arm/Kconfig, every ARM board is referred to twice:
>>
>> - once in a "config TARGET_<board>" block;
>>
>> - once in a "source board[/<maker>]/<board>/Kconfig directive.
>>
>> Would it be possible to move each "TARGET_<board>" block from
>> arch/arm/Kconfig to the corresponding board[/<maker>]/<board>/Kconfig
>> and only keep the "source" directives in arch/arm/Kconfig?
>
> I think it is impossible.
>
> The first one appears in "config choice" .. "endchoice"
> to select an appropriate board/platform.
>
>
>> (and then, I'd *really* like a way to source all ARM-based boards in a
>> few lines, e.g. source /board/*/Kconfig + source board/*/*/Kconfig)
>>
>> It would be nice if all Kconfig settings for a given board were found
>> in the board's Kconfig.
> I have no idea to achieve this.
>
@Albert: I will see what can I do.

Should I post the Kconfig patch in a separate series ?
>
>
> Best Regards
> Masahiro Yamada
>

Thanks for your advices,

Regards,

Georges