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

Georges Savoundararadj
2014-10-22T20:52:19Z

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