[U-Boot] [PATCH] ARM: armv7: Add early stack for erratum workarounds
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com --- arch/arm/cpu/armv7/start.S | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 81edec01bf..0cb6dd39cc 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,6 +205,15 @@ ENTRY(cpu_init_cp15) mov r2, r3, lsl #4 @ shift variant field for combined value orr r2, r4, r2 @ r2 has combined CPU variant + revision
+/* Early stack for ERRATA that needs into call C code */ +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK) + ldr r0, =(CONFIG_SPL_STACK) +#else + ldr r0, =(CONFIG_SYS_INIT_SP_ADDR) +#endif + bic r0, r0, #7 /* 8-byte alignment for ABI compliance */ + mov sp, r0 + #ifdef CONFIG_ARM_ERRATA_798870 cmp r2, #0x30 @ Applies to lower than R3p0 bge skip_errata_798870 @ skip if not affected rev
On Mon, Nov 19, 2018 at 02:47:53PM -0600, Andrew F. Davis wrote:
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com
arch/arm/cpu/armv7/start.S | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 81edec01bf..0cb6dd39cc 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,6 +205,15 @@ ENTRY(cpu_init_cp15) mov r2, r3, lsl #4 @ shift variant field for combined value orr r2, r4, r2 @ r2 has combined CPU variant + revision
+/* Early stack for ERRATA that needs into call C code */ +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
- ldr r0, =(CONFIG_SPL_STACK)
+#else
- ldr r0, =(CONFIG_SYS_INIT_SP_ADDR)
+#endif
- bic r0, r0, #7 /* 8-byte alignment for ABI compliance */
Perhaps use the same comment-style ('@') and alignment like the rest of that file?
Other than that looks good to me. Just wonder how this was noticed just now...
Acked-by: Andreas Dannenberg dannenberg@ti.com
- mov sp, r0
#ifdef CONFIG_ARM_ERRATA_798870 cmp r2, #0x30 @ Applies to lower than R3p0 bge skip_errata_798870 @ skip if not affected rev -- 2.19.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 11/20/18 8:48 AM, Andreas Dannenberg wrote:
On Mon, Nov 19, 2018 at 02:47:53PM -0600, Andrew F. Davis wrote:
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com
arch/arm/cpu/armv7/start.S | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 81edec01bf..0cb6dd39cc 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,6 +205,15 @@ ENTRY(cpu_init_cp15) mov r2, r3, lsl #4 @ shift variant field for combined value orr r2, r4, r2 @ r2 has combined CPU variant + revision
+/* Early stack for ERRATA that needs into call C code */ +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
- ldr r0, =(CONFIG_SPL_STACK)
+#else
- ldr r0, =(CONFIG_SYS_INIT_SP_ADDR)
+#endif
- bic r0, r0, #7 /* 8-byte alignment for ABI compliance */
Perhaps use the same comment-style ('@') and alignment like the rest of that file?
This code was borrowed from crt0.S where this is the comment style, so seems assembly comments are not aligned across U-Boot, but I agree they should be at least within a single file like here.
Other than that looks good to me. Just wonder how this was noticed just now...
I'm wondering that too, took me a while to believe I was the first to hit this and just thought that I was missing something.
The reason is probably that these errata fixes are not enabled for any of our production builds by default. Just now with this Spectre workaround do we enable one by default. Plus on non-HS platforms null pointer dereferences and pushing to a null stack this early only silently drop values it seems, only on HS does a firewall violation alert one of such a problem.
Andrew
Acked-by: Andreas Dannenberg dannenberg@ti.com
- mov sp, r0
#ifdef CONFIG_ARM_ERRATA_798870 cmp r2, #0x30 @ Applies to lower than R3p0 bge skip_errata_798870 @ skip if not affected rev -- 2.19.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 14:47-20181119, Andrew F. Davis wrote:
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com
arch/arm/cpu/armv7/start.S | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 81edec01bf..0cb6dd39cc 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,6 +205,15 @@ ENTRY(cpu_init_cp15) mov r2, r3, lsl #4 @ shift variant field for combined value orr r2, r4, r2 @ r2 has combined CPU variant + revision
+/* Early stack for ERRATA that needs into call C code */ +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
- ldr r0, =(CONFIG_SPL_STACK)
+#else
- ldr r0, =(CONFIG_SYS_INIT_SP_ADDR)
+#endif
- bic r0, r0, #7 /* 8-byte alignment for ABI compliance */
- mov sp, r0
Dumb q: Is this a little too late? bl cpu_init_cp15 gets invoked from reset.. doesnt sp need to be setup prior to the call in?
#ifdef CONFIG_ARM_ERRATA_798870 cmp r2, #0x30 @ Applies to lower than R3p0 bge skip_errata_798870 @ skip if not affected rev -- 2.19.1
On 17:02-20181120, Nishanth Menon wrote:
On 14:47-20181119, Andrew F. Davis wrote:
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com
arch/arm/cpu/armv7/start.S | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 81edec01bf..0cb6dd39cc 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,6 +205,15 @@ ENTRY(cpu_init_cp15) mov r2, r3, lsl #4 @ shift variant field for combined value orr r2, r4, r2 @ r2 has combined CPU variant + revision
+/* Early stack for ERRATA that needs into call C code */ +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
- ldr r0, =(CONFIG_SPL_STACK)
+#else
- ldr r0, =(CONFIG_SYS_INIT_SP_ADDR)
+#endif
- bic r0, r0, #7 /* 8-byte alignment for ABI compliance */
- mov sp, r0
Dumb q: Is this a little too late? bl cpu_init_cp15 gets invoked from reset.. doesnt sp need to be setup prior to the call in?
OK. I guess I will answer my own dumb question -> we don't push till we get here! Sigh.. I should have figured that when i introduced the series!
I guess I introduced the bug in c616a0df297e ("ARM: Introduce erratum workaround for 798870", just a mere 3 years ago :(...
Apologies, but not sure if intermediate stack setup should be something folks prefer earlier or not.. will let ARM folks comment.
Reviewed-by: Nishanth Menon nm@ti.com
On Mon, Nov 19, 2018 at 02:47:53PM -0600, Andrew F. Davis wrote:
Some erratum workarounds call into C code before the stack is setup, this can lead to values pushed onto the stack being lost, firewall exceptions, and other undefined behavior.
Setup a temporary stack to allow these functions to work correctly.
Signed-off-by: Andrew F. Davis afd@ti.com Acked-by: Andreas Dannenberg dannenberg@ti.com Reviewed-by: Nishanth Menon nm@ti.com
Applied to u-boot/master, thanks!
participants (4)
-
Andreas Dannenberg -
Andrew F. Davis -
Nishanth Menon -
Tom Rini