[U-Boot] memory corruption on nios2 due to overlap of gbl data and malloc
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET; [...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
In other words, bd and the malloc base overlap, causing memory corruption in some of the malloc'd memory when some bd elements are populated. In my case in particular some content of the flash mtd eraseregions is getting corrupted by the write to bd->bi_ip_addr after initializing the flash stuff.
I'm not sure how this should be dealt with - I'd think the best approach here is to change the CONFIG_SYS_GBL_DATA_OFFSET to include some space for the bd, or malloc'ing the bd.
If you let me know which one is the preferred approach, I'll gladly provide a patch.
Cheers, Alex Hornung
Hi Alex,
Le 21/02/2012 00:24, Alex Hornung a écrit :
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET;[...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd and bd, which I suspect is not the case; but if it is supposed to only contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in that it does not account for gd and bd as it should.
(BTW, what makes GENERATED_GBL_DATA_SIZE different from sizeof(gd_t)?)
In other words, bd and the malloc base overlap, causing memory corruption in some of the malloc'd memory when some bd elements are populated. In my case in particular some content of the flash mtd eraseregions is getting corrupted by the write to bd->bi_ip_addr after initializing the flash stuff.
I'm not sure how this should be dealt with - I'd think the best approach here is to change the CONFIG_SYS_GBL_DATA_OFFSET to include some space for the bd, or malloc'ing the bd.
If you let me know which one is the preferred approach, I'll gladly provide a patch.
Hmm... You have sizeof(bd_t) available, so you could do something like
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ sizeof(bd_t) - \
GENERATED_GBL_DATA_SIZE)
That would ensure you have space available for a gd and bd.
Amicalement,
Hi Albert,
On Wed, Feb 29, 2012 at 9:29 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Alex,
Le 21/02/2012 00:24, Alex Hornung a écrit :
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET; [...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd and bd, which I suspect is not the case; but if it is supposed to only contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in that it does not account for gd and bd as it should.
The global data struct only contains a pointer to the board data struct.
IMHO I think the approach taken (but almost all arches) is very errror prone as it relies on manually laying out gd and bd in memory with bd sitting immediately above or below gd. In theory, this layout should never be tampered with, but I still don't like it.
For x86, gd and bd are in BSS after relocation, so there is no need to hack around them when calculating the heap or stack, but I have a sneaking suspicion that this could make debugging harder as there is no way to reliably find the relocation offset as gd is never located at a known location in memory...
Regards,
Graeme
Hi Graeme,
Le 28/02/2012 23:39, Graeme Russ a écrit :
Hi Albert,
On Wed, Feb 29, 2012 at 9:29 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Alex,
Le 21/02/2012 00:24, Alex Hornung a écrit :
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET;[...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd and bd, which I suspect is not the case; but if it is supposed to only contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in that it does not account for gd and bd as it should.
The global data struct only contains a pointer to the board data struct.
IMHO I think the approach taken (but almost all arches) is very errror prone as it relies on manually laying out gd and bd in memory with bd sitting immediately above or below gd. In theory, this layout should never be tampered with, but I still don't like it.
For x86, gd and bd are in BSS after relocation, so there is no need to hack around them when calculating the heap or stack, but I have a sneaking suspicion that this could make debugging harder as there is no way to reliably find the relocation offset as gd is never located at a known location in memory...
Duh. I had misread the code... Time for me to go to sleep. :/
For ARM we have gd in r8, which makes things simpler.
Anyway -- this does not affect the fact that GENERATED_GBL_DATA_SIZE should be equal to or greater than sizeof(gd_t)+sizeof(bd-t), right?
Regards,
Graeme
Amicalement,
Hi Albert,
On Wed, Feb 29, 2012 at 9:55 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Graeme,
Le 28/02/2012 23:39, Graeme Russ a écrit :
Hi Albert,
On Wed, Feb 29, 2012 at 9:29 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Alex,
Le 21/02/2012 00:24, Alex Hornung a écrit :
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET; [...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd and bd, which I suspect is not the case; but if it is supposed to only contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in that it does not account for gd and bd as it should.
The global data struct only contains a pointer to the board data struct.
IMHO I think the approach taken (but almost all arches) is very errror prone as it relies on manually laying out gd and bd in memory with bd sitting immediately above or below gd. In theory, this layout should never be tampered with, but I still don't like it.
For x86, gd and bd are in BSS after relocation, so there is no need to hack around them when calculating the heap or stack, but I have a sneaking suspicion that this could make debugging harder as there is no way to reliably find the relocation offset as gd is never located at a known location in memory...
Duh. I had misread the code... Time for me to go to sleep. :/
For ARM we have gd in r8, which makes things simpler.
Of course :) - x86 now has it in FS so it should be easy to find
Anyway -- this does not affect the fact that GENERATED_GBL_DATA_SIZE should be equal to or greater than sizeof(gd_t)+sizeof(bd-t), right?
No - GENERATED_GBL_DATA_SIZE should be sizeof(gd_t)
The space reserved between U-Boot and the heap needs to be sizeof(gd_t) + sizeof(bd-t) (on the delicate proviso that only gd and bd live there, and that gd and bd are immediately next to each other)
Regards,
Graeme
Le 29/02/2012 00:20, Graeme Russ a écrit :
Hi Albert,
On Wed, Feb 29, 2012 at 9:55 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Graeme,
Le 28/02/2012 23:39, Graeme Russ a écrit :
Hi Albert,
On Wed, Feb 29, 2012 at 9:29 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Alex,
Le 21/02/2012 00:24, Alex Hornung a écrit :
Hi,
I've run into some memory corruption due to an error in the logic used to allocate the bd (and gd) during board_init of the nios2.
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_GBL_DATA_SIZE) [...]
gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET;[...] gd->bd = (bd_t *)(gd+1); /* At end of global data */ [...] mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
Given that gd is 68 bytes big, now the start of bd is only 12 bytes from the beginning of the malloc base - but the size of bd is 36 bytes!
So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd and bd, which I suspect is not the case; but if it is supposed to only contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in that it does not account for gd and bd as it should.
The global data struct only contains a pointer to the board data struct.
IMHO I think the approach taken (but almost all arches) is very errror prone as it relies on manually laying out gd and bd in memory with bd sitting immediately above or below gd. In theory, this layout should never be tampered with, but I still don't like it.
For x86, gd and bd are in BSS after relocation, so there is no need to hack around them when calculating the heap or stack, but I have a sneaking suspicion that this could make debugging harder as there is no way to reliably find the relocation offset as gd is never located at a known location in memory...
Duh. I had misread the code... Time for me to go to sleep. :/
For ARM we have gd in r8, which makes things simpler.
Of course :) - x86 now has it in FS so it should be easy to find
Anyway -- this does not affect the fact that GENERATED_GBL_DATA_SIZE should be equal to or greater than sizeof(gd_t)+sizeof(bd-t), right?
No - GENERATED_GBL_DATA_SIZE should be sizeof(gd_t)
The space reserved between U-Boot and the heap needs to be sizeof(gd_t) + sizeof(bd-t) (on the delicate proviso that only gd and bd live there, and that gd and bd are immediately next to each other)
Ok, so :
1. do you know why here gd = 68 bytes and GENERATED_GBL_DATA_SIZE is 80?
2. luckily for my ego, my proposal was actually correct when I suggested the following, right?
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ sizeof(bd_t) - \ GENERATED_GBL_DATA_SIZE)
Regards,
Graeme
Amicalement,
Hi Albert,
On Wed, Feb 29, 2012 at 10:24 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Le 29/02/2012 00:20, Graeme Russ a écrit :
Hi Albert,
No - GENERATED_GBL_DATA_SIZE should be sizeof(gd_t)
The space reserved between U-Boot and the heap needs to be sizeof(gd_t) + sizeof(bd-t) (on the delicate proviso that only gd and bd live there, and that gd and bd are immediately next to each other)
Ok, so :
- do you know why here gd = 68 bytes and GENERATED_GBL_DATA_SIZE is 80?
It gets padded:
/* Round up to make sure size gives nice stack alignment */ DEFINE(GENERATED_GBL_DATA_SIZE, (sizeof(struct global_data) + 15) & ~15);
DEFINE(GENERATED_BD_INFO_SIZE, (sizeof(struct bd_info) + 15) & ~15);
- luckily for my ego, my proposal was actually correct when I suggested the
following, right?
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ sizeof(bd_t) - \ GENERATED_GBL_DATA_SIZE)
Probably, but I'm really not sure...
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
So no, your calculation is not right - It should be:
#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_MALLOC_BASE - \ GENERATED_BD_INFO_SIZE - \ GENERATED_GBL_DATA_SIZE)
Regards,
Graeme
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout: http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-layou... -mike
Hi Mike,
On Thu, Mar 1, 2012 at 6:04 AM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout: http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-layou...
I had a look at this and noticed that you statically allocate locations for gd and bd (CONFIG_SYS_GBL_DATA_ADDR, CONFIG_SYS_BD_INFO_ADDR)
Considering that:
a) the gd pointer is in a register (P3) and thus easily locatable by a debugger, and; b) the bd pointer is in gd
Is there any reason not to have gd and bd in BSS?
Regards,
Graeme
On Wednesday 29 February 2012 17:22:26 Graeme Russ wrote:
On Thu, Mar 1, 2012 at 6:04 AM, Mike Frysinger wrote:
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout: http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-la yout
I had a look at this and noticed that you statically allocate locations for gd and bd (CONFIG_SYS_GBL_DATA_ADDR, CONFIG_SYS_BD_INFO_ADDR)
Considering that:
a) the gd pointer is in a register (P3) and thus easily locatable by a debugger, and; b) the bd pointer is in gd
Is there any reason not to have gd and bd in BSS?
in the Blackfin case, most likely not. we don't do relocation, and the bss is cleared long before board_init_f() gets called. the only reason for allowing the config to override would be if someone wanted to put gd/bd into on-chip L1 data, but i can't imagine this structure being performance critical enough to warrant that. -mike
Hi Mike,
On Thu, Mar 1, 2012 at 9:29 AM, Mike Frysinger vapier@gentoo.org wrote:
On Wednesday 29 February 2012 17:22:26 Graeme Russ wrote:
On Thu, Mar 1, 2012 at 6:04 AM, Mike Frysinger wrote:
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout: http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-la yout
I had a look at this and noticed that you statically allocate locations for gd and bd (CONFIG_SYS_GBL_DATA_ADDR, CONFIG_SYS_BD_INFO_ADDR)
Considering that:
a) the gd pointer is in a register (P3) and thus easily locatable by a debugger, and; b) the bd pointer is in gd
Is there any reason not to have gd and bd in BSS?
in the Blackfin case, most likely not. we don't do relocation, and the bss is cleared long before board_init_f() gets called. the only reason for allowing the config to override would be if someone wanted to put gd/bd into on-chip L1 data, but i can't imagine this structure being performance critical enough to warrant that.
I thought as much - I moved gd/bd into BSS for x86 without really thinking about why everyone else calculates the location of these data structures around the stack and heap. The longer I think about it, the more I think that it was not a bad move and that maybe other arches can follow suit as part of standardising the init sequences
Regards,
Graeme
As suggested by Graeme Russ, move gd and bd data structrures to BSS instead of calculating the locations around the stack and heap.
CC: Graeme Russ graeme.russ@gmail.com CC: Mike Frysinger vapier@gentoo.org CC: Alex Hornung alex@alexhornung.com Signed-off-by: Thomas Chou thomas@wytron.com.tw --- For u-boot.
arch/nios2/lib/board.c | 13 ++++++------- include/configs/nios2-generic.h | 8 ++------ 2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/arch/nios2/lib/board.c b/arch/nios2/lib/board.c index 65de26e..19e688a 100644 --- a/arch/nios2/lib/board.c +++ b/arch/nios2/lib/board.c @@ -83,21 +83,20 @@ init_fnc_t *init_sequence[] = {
/***********************************************************************/ +gd_t gd_data; +bd_t bd_data; + void board_init (void) { bd_t *bd; init_fnc_t **init_fnc_ptr;
- /* Pointer is writable since we allocated a register for it. - * Nios treats CONFIG_SYS_GBL_DATA_OFFSET as an address. - */ - gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET; + /* Pointer is writable since we allocated a register for it. */ + gd = &gd_data; /* compiler optimization barrier needed for GCC >= 3.4 */ __asm__ __volatile__("": : :"memory");
- memset( gd, 0, GENERATED_GBL_DATA_SIZE ); - - gd->bd = (bd_t *)(gd+1); /* At end of global data */ + gd->bd = &bd_data; gd->baudrate = CONFIG_BAUDRATE; gd->cpu_clk = CONFIG_SYS_CLK_FREQ;
diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h index 17017a5..dccb66c 100644 --- a/include/configs/nios2-generic.h +++ b/include/configs/nios2-generic.h @@ -119,8 +119,7 @@ * MEMORY ORGANIZATION * -Monitor at top of sdram. * -The heap is placed below the monitor - * -Global data is placed below the heap. - * -The stack is placed below global data (&grows down). + * -The stack is placed below the heap (&grows down). */ #define CONFIG_MONITOR_IS_IN_RAM #define CONFIG_SYS_MONITOR_LEN 0x40000 /* Reserve 256k */ @@ -130,10 +129,7 @@ #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 0x20000) #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_BD_INFO_SIZE) -#define CONFIG_SYS_INIT_SP CONFIG_SYS_GBL_DATA_OFFSET +#define CONFIG_SYS_INIT_SP CONFIG_SYS_MALLOC_BASE
/* * MISC
On Thursday 01 March 2012 02:09:05 Thomas Chou wrote:
--- a/arch/nios2/lib/board.c +++ b/arch/nios2/lib/board.c
+gd_t gd_data; +bd_t bd_data;
mark both static
--- a/include/configs/nios2-generic.h +++ b/include/configs/nios2-generic.h
- MEMORY ORGANIZATION
- -Monitor at top of sdram.
- -The heap is placed below the monitor
- -Global data is placed below the heap.
- -The stack is placed below global data (&grows down).
- -The stack is placed below the heap (&grows down).
you've got a tab after ther "*" but none of the previous lines do ... -mike
As suggested by Graeme Russ, move gd and bd data structrures to BSS instead of calculating the locations around the stack and heap.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- For u-boot. v2, mark gd/bd static
arch/nios2/lib/board.c | 12 +++++------- include/configs/nios2-generic.h | 12 ++++-------- 2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/nios2/lib/board.c b/arch/nios2/lib/board.c index 65de26e..6aa6b9c 100644 --- a/arch/nios2/lib/board.c +++ b/arch/nios2/lib/board.c @@ -87,17 +87,15 @@ void board_init (void) { bd_t *bd; init_fnc_t **init_fnc_ptr; + static gd_t gd_data; + static bd_t bd_data;
- /* Pointer is writable since we allocated a register for it. - * Nios treats CONFIG_SYS_GBL_DATA_OFFSET as an address. - */ - gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET; + /* Pointer is writable since we allocated a register for it. */ + gd = &gd_data; /* compiler optimization barrier needed for GCC >= 3.4 */ __asm__ __volatile__("": : :"memory");
- memset( gd, 0, GENERATED_GBL_DATA_SIZE ); - - gd->bd = (bd_t *)(gd+1); /* At end of global data */ + gd->bd = &bd_data; gd->baudrate = CONFIG_BAUDRATE; gd->cpu_clk = CONFIG_SYS_CLK_FREQ;
diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h index 17017a5..6a79d09 100644 --- a/include/configs/nios2-generic.h +++ b/include/configs/nios2-generic.h @@ -117,10 +117,9 @@
/* * MEMORY ORGANIZATION - * -Monitor at top of sdram. - * -The heap is placed below the monitor - * -Global data is placed below the heap. - * -The stack is placed below global data (&grows down). + * -Monitor at top of sdram. + * -The heap is placed below the monitor + * -The stack is placed below the heap (&grows down). */ #define CONFIG_MONITOR_IS_IN_RAM #define CONFIG_SYS_MONITOR_LEN 0x40000 /* Reserve 256k */ @@ -130,10 +129,7 @@ #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 0x20000) #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_BD_INFO_SIZE) -#define CONFIG_SYS_INIT_SP CONFIG_SYS_GBL_DATA_OFFSET +#define CONFIG_SYS_INIT_SP CONFIG_SYS_MALLOC_BASE
/* * MISC
Acked-by: Mike Frysinger vapier@gentoo.org -mike
Hi Graeme,
Le 29/02/2012 23:41, Graeme Russ a écrit :
Hi Mike,
On Thu, Mar 1, 2012 at 9:29 AM, Mike Frysingervapier@gentoo.org wrote:
On Wednesday 29 February 2012 17:22:26 Graeme Russ wrote:
On Thu, Mar 1, 2012 at 6:04 AM, Mike Frysinger wrote:
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout: http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-la yout
I had a look at this and noticed that you statically allocate locations for gd and bd (CONFIG_SYS_GBL_DATA_ADDR, CONFIG_SYS_BD_INFO_ADDR)
Considering that:
a) the gd pointer is in a register (P3) and thus easily locatable by a debugger, and; b) the bd pointer is in gd
Is there any reason not to have gd and bd in BSS?
in the Blackfin case, most likely not. we don't do relocation, and the bss is cleared long before board_init_f() gets called. the only reason for allowing the config to override would be if someone wanted to put gd/bd into on-chip L1 data, but i can't imagine this structure being performance critical enough to warrant that.
I thought as much - I moved gd/bd into BSS for x86 without really thinking about why everyone else calculates the location of these data structures around the stack and heap. The longer I think about it, the more I think that it was not a bad move and that maybe other arches can follow suit as part of standardising the init sequences
ARMs relocate and don't have a valid BSS until board_init_r() but require gd as early as board_init_f().
Regards,
Graeme
Amicalement,
Hi Albert,
On Fri, Mar 2, 2012 at 8:57 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Graeme,
Le 29/02/2012 23:41, Graeme Russ a écrit :
Hi Mike,
On Thu, Mar 1, 2012 at 9:29 AM, Mike Frysingervapier@gentoo.org wrote:
On Wednesday 29 February 2012 17:22:26 Graeme Russ wrote:
On Thu, Mar 1, 2012 at 6:04 AM, Mike Frysinger wrote:
On Tuesday 28 February 2012 18:32:57 Graeme Russ wrote:
And this is why I dislike the implementation - You have to do all sorts of weird calucations to put things in the right place when, in fact, the location of gd and bd in memory is totally irrelavent.
right, that's why i minimized the pain for Blackfin users -- this is all handled in the arch's config-pre.h header. board porters only need to declare the size of regions they care about (monitor and heap sizes).
Ow, ouch! - And that padding makes things more fun - The memory layout is
U-Boot | gd | pad | bd | pad | heap
fwiw, i documented the Blackfin memory layout:
http://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot:memory-la yout
I had a look at this and noticed that you statically allocate locations for gd and bd (CONFIG_SYS_GBL_DATA_ADDR, CONFIG_SYS_BD_INFO_ADDR)
Considering that:
a) the gd pointer is in a register (P3) and thus easily locatable by a debugger, and; b) the bd pointer is in gd
Is there any reason not to have gd and bd in BSS?
in the Blackfin case, most likely not. we don't do relocation, and the bss is cleared long before board_init_f() gets called. the only reason for allowing the config to override would be if someone wanted to put gd/bd into on-chip L1 data, but i can't imagine this structure being performance critical enough to warrant that.
I thought as much - I moved gd/bd into BSS for x86 without really thinking about why everyone else calculates the location of these data structures around the stack and heap. The longer I think about it, the more I think that it was not a bad move and that maybe other arches can follow suit as part of standardising the init sequences
ARMs relocate and don't have a valid BSS until board_init_r() but require gd as early as board_init_f().
So does x86 - A temporary gd is kept in Cache-As-RAM until SDRAM is initialised.
I just noticed that for x86, only bd is in bss - I still calculate a permanent resting place for gd around the relocated U-Boot, heap and stack but I plan to change this so the init sequence will be:
- Create temporary gd and stack in CAR for board_init_f - After SDRAM is initialised and the new stack created, 'pivot' U-Boot so it is running from flash, but using SDRAM for stack - Copy gd from CAR to stack - Copy U-Boot to SDDRAM, clear BSS, do relocation fixups - Copy gd from stack to BSS - 'pivot' execution from flash to SDRAM (this may involve resetting the stack pointer, just to save a few bytes used by the no longer needed call stack and temporary gd, but this is not a neccessity) - Create heap below U-Boot
So the end memory layout is:
----------- Top Of SDRAM ----------- .bss \ .data + - U-Boot.bin .text / ------------------------------------
heap
------------------------------------
stack (grows down)
....................................
Free Memory
--- Bottom of SDRAM (0x00000000) ---
Simple :)
Regards,
Graeme
participants (5)
-
Albert ARIBAUD -
Alex Hornung -
Graeme Russ -
Mike Frysinger -
Thomas Chou