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

Georges Savoundararadj
2014-10-20T21:08:30Z

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@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@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?

Regards,

Georges