[U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Thomas,

On Sat, 14 Nov 2015 14:23:31 +0800, Thomas Chou <thomas at 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.]

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.

> Best regards,
> Thomas

Amicalement,
-- 
Albert.