[U-Boot] Microblaze stack clobbered in d4e8ada0f6d51e0e3b80790fb9375ac8910f5352 (Consolidate arch-specific mem_malloc_init() implementations)

[U-Boot] Microblaze stack clobbered in d4e8ada0f6d51e0e3b80790fb9375ac8910f5352 (Consolidate arch-specific mem_malloc_init() implementations)

[Graeme Smecher]

Hi Michal,

It appears there's a problem with the default memory map in u-boot's 
"microblaze-generic" configuration. We have (from 
include/configs/microblaze-generic.h):


    /* ddr sdram - main memory */
    #define    CONFIG_SYS_SDRAM_BASE        XILINX_RAM_START
    #define    CONFIG_SYS_SDRAM_SIZE        XILINX_RAM_SIZE
    #define    CONFIG_SYS_MEMTEST_START    CONFIG_SYS_SDRAM_BASE
    #define    CONFIG_SYS_MEMTEST_END        (CONFIG_SYS_SDRAM_BASE + 
0x1000)

    /* global pointer */
    #define    CONFIG_SYS_GBL_DATA_SIZE    128 /* size of global data */
    /* start of global data */
    #define    CONFIG_SYS_GBL_DATA_OFFSET    (CONFIG_SYS_SDRAM_BASE + 
CONFIG_SYS_SDRAM_SIZE - CONFIG_SYS_GBL_DATA_SIZE)

    /* monitor code */
    #define    SIZE            0x40000
    #define    CONFIG_SYS_MONITOR_LEN        (SIZE - 
CONFIG_SYS_GBL_DATA_SIZE)
    #define    CONFIG_SYS_MONITOR_BASE    (CONFIG_SYS_GBL_DATA_OFFSET - 
CONFIG_SYS_MONITOR_LEN)
    #define    CONFIG_SYS_MONITOR_END        (CONFIG_SYS_MONITOR_BASE + 
CONFIG_SYS_MONITOR_LEN)
    #define    CONFIG_SYS_MALLOC_LEN        SIZE
    #define    CONFIG_SYS_MALLOC_BASE        (CONFIG_SYS_MONITOR_BASE - 
CONFIG_SYS_MALLOC_LEN)

    /* stack */
    #define    CONFIG_SYS_INIT_SP_OFFSET    CONFIG_SYS_MONITOR_BASE


This builds a memory map like the following:

    0x47f80000 - 0x47fbffff: malloc region
    0x47fc0000 - 0x47ffff7f: monitor
    0x47ffff80 - 0x47ffffff: globals

The initial SP is set to 0x47fc0000. This collides with the malloc 
region, and when the malloc region is cleared during mem_malloc_init(), 
the stack is wiped out. This includes the return address from 
mem_malloc_init(), among other things.

Can you confirm that CONFIG_SYS_INIT_SP_OFFSET should be 
CONFIG_SYS_MALLOC_BASE instead of CONFIG_SYS_MONITOR_BASE? That agrees 
with your memory diagram at include/configs/microblaze-generic.h:103.

thanks,
Graeme

[U-Boot] Microblaze stack clobbered in d4e8ada0f6d51e0e3b80790fb9375ac8910f5352 (Consolidate arch-specific mem_malloc_init() implementations)

[Michal Simek]

Hi Graeme,

Graeme Smecher wrote:
> Hi Michal,
> 
> It appears there's a problem with the default memory map in u-boot's 
> "microblaze-generic" configuration. We have (from 
> include/configs/microblaze-generic.h):
> 
> 
>    /* ddr sdram - main memory */
>    #define    CONFIG_SYS_SDRAM_BASE        XILINX_RAM_START
>    #define    CONFIG_SYS_SDRAM_SIZE        XILINX_RAM_SIZE
>    #define    CONFIG_SYS_MEMTEST_START    CONFIG_SYS_SDRAM_BASE
>    #define    CONFIG_SYS_MEMTEST_END        (CONFIG_SYS_SDRAM_BASE + 
> 0x1000)
> 
>    /* global pointer */
>    #define    CONFIG_SYS_GBL_DATA_SIZE    128 /* size of global data */
>    /* start of global data */
>    #define    CONFIG_SYS_GBL_DATA_OFFSET    (CONFIG_SYS_SDRAM_BASE + 
> CONFIG_SYS_SDRAM_SIZE - CONFIG_SYS_GBL_DATA_SIZE)
> 
>    /* monitor code */
>    #define    SIZE            0x40000
>    #define    CONFIG_SYS_MONITOR_LEN        (SIZE - 
> CONFIG_SYS_GBL_DATA_SIZE)
>    #define    CONFIG_SYS_MONITOR_BASE    (CONFIG_SYS_GBL_DATA_OFFSET - 
> CONFIG_SYS_MONITOR_LEN)
>    #define    CONFIG_SYS_MONITOR_END        (CONFIG_SYS_MONITOR_BASE + 
> CONFIG_SYS_MONITOR_LEN)
>    #define    CONFIG_SYS_MALLOC_LEN        SIZE
>    #define    CONFIG_SYS_MALLOC_BASE        (CONFIG_SYS_MONITOR_BASE - 
> CONFIG_SYS_MALLOC_LEN)
> 
>    /* stack */
>    #define    CONFIG_SYS_INIT_SP_OFFSET    CONFIG_SYS_MONITOR_BASE
> 
> 
> This builds a memory map like the following:
> 
>    0x47f80000 - 0x47fbffff: malloc region
>    0x47fc0000 - 0x47ffff7f: monitor
>    0x47ffff80 - 0x47ffffff: globals
> 
> The initial SP is set to 0x47fc0000. This collides with the malloc 
> region, and when the malloc region is cleared during mem_malloc_init(), 
> the stack is wiped out. This includes the return address from 
> mem_malloc_init(), among other things.
> 
> Can you confirm that CONFIG_SYS_INIT_SP_OFFSET should be 
> CONFIG_SYS_MALLOC_BASE instead of CONFIG_SYS_MONITOR_BASE? That agrees 
> with your memory diagram at include/configs/microblaze-generic.h:103.

yes, you are correct.
Please create proper patch and I will add it to mainline.

Thanks,
Michal


> 
> thanks,
> Graeme


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian