[U-Boot] [PATCH 4/8] x86: fsp: Enlarge the size of malloc() pool before relocation

[Bin Meng]

Hi Simon,

On Tue, Aug 18, 2015 at 10:18 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 15 August 2015 at 01:07, Bin Meng <bmeng.cn at gmail.com> wrote:
>> After fsp_init() returns, the stack has already been switched to a
>> place within system memory as defined by CONFIG_FSP_TEMP_RAM_ADDR.
>> Enlarge the size of malloc() pool before relocation since we have
>> plenty of memory now.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>  arch/x86/Kconfig              | 7 +++++++
>>  arch/x86/cpu/start.S          | 8 ++++++++
>>  arch/x86/lib/fsp/fsp_common.c | 6 +++++-
>>  3 files changed, 20 insertions(+), 1 deletion(-)
>
> Thanks for explaining the patch.
>
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 065bbe4..687208f 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -232,6 +232,13 @@ config FSP_TEMP_RAM_ADDR
>>           Stack top address which is used in FspInit after DRAM is ready and
>
> (separate point for a possible future patch: FspInit should probably
> change to fsp_init globally)

Will do in v2.

>
>>           CAR is disabled.
>>
>> +config FSP_SYS_MALLOC_F_LEN
>> +       hex
>> +       depends on HAVE_FSP
>> +       default 0x100000
>> +       help
>> +         Additional size of malloc() pool before relocation.
>> +
>>  config SMP
>>         bool "Enable Symmetric Multiprocessing"
>>         default n
>> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
>> index e94ddc4..e2b5ef4 100644
>> --- a/arch/x86/cpu/start.S
>> +++ b/arch/x86/cpu/start.S
>> @@ -141,6 +141,14 @@ car_init_ret:
>>         jz      skip_hob
>>         movl    %esi, GD_HOB_LIST(%edx)
>
> At some point could you please add a comment here about the fact that
> we enter this code twice on boot?
>

Will do in v2.

>>
>> +       /*
>> +        * After fsp_init() returns, the stack has already been switched to a
>> +        * place within system memory as defined by CONFIG_FSP_TEMP_RAM_ADDR.
>> +        * Enlarge the size of malloc() pool before relocation since we have
>> +        * plenty of memory now.
>> +        */
>> +       subl    $CONFIG_FSP_SYS_MALLOC_F_LEN, %esp
>> +       movl    %esp, GD_MALLOC_BASE(%edx)
>>  skip_hob:
>>  #else
>>         /* Store table pointer */
>> diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
>> index 6f72c6d..9c316d4 100644
>> --- a/arch/x86/lib/fsp/fsp_common.c
>> +++ b/arch/x86/lib/fsp/fsp_common.c
>> @@ -56,8 +56,12 @@ void board_final_cleanup(void)
>>
>>  int x86_fsp_init(void)
>>  {
>> -       if (!gd->arch.hob_list)
>> +       if (!gd->arch.hob_list) {
>>                 fsp_init(CONFIG_FSP_TEMP_RAM_ADDR, BOOT_FULL_CONFIG, NULL);
>> +       } else {
>> +               /* We should adjust gd->malloc_limit here */
>> +               gd->malloc_limit += CONFIG_FSP_SYS_MALLOC_F_LEN;
>
> Should this increase the limit by this much, or
> CONFIG_FSP_SYS_MALLOC_F_LEN - CONFIG_SYS_MALLOC_F_LEN?

No, as you see in start.S the esp was subtracted by
CONFIG_FSP_SYS_MALLOC_F_LEN so the limit should be added by
CONFIG_FSP_SYS_MALLOC_F_LEN as well. I will add a more detailed
comments for this in v2.

>
>> +       }
>>
>>         return 0;
>>  }
>> --

Regards,
Bin