[PATCH u-boot-marvell] arm: mvebu: Do not define or overwrite vectors in SPL build

[Pali Rohár]

On Wednesday 16 February 2022 11:08:19 Stefan Roese wrote:
> On 2/15/22 20:02, Pali Rohár wrote:
> > U-Boot SPL is executed by the BootROM. And BootROM expects that U-Boot SPL
> > code returns back to the BootROM. Reset vectors during execution of
> > U-Boot SPL should not be changed as BootROM does not expect it and uses its
> > own reset vectors.
> 
> Just curious: How did you detect this problem? What actually happens
> when the reset vectors are overwritten here?

BootROM loose ability to detect / count issues with unbootable sources
which crashes. Nothing important when SPI source is fully bootable
without any issue. There are registers into which BootROM stores reset
information, so during _early_ debugging it could be useful to have it
filled.

> > Add ifdefs which disable overwriting reset vectors for 32-bit mvebu
> > platforms on which U-Boot SPL should returns back to the BootROM.
> > 
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> >   arch/arm/cpu/armv7/start.S | 7 ++++---
> >   arch/arm/lib/vectors.S     | 6 ++++++
> >   2 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index af87a5432ae5..b8175ea3808b 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -99,10 +99,11 @@ switch_to_hypervisor_ret:
> >   /*
> >    * Setup vector:
> > - * (OMAP4 spl TEXT_BASE is not 32 byte aligned.
> > - * Continue to use ROM code vector only in OMAP4 spl)
> > + * OMAP4 spl TEXT_BASE is not 32 byte aligned.
> > + * 32-bit mvebu spl returns execution back to BootROM and should not change vectors.
> > + * Continue to use ROM code vector only in OMAP4 or 32-bit mvebu spl.
> >    */
> > -#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
> > +#if !((defined(CONFIG_OMAP44XX) || (defined(CONFIG_ARCH_MVEBU) && defined(CONFIG_ARMADA_32BIT))) && defined(CONFIG_SPL_BUILD))
> 
> Will this file ever be compiled for (defined(CONFIG_ARCH_MVEBU) &&
> !defined(CONFIG_ARMADA_32BIT))? Meaning cpu/armv7 pretty much means
> 32bit, or did I miss something? If this is correct, you can probably
> drop one of the checks making this line a bit easier to read (again).

First I added just CONFIG_ARCH_MVEBU and realized that it is enabled
also for 64-bit mvebu platforms. So later I added CONFIG_ARMADA_32BIT
and forgot about it. And you are right CONFIG_ARMADA_32BIT should be
enough.

> Perhaps it's better to introduce a new Kconfig option for this instead
> and select it for the necessary platforms/targets?

This should be possible, but I do not know how to do this config symbol
magic correctly when it should be applied only for SPL builds...

> Thanks,
> Stefan
> 
> >   	/* 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/lib/vectors.S b/arch/arm/lib/vectors.S
> > index 56f36815582b..a0646e213b6d 100644
> > --- a/arch/arm/lib/vectors.S
> > +++ b/arch/arm/lib/vectors.S
> > @@ -24,6 +24,7 @@
> >   #else
> >   	b	reset
> >   #endif
> > +#if !(defined(CONFIG_ARCH_MVEBU) && defined(CONFIG_ARMADA_32BIT) && defined(CONFIG_SPL_BUILD))
> >   	ldr	pc, _undefined_instruction
> >   	ldr	pc, _software_interrupt
> >   	ldr	pc, _prefetch_abort
> > @@ -31,6 +32,7 @@
> >   	ldr	pc, _not_used
> >   	ldr	pc, _irq
> >   	ldr	pc, _fiq
> > +#endif
> >   	.endm
> > @@ -87,6 +89,7 @@ _start:
> >   	ARM_VECTORS
> >   #endif /* !defined(CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK) */
> > +#if !(defined(CONFIG_ARCH_MVEBU) && defined(CONFIG_ARMADA_32BIT) && defined(CONFIG_SPL_BUILD))
> >   /*
> >    *************************************************************************
> >    *
> > @@ -118,6 +121,7 @@ _irq:			.word irq
> >   _fiq:			.word fiq
> >   	.balignl 16,0xdeadbeef
> > +#endif
> >   /*
> >    *************************************************************************
> > @@ -131,6 +135,7 @@ _fiq:			.word fiq
> >   #ifdef CONFIG_SPL_BUILD
> > +#if !(defined(CONFIG_ARCH_MVEBU) && defined(CONFIG_ARMADA_32BIT) && defined(CONFIG_SPL_BUILD))
> >   	.align	5
> >   undefined_instruction:
> >   software_interrupt:
> > @@ -141,6 +146,7 @@ irq:
> >   fiq:
> >   1:
> >   	b	1b			/* hang and never return */
> > +#endif
> >   #else	/* !CONFIG_SPL_BUILD */
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de