[U-Boot] malloc_simple: Allow malloc_simple to be used with non stack RAM
Hi Tom and Simon,
Here is an old patch of mine, which at the time I send it got reviewed by Simon, but never got merged.
I've just brushed this patch of, because it shrinks the SPL by about 3k, and with the spl nand support we've recently added we are going over the size limit with the SPL on some sunxi boards.
So I would like to see this merged. Simon, does your ack still stand for this patch ? Tom, if so can you merge this please ?
Note this patch has close to 0 chance of causing regressions as it is a nop unless the new CONFIG_SYS_MALLOC_F_BASE define is defined.
Patch 2 / 3 are merely added as example / for Ian to review :) I will merge these through the sunxi tree once patch 1 is merged.
Thanks & Regards,
Hans
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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org --- 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)) {
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
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?
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.
Regards, Simon
Hi,
On 18-08-15 03:59, Simon Glass wrote:
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
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.
Regards,
Hans
Hi Hans,
On 18 August 2015 at 03:23, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 18-08-15 03:59, Simon Glass wrote:
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
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.
Regards, Simon
Hi Hans,
On 18 August 2015 at 06:45, Simon Glass sjg@chromium.org wrote:
Hi Hans,
On 18 August 2015 at 03:23, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 18-08-15 03:59, Simon Glass wrote:
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
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().
(Little update - this is not strictly true - with your patch you needed to change board_init_f_mem(), but with my thoughts below you don't need to)
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.
Regards, Simon
Hi,
On 18-08-15 14:45, Simon Glass wrote:
Hi Hans,
On 18 August 2015 at 03:23, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 18-08-15 03:59, Simon Glass wrote:
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
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
common/dlmalloc.c is quite big, both in .text and .data usage. E.g. for a Mele_M9 sun6i board build this reduces .text from 0x4214 to 0x3b94 bytes, and .data from 0x54c to 0x144 bytes.
Signed-off-by: Hans de Goede hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org --- include/configs/sunxi-common.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 837feaf..1776609 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -80,7 +80,7 @@ #define CONFIG_SYS_LOAD_ADDR 0x22000000 /* default load address */ #define CONFIG_SYS_TEXT_BASE 0x2a000000 #define CONFIG_PRE_CON_BUF_ADDR 0x2f000000 -#define CONFIG_SYS_SPL_MALLOC_START 0x2ff00000 +#define CONFIG_SPL_MALLOC_F_BASE 0x2ff00000 #define CONFIG_SPL_BSS_START_ADDR 0x2ff80000 #else #define SDRAM_OFFSET(x) 0x4##x @@ -88,12 +88,19 @@ #define CONFIG_SYS_LOAD_ADDR 0x42000000 /* default load address */ #define CONFIG_SYS_TEXT_BASE 0x4a000000 #define CONFIG_PRE_CON_BUF_ADDR 0x4f000000 -#define CONFIG_SYS_SPL_MALLOC_START 0x4ff00000 +#define CONFIG_SPL_MALLOC_F_BASE 0x4ff00000 #define CONFIG_SPL_BSS_START_ADDR 0x4ff80000 #endif
#define CONFIG_SPL_BSS_MAX_SIZE 0x00080000 /* 512 KiB */ -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00080000 /* 512 KiB */ +#define CONFIG_SPL_MALLOC_F_LEN 0x00080000 /* 512 KiB */ + +#ifdef CONFIG_SPL_BUILD +#define CONFIG_SYS_MALLOC_SIMPLE /* Use malloc_simple to save space */ +#undef CONFIG_SYS_MALLOC_F_LEN +#define CONFIG_SYS_MALLOC_F_LEN CONFIG_SPL_MALLOC_F_LEN +#define CONFIG_SYS_MALLOC_F_BASE CONFIG_SPL_MALLOC_F_BASE +#endif
#ifdef CONFIG_MACH_SUN9I /*
On Mon, 2015-08-17 at 18:08 +0200, Hans de Goede wrote:
common/dlmalloc.c is quite big, both in .text and .data usage. E.g. for a Mele_M9 sun6i board build this reduces .text from 0x4214 to 0x3b94 bytes, and .data from 0x54c to 0x144 bytes.
Signed-off-by: Hans de Goede hdegoede@redhat.com Reviewed-by: Simon Glass sjg@chromium.org
Acked-by: Ian Campbell ijc@hellion.org.uk
Move some #define-s around from one #ifdef block to another to reduce the number of #ifdef blocks (note this causes no functional changes even though the conditions are not always exactly the same) and move generic #include statements to the top.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- include/configs/sunxi-common.h | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 1776609..915b597 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -13,6 +13,7 @@ #ifndef _SUNXI_COMMON_CONFIG_H #define _SUNXI_COMMON_CONFIG_H
+#include <asm/arch/cpu.h> #include <linux/stringify.h>
#ifdef CONFIG_OLD_SUNXI_KERNEL_COMPAT @@ -35,27 +36,15 @@ * High Level Configuration Options */ #define CONFIG_SUNXI /* sunxi family */ -#ifdef CONFIG_SPL_BUILD -#define CONFIG_SYS_THUMB_BUILD /* Thumbs mode to save space in SPL */ -#endif - -#include <asm/arch/cpu.h> /* get chip and board defs */ - -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM_SERIAL) -# define CONFIG_DW_SERIAL -#endif - -/* - * Display CPU information - */ -#define CONFIG_DISPLAY_CPUINFO
/* Serial & console */ #define CONFIG_SYS_NS16550 #define CONFIG_SYS_NS16550_SERIAL /* ns16550 reg in the low bits of cpu reg */ #define CONFIG_SYS_NS16550_CLK 24000000 -#ifndef CONFIG_DM_SERIAL +#ifdef CONFIG_DM_SERIAL +# define CONFIG_DW_SERIAL +#else # define CONFIG_SYS_NS16550_REG_SIZE -4 # define CONFIG_SYS_NS16550_COM1 SUNXI_UART0_BASE # define CONFIG_SYS_NS16550_COM2 SUNXI_UART1_BASE @@ -65,6 +54,7 @@ #endif
/* CPU */ +#define CONFIG_DISPLAY_CPUINFO #define CONFIG_SYS_CACHELINE_SIZE 64
/* @@ -96,6 +86,7 @@ #define CONFIG_SPL_MALLOC_F_LEN 0x00080000 /* 512 KiB */
#ifdef CONFIG_SPL_BUILD +#define CONFIG_SYS_THUMB_BUILD /* Thumbs mode to save space in SPL */ #define CONFIG_SYS_MALLOC_SIMPLE /* Use malloc_simple to save space */ #undef CONFIG_SYS_MALLOC_F_LEN #define CONFIG_SYS_MALLOC_F_LEN CONFIG_SPL_MALLOC_F_LEN
On Mon, 2015-08-17 at 18:08 +0200, Hans de Goede wrote:
Move some #define-s around from one #ifdef block to another to reduce the number of #ifdef blocks (note this causes no functional changes even though the conditions are not always exactly the same) and move generic #include statements to the top.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
participants (3)
-
Hans de Goede -
Ian Campbell -
Simon Glass