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

Georges Savoundararadj
2014-10-22T20:29:42Z

Hi Masahiro,

Le 21/10/2014 07:41, Masahiro Yamada a ?crit :
> Hi Georges and Albert,
>
> Sorry for late reply because I was out of office for ELCE2014
> and missed this thread.
>
>
>
> 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?
>>
> Agreed on 1).
>
> I was thinking about this since I introduced Kconfig at 2014.10-rc1.
> It is good to know you're working on this since it can save my time. :-)
>
> My only request is, can you use CPU_ARM1176, CPU_V7 instead of ARM1176, ARMV7 ?
> It looks like arm/arm/mm/Kconfig uses this way and CONFIG_CPU_ prefix makes things clear.

OK, I will use them.
>
> CONFIG_ARM1176 and CONFIG_ARMV7 are never referenced at all.
> Also, CONFIG_ARMV7 is only defined in some armv7 boards.
> For instance, Zynq boards define it but Tegra boards don't.
> It is useless and should be removed someday.
OK.
>
>
> I have a question:
>
> You are covering only arm1176 and armv7.
> What about arm1136?
>
> I am not sure, but arm1136 and arm1176 both belong to ARMv6 generation?
> If so, does arm1136 have VBAR register, doesn't it?
>
>
>
>
>
> Best Regards
> Masahiro Yamada
>
Thanks,

Georges