From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Wed, 19 Aug 2015 13:40:04 +0200 Subject: [U-Boot] [PATCH 1/3] malloc_simple: Allow malloc_simple to be used with non stack RAM In-Reply-To: References: <1439827706-1748-1-git-send-email-hdegoede@redhat.com> <1439827706-1748-2-git-send-email-hdegoede@redhat.com> <55D2F985.9050208@redhat.com> Message-ID: <55D46B14.9080200@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 18-08-15 14:45, Simon Glass wrote: > Hi Hans, > > On 18 August 2015 at 03:23, Hans de Goede wrote: >> Hi, >> >> >> On 18-08-15 03:59, Simon Glass wrote: >>> >>> Hi Hans, >>> >>> On 17 August 2015 at 10:08, Hans de Goede wrote: >>>> >>>> Before this patch malloc_simple would always allocate a chunk of RAM from >>>> the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when >>>> set directly specifies the memory address to use for the heap with >>>> malloc_simple. >>>> >>>> Signed-off-by: Hans de Goede >>>> Reviewed-by: Simon Glass >>>> --- >>>> arch/arm/lib/crt0.S | 2 +- >>>> common/board_f.c | 4 ++++ >>>> common/dlmalloc.c | 4 ++++ >>>> common/spl/spl.c | 3 +++ >>>> 4 files changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S >>>> index afd4f10..5e6619f 100644 >>>> --- a/arch/arm/lib/crt0.S >>>> +++ b/arch/arm/lib/crt0.S >>>> @@ -96,7 +96,7 @@ clr_gd: >>>> strlo r0, [r1] /* clear 32-bit GD word */ >>>> addlo r1, r1, #4 /* move to next */ >>>> blo clr_gd >>>> -#if defined(CONFIG_SYS_MALLOC_F_LEN) >>>> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && >>>> !defined(CONFIG_SYS_MALLOC_F_BASE) >>>> sub sp, sp, #CONFIG_SYS_MALLOC_F_LEN >>>> str sp, [r9, #GD_MALLOC_BASE] >>>> #endif >>>> diff --git a/common/board_f.c b/common/board_f.c >>>> index c959774..7f3b96f 100644 >>>> --- a/common/board_f.c >>>> +++ b/common/board_f.c >>>> @@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top) >>>> arch_setup_gd(gd_ptr); >>>> >>>> #ifdef CONFIG_SYS_MALLOC_F_LEN >>>> +#if defined(CONFIG_SYS_MALLOC_F_BASE) >>>> + gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE; >>>> +#else >>>> top -= CONFIG_SYS_MALLOC_F_LEN; >>>> gd->malloc_base = top; >>>> #endif >>>> +#endif >>>> >>>> return top; >>>> } >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >>>> index b5bb051..9b14033 100644 >>>> --- a/common/dlmalloc.c >>>> +++ b/common/dlmalloc.c >>>> @@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number; >>>> int value; >>>> int initf_malloc(void) >>>> { >>>> #ifdef CONFIG_SYS_MALLOC_F_LEN >>>> +#if defined(CONFIG_SYS_MALLOC_F_BASE) >>>> + gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE; >>>> +#else >>>> assert(gd->malloc_base); /* Set up by crt0.S */ >>>> +#endif >>>> gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN; >>>> gd->malloc_ptr = 0; >>>> #endif >>>> diff --git a/common/spl/spl.c b/common/spl/spl.c >>>> index 94b01da..811452b 100644 >>>> --- a/common/spl/spl.c >>>> +++ b/common/spl/spl.c >>>> @@ -156,6 +156,9 @@ int spl_init(void) >>>> #if defined(CONFIG_SYS_MALLOC_F_LEN) >>>> gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN; >>>> gd->malloc_ptr = 0; >>>> +#if defined(CONFIG_SYS_MALLOC_F_BASE) >>>> + gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE; >>>> +#endif >>>> #endif >>>> if (IS_ENABLED(CONFIG_OF_CONTROL) && >>>> !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) { >>>> -- >>>> 2.4.3 >>>> >>> >>> Why does this save memory? >> >> >> See patch 2/3, which does #define CONFIG_SYS_MALLOC_SIMPLE when building >> the SPL, removing common/dlmalloc.c and only using common/malloc_simple.c >> both pre and post reloc. >> >> We need this patch to do this because we do not have room on the stack >> (which sits in SRAM) and setting CONFIG_SYS_MALLOC_F_LEN normally puts >> the malloc_simple heap there. >> >> We do however have room in DRAM and the SPL (which does not use device- >> model on sunxi) does not need malloc until after DRAM has been brought >> up, so we use this to point the malloc_simple.c heap at DRAM (far far >> away from where u-boot.bin will be loaded). >> >>> In general we should move away from hard-coding specific addresses I >>> think, and just work out the memory from a single address, subtracting >>> space for each area we need. >> >> >> I understand and agree, but I've been unable to find another easy >> solution for this, and now that we are adding nand support we are really >> running out of space in the SPL on sunxi, and could really use the circa >> 3k (out of 24k total) dlmalloc is costing us. > > OK thanks for the explanation. > > You say that you are trying to change this in SPL but your patch > changes U-Boot proper also. If it is just SPL you should not need to > change board_init_f_mem() and initf_malloc(). > > For SPL we now have spl_relocate_stack_gd() which sets up the stack in > SDRAM before calling board_init_r(). This implements the > CONFIG_SPL_STACK_R option. > > We should avoid hard-coding an address if we can. I wonder if we could > have bool CONFIG_SPL_MALLOC_R and hex CONFIG_SPL_MALLOC_R_LEN (or > hopefully you can think of better names). Then you change could go in > spl_relocate_stack_gd(), something like: > > if (IS_ENABLED(CONFIG_SPL_STACK_R)) { > ptr -= CONFIG_SPL_MALLOC_R_LEN; > gd->malloc_base = ptr; > } > > You'll unfortunately need to add another conditional to the top of > spl_init() since gd->malloc_limit will need to be set to a different > value. Sounds like a plan, I'll give this a try, may be a while before I get around to doing this though. Regards, Hans