[U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info

[Alex Hornung]

Hi Graeme,

On 21/02/12 10:33, Graeme Russ wrote:
> Hi Alex,
> 
> On 02/21/2012 10:45 AM, Alex Hornung wrote:
>>  * Adjust the GBL_DATA_OFFSET to account for the fact that we'll be
>>    using the space between that offset and the start of the malloc
>>    region to allocate both a gd structure and a board info structure.
>>
>>  * This fixes a memory corruption bug due to overlap of the malloc
>>    region and the bd_info structure.
>>
>> Signed-off-by: Alex Hornung <alex at alexhornung.com>
>> ---
>>  include/configs/nios2-generic.h |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h
>> index 9ba35e8..17017a5 100644
>> --- a/include/configs/nios2-generic.h
>> +++ b/include/configs/nios2-generic.h
>> @@ -131,7 +131,8 @@
>>  #define CONFIG_SYS_MALLOC_BASE		(CONFIG_SYS_MONITOR_BASE - \
>>  					 CONFIG_SYS_MALLOC_LEN)
>>  #define CONFIG_SYS_GBL_DATA_OFFSET	(CONFIG_SYS_MALLOC_BASE - \
>> -					 GENERATED_GBL_DATA_SIZE)
>> +					 GENERATED_GBL_DATA_SIZE - \
>> +					 GENERATED_BD_INFO_SIZE)
>>  #define CONFIG_SYS_INIT_SP		CONFIG_SYS_GBL_DATA_OFFSET
>>  
>>  /*
> 
> Oh I really want to NAK this, but I think I might be out-voted
> 
> in arch/x86/lib/init_helpers.c I do:
> 
> bd_t bd_data;
> 
> int init_bd_struct_r(void)
> {
> 	gd->bd = &bd_data;
> 	memset(gd->bd, 0, sizeof(bd_t));
> 
> 	return 0;
> }
> 
> So the bd struct is in bss
> 
> But everyone else (ARM, PPC) seems to do:
> 
> 	addr_sp -= sizeof(bd_t);
> 	bd = (bd_t *) addr_sp;
> 
> Why?
> 
> Well I see that a lot of bd is setup while still running from flash, so bss
> is not available. But is this really necessary?
> 
> And for the sake of one more copy and pointer adjustment, even gd could be
> moved into bss which would save all the hackish calculations of sticking gd
> and bd in a magic void below the stack
> 
> But maybe that's a battle for another day...

To be honest I just started using u-boot, and for that matter, just
started using Nios2. I just ran across the issue and had to fix it to
get u-boot working on my system.

Nonetheless I'd generally agree that less hackery is a better idea, and
putting it in BSS is a good option - but reality is, as you point out,
that most other platforms don't do that, either. For now I think the
above patch should do.

> Oh, and Nios is particularly nasty - It has no init_r / init_f so there is
> no support for relocation - Will there every be an intent to enable
> relocation for Nios or is it pointless?

As I mentioned I'm not really involved with u-boot on Nios, so I can't
answer that question.


Cheers,
Alex