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

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

[Pali Rohár]

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.

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@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))
 	/* 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 */
 
-- 
2.20.1

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

[Stefan Roese]

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?

> 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@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).

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

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@denx.de

Re: [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@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@denx.de

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

[Pali Rohár]

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@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
+
 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
-- 
2.20.1

[PATCH u-boot-marvell v2 2/3] arm: Do not compile vector table when SYS_NO_VECTOR_TABLE is enabled

[Pali Rohár]

Vector table is not used when SYS_NO_VECTOR_TABLE is enabled.
So do not compile it and reduce image size.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/lib/vectors.S | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index 56f36815582b..a54c84b062b1 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -24,6 +24,7 @@
 #else
 	b	reset
 #endif
+#if !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
 	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 !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
 /*
  *************************************************************************
  *
@@ -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 !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
 	.align	5
 undefined_instruction:
 software_interrupt:
@@ -141,6 +146,7 @@ irq:
 fiq:
 1:
 	b	1b			/* hang and never return */
+#endif
 
 #else	/* !CONFIG_SPL_BUILD */
 
-- 
2.20.1

[PATCH u-boot-marvell v2 3/3] arm: mvebu: Enable CONFIG_SPL_SYS_NO_VECTOR_TABLE for 32-bit mvebu

[Pali Rohár]

U-Boot SPL is on 32-bit mvebu executed by the BootROM. And BootROM expects
that U-Boot SPL returns execution back to the BootROM. Vectors during
execution of U-Boot SPL should not be changed as BootROM does not expect it
and uses its own vectors. So do not overwrite vectors in SPL build.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index fa83ae712eb1..c8e792306872 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -16,6 +16,7 @@ config ARMADA_32BIT
 	select SUPPORT_SPL
 	select TRANSLATION_OFFSET
 	select TOOLS_KWBIMAGE if SPL
+	select SPL_SYS_NO_VECTOR_TABLE if SPL
 
 config ARMADA_64BIT
 	bool
-- 
2.20.1

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

[Stefan Roese]

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@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   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
> +
>   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

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@denx.de

Re: [PATCH u-boot-marvell v2 2/3] arm: Do not compile vector table when SYS_NO_VECTOR_TABLE is enabled

[Stefan Roese]

On 4/6/22 16:20, Pali Rohár wrote:
> Vector table is not used when SYS_NO_VECTOR_TABLE is enabled.
> So do not compile it and reduce image size.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   arch/arm/lib/vectors.S | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 56f36815582b..a54c84b062b1 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -24,6 +24,7 @@
>   #else
>   	b	reset
>   #endif
> +#if !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
>   	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 !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
>   /*
>    *************************************************************************
>    *
> @@ -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 !CONFIG_IS_ENABLED(SYS_NO_VECTOR_TABLE)
>   	.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@denx.de

Re: [PATCH u-boot-marvell v2 3/3] arm: mvebu: Enable CONFIG_SPL_SYS_NO_VECTOR_TABLE for 32-bit mvebu

[Stefan Roese]

On 4/6/22 16:20, Pali Rohár wrote:
> U-Boot SPL is on 32-bit mvebu executed by the BootROM. And BootROM expects
> that U-Boot SPL returns execution back to the BootROM. Vectors during
> execution of U-Boot SPL should not be changed as BootROM does not expect it
> and uses its own vectors. So do not overwrite vectors in SPL build.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index fa83ae712eb1..c8e792306872 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -16,6 +16,7 @@ config ARMADA_32BIT
>   	select SUPPORT_SPL
>   	select TRANSLATION_OFFSET
>   	select TOOLS_KWBIMAGE if SPL
> +	select SPL_SYS_NO_VECTOR_TABLE if SPL
>   
>   config ARMADA_64BIT
>   	bool

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@denx.de

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

[Heinrich Schuchardt]

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@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?

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

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

Re: [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@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
>