[U-Boot] [PATCH 3/4] ARM: renesas: Save boot parameters passed in by ATF

[Marek Vasut]

On 3/12/19 9:29 PM, Eugeniu Rosca wrote:
> Hi Tom,

Hi,

> On Tue, Mar 12, 2019 at 03:42:24PM -0400, Tom Rini wrote:
>> On Tue, Mar 12, 2019 at 07:59:40PM +0100, Eugeniu Rosca wrote:
>>> Hi Marek,
>>>
>>> On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>>> [..]
>>>> +.align 8
>>>> +.globl rcar_atf_boot_args
>>>> +rcar_atf_boot_args:
>>>> +       .dword 0
>>>> +       .dword 0
>>>> +       .dword 0
>>>> +       .dword 0
>>>> +
>>>> +ENTRY(save_boot_params)
>>>> +       adr     x8, rcar_atf_boot_args
>>>> +       stp     x0, x1, [x8], #16
>>>> +       stp     x2, x3, [x8], #16
>>>> +       b       save_boot_params_ret
>>>> +ENDPROC(save_boot_params)
>>>
>>> What about relocating the function to C like in [1] and passing the 4
>>> arguments to it from ASM like in [2]?
>>> This would allow to:
>>>  - reduce asm operations to minimum (mov and bl) and make code transparent.
>>>  - avoid custom globals and store the ATF information in some C-defined struct.
>>>
>>> [1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/bl1/bl1_main.c#L263
>>> [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/bl1/aarch32/bl1_exceptions.S#L133
>>
>> This is super early in the boot process and I'm really not a fan in
>> general of writing and calling C before we have things setup for C calls
>> to actually work normally, even more so for small things that can be
>> commented to be obvious as to what's going on.
> 
> I don't have a strong preference about that. It is just my experience
> that Renesas once rewrote the elaborate Bosch-contributed BL2 routine
> of enabling the cntfrq_el0 system counter from ASM [1] to C [2].

At that point, BL2 already has a C runtime set up.

This code is instead running without the C runtime and we want to make
sure the correct registers get stored at the correct location, without
the compiler interfering with it in any way. Keep in mind this code runs
right at the beginning of U-Boot execution to make sure registers used
to pass parameters stay intact (which I believe is what Tom meant by
"early in the boot process").

> This
> is at the BL1-BL2 boundary, while we are still in EL3, so much earlier
> in the boot process. This implementation currently runs on the targets
> of our customers.
> 
> I am fine with any working solution. Thanks for commenting!
> 
> [1] https://github.com/renesas-rcar/arm-trusted-firmware/commit/e23524689abb637b14a2961a305f6cfb43909319
> [2] https://github.com/renesas-rcar/arm-trusted-firmware/commit/867d84191319#diff-471251fb86bd634ba14891d950c1440eR177
> 
>> -- 
>> Tom
> 
> Best regards,
> Eugeniu.
> 


-- 
Best regards,
Marek Vasut