[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Andre Przywara]

Hi Lokesh,

thanks for having a look!

On 02/02/17 05:35, Lokesh Vutla wrote:
> 
> 
> On 2/2/2017 5:18 AM, André Przywara wrote:
>> On 27/01/17 16:39, Andrew F. Davis wrote:
>>
>> Hi,
>>
>>> spl_init on some boards is called after stack and heap relocation, on
>>> some platforms spl_relocate_stack_gd is called to handle setting the
>>> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
>>> SPL malloc is enabled during relocation. spl_init should then not
>>> re-assign the old pre-relocation limit when this is defined.
>>
>> I am sorry to say this, but this very patch breaks SPL boot on sunxi
>> boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
>> SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).
>>
>> U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
>> DRAM: 1024 MiB
>> Trying to boot from MMC1
>> MMC Device 0 not found
>> spl: could not find mmc device. error: -19
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>>
>> sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.
> 
> In your case, before this patch malloc_limit is MALLOC_F_LEN(0x400),
> after this patch malloc_limit is SPL_STACK_R_MALLOC_SIMPLE_LEN(0x100000)
> . Stack relocated address is 0x4fe00000(is this in DDR?). SPL_STACK_R is
> mainly targetted for relocating SPL stack to DDR.

Yes, this is at DDR (which starts at 1G).

> - Is 1MB available at 0x4fe00000?

Yes, the smallest board I have has 256MB, though I've seen this on
boards with one GB as well. But even in the worst case this is 2 MB
below end of DDR.
The BSS is at 0x4ff80000, but that shouldn't conflict then.

Is this address actually the _starting_ address for the malloc heap? So
malloc uses memory above this address, the stack below? Or does malloc
use some stack allocation for its purpose?

> - Can you reduce the size or change SPL_STACK_R to DDR and verify?

I will check tonight (no boards here at the moment).

Thanks for the explanation and the pointers, this gives me something to
debug on.

Cheers,
Andre


> 
> Thanks and regards,
> Lokesh
> 
>>
>> At the moment the code there is beyond me, so I can't really say how
>> this breaks, but it clearly does: reverting this patch makes U-Boot
>> happy again.
>>
>> Any ideas?
>>
>> Cheers,
>> Andre.
>>
>>>
>>> Signed-off-by: Andrew F. Davis <afd at ti.com>
>>> ---
>>>  common/spl/spl.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index 462c3a2b97..a3808a988b 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -183,7 +183,12 @@ int spl_init(void)
>>>  	int ret;
>>>  
>>>  	debug("spl_init()\n");
>>> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
>>> +/*
>>> + * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
>>> + * malloc_limit in spl_relocate_stack_gd
>>> + */
>>> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
>>> +	!defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
>>>  #ifdef CONFIG_MALLOC_F_ADDR
>>>  	gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>>>  #endif
>>>
>>