From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Chou <thomas@wytron.com.tw>
Date: Sun, 15 Nov 2015 10:11:02 +0800
Subject: [U-Boot] [PATCH v3] Fix board init code to use a valid C
 runtime environment
In-Reply-To: <20151114160608.3cda262c@lilith>
References: <1447425803-24281-1-git-send-email-albert.u.boot@aribaud.net>
 <5646D363.6000404@wytron.com.tw> <20151114160608.3cda262c@lilith>
Message-ID: <5647E9B6.3050404@wytron.com.tw>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Albert,

On 2015?11?14? 23:06, Albert ARIBAUD wrote:
> Hello Thomas,
>
> On Sat, 14 Nov 2015 14:23:31 +0800, Thomas Chou <thomas@wytron.com.tw>
> wrote:
>> Hi Albert,
>>
>> On 2015?11?13? 22:43, Albert ARIBAUD wrote:
>>> +/*
>>> + * Initialize reserved space (which has been safely allocated on the C
>>> + * stack from the C runtime environment handling code).
>>> + *
>>> + * Do not use 'gd->' until arch_setup_gd() has been called!
>>> + */
>>> +
>>> +void board_init_f_init_reserve(ulong base)
>>>    {
>>>    	struct global_data *gd_ptr;
>>>    #ifndef _USE_MEMCPY
>>>    	int *ptr;
>>>    #endif
>>>
>>> -	/* Leave space for the stack we are running with now */
>>> -	top -= 0x40;
>>> +	/*
>>> +	 * clear GD entirely
>>> +	 */
>>>
>>> -	top -= sizeof(struct global_data);
>>> -	top = ALIGN(top, 16);
>>> -	gd_ptr = (struct global_data *)top;
>>> +	gd_ptr = (struct global_data *)base;
>>> +	/* go down one GD plus 16-byte alignment */
>>> +	base += roundup(sizeof(struct global_data), 16);
>>
>> Maybe it can keep the original order, top--gd--malloc--base.
>
> Actually, I inverted the two between v2 and v3 for a reason, but I
> should have made it more explicit.
>
> This reason is, for architectures which may not be able to write to
> gd from arch_setup_gd(), the C runtime environment handling code (crt0.S
> for ARM, etc) needs a way to easily find our where gd_ptr was allocated.
>
> So, it was either:
>
> - keeping *gd_ptr above the malloc arena and having to pass gd_ptr back
>    to board_init_f_reserve_size's caller; but board_init_f_reserve_size
>    already returns a value back to its caller, and returning two values
>    from a C function takes resources (a pointer argument and a memory
>    location at least);
>
> or:
>
> - putting *gd_ptr at the base of the allocated space, below the malloc
>    arena, so that the C runtime environment handling code knows where to
>    point gd to without incurring the cost of passing additional results
>    back from board_init_f_reserve_init.
>
> This is the reason why I placed the global data below the malloc arena.
>
> [besides, considering that our malloc implementation starts from the
> base of the arena and allocates upward, it is (admittedly very slightly)
> safer to have GD placed below it than above.]

Agree.

>
> I will add a note in v4 about the reason for this (re)ordering of
> allocations.
>
>>> +	base += CONFIG_SYS_MALLOC_F_LEN;
>>
>> Useless statement.
>
> Right now it is indeed useless. However, if and when some other space
> gets reserved and initialized besides GD and the malloc arena, this
> statement will be needed before allocating / accessing the space
> allocated above the malloc arena.
>
> Note that this statement will not result in any useless *code*, as the
> compiler's optimizer detects that base remains unused after it, and
> thus removes it from the generated object code.
>
> So I'll insist on keeping this statement, although I will add a comment
> next to it explaining why it matters despite not having any visible
> effect.
>

OK.

Best regards,
Thomas