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

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

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,
-- 
Albert.