Hi Simon,
On 26/11/2024 01:32, Simon Glass wrote:
> Hi Caleb,
>
> On Sun, 24 Nov 2024 at 11:38, Caleb Connolly <caleb.conno...@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.conno...@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)
