[PATCH] arm/start.S: Do not use a scratch register to hold the stack pointer

[Chris Johns]

On 26/7/19 3:42 pm, Sebastian Huber wrote:
> On 26/07/2019 07:37, Chris Johns wrote:
>> On 26/7/19 3:06 pm, Sebastian Huber wrote:
>>> Hello Chris,
>>>
>>> I am not sure, if using r8 is the right thing to do since r8..r14 are banked in
>>> FIQ mode.
>>
>> The ARM docs I referenced say the register is general purpose. There is other
>> docs they say something else?
> 
> This is a standard ARM feature, see for example "B1.3.2
> ARM core registers" in the ARMv7-AR reference manual.

Thanks.

>>
>> This is early boot code who why would FIQ banks be an issue? Which BSPs would
>> that effect?
> 
> You set the stack in FIQ mode in start.S:
> 
>     /* Initialize stack pointer registers for the various modes */
> 
>     /* Enter FIQ mode and set up the FIQ stack pointer */
>     mov    r0, #(ARM_PSR_M_FIQ | ARM_PSR_I | ARM_PSR_F)
>     msr    cpsr, r0
>     ldr    r1, =bsp_stack_fiq_size
>     mov    sp, r8
>     sub    r8, r8, r1
> 

That can worked around locally by moving r8 to r3 until SVC mode is entered.

> #ifdef BSP_START_NEEDS_REGISTER_INITIALIZATION
>     bl bsp_start_init_registers_banked_fiq
> #endif
> 
>>
>>> I think the bsp_start_arm_drop_hyp_mode needs to be changed to not
>>> touch r3, it can use r1 instead.
>>
>> That is a bad idea. The registers are documented as scratch and it's use here
>> across a call is wrong.
> 
> We are here in the low level startup code, so we have to be a bit flexible if it
> comes to the ABI. We should move these low level functions to start.S.

It depends. If you call other external functions it is good practice to observe
the ABI or things become fragile and what seems like a pretty straight forward
commit breaks things. :)

If the call is local and in sight then there is a case that can be argued.

Chris