[PATCH] common/board_f: init malloc earlier

[Caleb Connolly]

Hi Simon,

On 26/11/2024 01:32, Simon Glass wrote:
> Hi Caleb,
> 
> On Sun, 24 Nov 2024 at 11:38, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>> Currently the early malloc initialisation is done partially in
>> board_init_f_init_reserve() (on arm64 at least), which configures
>> gd->malloc_base. But it isn't actually usable until initf_malloc() is
>> called which doesn't happen until after fdtdec_setup().
>>
>> This causes problems in a few scenarios:
>>
>> 1. when using MULTI_DTB_FIT as this needs a working malloc (especially
>>    for compressed FIT).
> 
> Hmmm, how does this work today?

I honestly have no idea, I assume boards that make use of it do some
custom board_f.
> 
>> 2. Some platforms may need to allocate memory as part of memory map
>>    initialisation (e.g. Qualcomm will need this to parse the memory map
>>    from SMEM).
> 
> That really needs to be tidied up. When does this fixup need to be
> done? I imagine it is somewhere prior to setup_dest_addr() ? Perhaps

It's necessary in order to figure out gd->ram_base/ram_size. We do it in
board_fdt_blob_setup() so that we can support another use-case where
U-Boot has an internal FDT (which it should use) but the memory layout
is provided via an external FDT which is unavailable (although this
usecase isn't enabled upstream yet).

> we could introduce an event to do 'memory map' stuff?

We could, but considering that on most platforms the pre-relocation
malloc is fixed at build time and at a known location relative to U-Boot
there is no reason for us to arbitrary decide that some codepaths at the
start of board_f aren't allowed to use malloc().

Just moving the malloc initialization earlier ensures malloc() is
consistently available and greatly simplifies things.

fwiw, I had another variation of this patch which dropped initf_malloc()
entirely and just set up gd->malloc_limit/malloc_ptr in
board_init_f_init_reserve() since that's where we set malloc_base
anyways. But after digging some more there seem to be quite a few other
entrypoints into U-Boot that don't go through
board_init_f_init_reserve() (e.g. sandbox, EFI app) and it seemed more
error prone to duplicate the implementation there.

Kind regards

> 
> Regards,
> Simon
> 
> 
>>
>> Move the initf_malloc() call earlier so that malloc is available during
>> fdtdec_setup().
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>>  common/board_f.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index 98dc2591e1d0..bddfa6b992b9 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -867,15 +867,15 @@ static int initf_upl(void)
>>  }
>>
>>  static const init_fnc_t init_sequence_f[] = {
>>         setup_mon_len,
>> +       initf_malloc,
>>  #ifdef CONFIG_OF_CONTROL
>>         fdtdec_setup,
>>  #endif
>>  #ifdef CONFIG_TRACE_EARLY
>>         trace_early_init,
>>  #endif
>> -       initf_malloc,
>>         initf_upl,
>>         log_init,
>>         initf_bootstage,        /* uses its own timer, so does not need DM */
>>         event_init,
>> --
>> 2.47.0
>>

-- 
// Caleb (they/them)