Hi Albert, On Wed, Feb 29, 2012 at 9:55 AM, Albert ARIBAUD <albert.u.b...@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.b...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
