[U-Boot] [PATCH] board_f.c: Insure gd->new_bootstage alignment

[Patrice CHOTARD]

Hi Heinrich

On 11/26/19 6:41 PM, Heinrich Schuchardt wrote:
> On 11/26/19 1:16 PM, Patrick DELAUNAY wrote:
>> Hi,
>>
>>> From: Patrice CHOTARD <patrice.chotard at st.com>
>>> Sent: lundi 25 novembre 2019 14:48
>>>
>>> In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In
>>> bootstage_relocate(), the platform hangs when getting access to data-
>>>> record[i].name.
>>> To avoid this issue, make gd->new_bootstage 16 byte aligned.
>>>
>>> Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm")
>>>
>>> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
>>> Reviewed-by: Vikas MANOCHA <vikas.manocha at st.com>
>>> ---
>>
>> This patch is mandatory for stm32mp1 (ARM plaform with bootstage feature activated).
>>
>> Without this patch, the boot failed for v2020.01-rc3 (crash has struct pointer new_bootstage is not aligned).
>>
>> Reviewed-by: Patrick Delaunay <patrick.delaunay at st.com>
>> Tested-by: Patrick Delaunay <patrick.delaunay at st.com>
>>
>>>   common/board_f.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/common/board_f.c b/common/board_f.c index e3591cbaeb..9cd4cfaf5f
>>> 100644
>>> --- a/common/board_f.c
>>> +++ b/common/board_f.c
>>> @@ -559,6 +559,7 @@ static int reserve_bootstage(void)
>>>       int size = bootstage_get_size();
>>>
>>>       gd->start_addr_sp -= size;
>>> +    gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16);
>
> A comment in the code describing why this ALIGN_DOWN() is done would be
> helpful.

Ok i will add a comment about that.


>
> Is this value sufficient for all boards?

I simply reuse the same alignment already applied for other memory area :

    _ start_addr_sp in reserve_stacks()

    _ new_bloblist in reserve_bloblist()


Thanks

Patrice


>
> I found references for:
>
> x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes
> ARMv8: ARMv8 Instruction Set Overview: quad word, 16 bytes
>
> Best regards
>
> Heinrich
>
>>>       gd->new_bootstage = map_sysmem(gd->start_addr_sp, size);
>>>       debug("Reserving %#x Bytes for bootstage at: %08lx\n", size,
>>>             gd->start_addr_sp);
>>> -- 
>>> 2.17.1
>>
>> Thanks
>>
>> Patrick
>>
>