[PATCH u-boot-marvell v2 1/3] arm: Introduce new CONFIG_SPL_SYS_NO_VECTOR_TABLE option

[Pali Rohár]

On Wednesday 03 August 2022 02:02:22 Heinrich Schuchardt wrote:
> On 4/6/22 16:20, Pali Rohár wrote:
> > Move OMAP4 specific option for disabling overwriting vector table into
> > config option CONFIG_SPL_SYS_NO_VECTOR_TABLE.
> > 
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> >   arch/arm/Kconfig            | 4 ++++
> >   arch/arm/cpu/armv7/start.S  | 4 +---
> >   arch/arm/mach-omap2/Kconfig | 1 +
> >   3 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 4567c183fb84..1be8d4470ec1 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -57,6 +57,10 @@ config SYS_INIT_SP_BSS_OFFSET
> >   	  that the early malloc region, global data (gd), and early stack usage
> >   	  do not overlap any appended DTB.
> > 
> > +config SPL_SYS_NO_VECTOR_TABLE
> > +	depends on SPL
> > +	bool
> > +
> 
> ARMv8 has symbol CONFIG_ARMV8_SPL_EXCEPTION_VECTORS.
> 
> Why do we need two separate setting which have the same usage (but with
> inverse logic)? Can't we consolidate the two?
> 
> Why should this symbol not be editable?

These two symbols are different. CONFIG_ARMV8_SPL_EXCEPTION_VECTORS
describes that it is there for ARMv8 and its usage is if user wants to
save space, to decrease U-Boot binary.

This new SPL_SYS_NO_VECTOR_TABLE is there for ARMv7 and as you pointed
correctly it is not user editable. This symbol is there for platforms
which _require_ that SPL does not modify exception tables. If platform
sets it as needed then user must not be able to disable this symbol.

I hope that I explained it in commit message, SPL on 32-bit mvebu boards
is started by BootROM "as a function". SPL must returns control to
BootROM and BootROM expects that vector table is not modified.

> I guess it needs the same warning as for ARMv8 that if you set it there
> will be no crash dump in SPL.

As this symbol is not user editable, it does not have description
(requirement by Kconfig language). If crash happens then it is handled
by BootROM itself.

But I think that description is not needed if symbol is not user
editable. It is up to the maintainer of architecture to decide if this
option must be enabled or disabled (default) based on other things.

Probably what could make sense is to "lift"
CONFIG_ARMV8_SPL_EXCEPTION_VECTORS option to be user selectable and for
both ARMv7 and ARMv8. To allow user to disable generating vectors on
ARMv7 too (but not to enable them if architecture has to have them
disabled).

> Best regards
> 
> Heinrich
> 
> >   config LINUX_KERNEL_IMAGE_HEADER
> >   	depends on ARM64
> >   	bool
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index af87a5432ae5..37036128a785 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -97,12 +97,10 @@ switch_to_hypervisor_ret:
> >   	orr	r0, r0, #0xc0		@ disable FIQ and IRQ
> >   	msr	cpsr,r0
> > 
> > +#if !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
> >   /*
> >    * Setup vector:
> > - * (OMAP4 spl TEXT_BASE is not 32 byte aligned.
> > - * Continue to use ROM code vector only in OMAP4 spl)
> >    */
> > -#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
> >   	/* Set V=0 in CP15 SCTLR register - for VBAR to point to vector */
> >   	mrc	p15, 0, r0, c1, c0, 0	@ Read CP15 SCTLR Register
> >   	bic	r0, #CR_V		@ V = 0
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 263142683b04..e1b9180a3bb3 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -32,6 +32,7 @@ config OMAP34XX
> >   config OMAP44XX
> >   	bool "OMAP44XX SoC"
> >   	select SPL_USE_TINY_PRINTF
> > +	select SPL_SYS_NO_VECTOR_TABLE if SPL
> >   	imply NAND_OMAP_ELM
> >   	imply NAND_OMAP_GPMC
> >   	imply SPL_DISPLAY_PRINT
>