[U-Boot] [PATCH] ARM: armv7: Add early stack for erratum workarounds

[Andrew F. Davis]

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 at 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 at 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 at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot