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

Masahiro Yamada
2014-10-22T09:54:17Z

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 (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).
>
> Hopefully you can devise a sed, awk o perl script to do the change
> without too much manual effort?
>
> 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.

Best Regards
Masahiro Yamada