All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
@ 2012-02-20 23:45 Alex Hornung
  2012-02-21 10:33 ` Graeme Russ
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Alex Hornung @ 2012-02-20 23:45 UTC (permalink / raw)
  To: u-boot

 * 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@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
 
 /*
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
  2012-02-20 23:45 [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info Alex Hornung
@ 2012-02-21 10:33 ` Graeme Russ
  2012-02-23  8:13   ` Alex Hornung
  2012-02-23 23:54 ` Mike Frysinger
  2012-02-24  3:03 ` Thomas Chou
  2 siblings, 1 reply; 6+ messages in thread
From: Graeme Russ @ 2012-02-21 10:33 UTC (permalink / raw)
  To: u-boot

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@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...

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?

Regards,

Graeme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
  2012-02-21 10:33 ` Graeme Russ
@ 2012-02-23  8:13   ` Alex Hornung
  2012-02-23  8:49     ` Graeme Russ
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Hornung @ 2012-02-23  8:13 UTC (permalink / raw)
  To: u-boot

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@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
  2012-02-23  8:13   ` Alex Hornung
@ 2012-02-23  8:49     ` Graeme Russ
  0 siblings, 0 replies; 6+ messages in thread
From: Graeme Russ @ 2012-02-23  8:49 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On 02/23/2012 07:13 PM, Alex Hornung wrote:
> 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@alexhornung.com>
>>> ---
>>>  include/configs/nios2-generic.h |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>

[snip]

>> 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.

I agree - as I said, a battle for another day :)

Regards,

Graeme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
  2012-02-20 23:45 [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info Alex Hornung
  2012-02-21 10:33 ` Graeme Russ
@ 2012-02-23 23:54 ` Mike Frysinger
  2012-02-24  3:03 ` Thomas Chou
  2 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2012-02-23 23:54 UTC (permalink / raw)
  To: u-boot

On Monday 20 February 2012 18:45:45 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@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
> 
>  /*

+cc the nios maintainers ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120223/c7f4321d/attachment.pgp>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info
  2012-02-20 23:45 [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info Alex Hornung
  2012-02-21 10:33 ` Graeme Russ
  2012-02-23 23:54 ` Mike Frysinger
@ 2012-02-24  3:03 ` Thomas Chou
  2 siblings, 0 replies; 6+ messages in thread
From: Thomas Chou @ 2012-02-24  3:03 UTC (permalink / raw)
  To: u-boot

On 02/21/2012 07: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@alexhornung.com>
> ---
>   include/configs/nios2-generic.h |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>

Dear Alex,

Applied. Thanks.

Best regards,
Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-24  3:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-20 23:45 [U-Boot] [PATCH 1/1] nios2 - adjust gbl data off to account for bd_info Alex Hornung
2012-02-21 10:33 ` Graeme Russ
2012-02-23  8:13   ` Alex Hornung
2012-02-23  8:49     ` Graeme Russ
2012-02-23 23:54 ` Mike Frysinger
2012-02-24  3:03 ` Thomas Chou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.