Skip to contentSkip to conversation

nios2 - adjust gbl data off to account for bd_info

PATCH New Alex Hornung 4 participants 6 messages Feb 20, 2012 – Feb 24, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

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

---
 include/configs/nios2-generic.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
include/configs/nios2-generic.h +2 -1
index 9ba35e8..17017a5 100644
@@ -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
 
 /*
Signature · 1 line
1.7.3.4
#1 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

Hi Alex,

Quoted 17 lines · Alex Hornung

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

---
 include/configs/nios2-generic.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
include/configs/nios2-generic.h +0 -0
index 9ba35e8..17017a5 100644

include/configs/nios2-generic.h line 138

Quoted 9 lines
include/configs/nios2-generic.h +2 -1
@@ -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

#2 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

Hi Graeme,

Quoted 53 lines · Graeme Russ

Hi Alex,

Quoted 17 lines · Alex Hornung

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

---
 include/configs/nios2-generic.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
include/configs/nios2-generic.h +0 -0
index 9ba35e8..17017a5 100644

include/configs/nios2-generic.h line 138

Quoted 9 lines
include/configs/nios2-generic.h +2 -1
@@ -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?

Quoted 8 lines

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.

Quoted 3 lines

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

#3 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

Hi Alex,

Quoted 18 lines · Alex Hornung

Hi Graeme,

Quoted 15 lines · Graeme Russ

Hi Alex,

Quoted 12 lines · Alex Hornung

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

---
 include/configs/nios2-generic.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

[snip]

Quoted 6 lines
Quoted 5 lines

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

Quoted 8 lines

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

#4 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

In reply to Alex Hornung
Quoted 17 lines · 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.

---
 include/configs/nios2-generic.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/include/configs/nios2-generic.h index 9ba35e8..17017a5 100644

include/configs/nios2-generic.h line 138

Quoted 9 lines
include/configs/nios2-generic.h +2 -1
@@ -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>

Applied
#5 lore raw

PATCH 1/1 nios2 - adjust gbl data off to account for bd_info

In reply to Alex Hornung
Quoted 4 lines · Alex Hornung

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

Quoted 8 lines

* This fixes a memory corruption bug due to overlap of the malloc region and the bd_info structure.

---
  include/configs/nios2-generic.h |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

Dear Alex,

Applied. Thanks.

Best regards,
Thomas