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

[Georges Savoundararadj <savoundg@gmail.com>]

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