Re: [U-Boot] [U-Boot-DM] [PATCH 1/1] early_malloc() introduced to ARM architecture
Dear Tomas Hlavacek,
I think we should still mark early patches as RFC.
early_malloc() introduced to ARM architecture (it is a proof of concept). GD datastructure extended for holding early-heap. mem_malloc_init() in board_init_r() has been put up by few lines.
I take it was sent by the gmail webIF ? Like WD said, not a good idea, git send- email is really the way. it's actually simple, we can discuss that tomorrow.
Signed-off-by: Tomas Hlavacek tmshlvck@gmail.com
arch/arm/include/asm/global_data.h | 4 +- arch/arm/lib/board.c | 32 +++++++++-- common/Makefile | 1 + common/earlymalloc.c | 102 ++++++++++++++++++++++++++++++++++++ include/common.h | 1 + include/earlymalloc.h | 84 +++++++++++++++++++++++++++++ lib/asm-offsets.c | 2 +- 7 files changed, 220 insertions(+), 6 deletions(-) create mode 100644 common/earlymalloc.c create mode 100644 include/earlymalloc.h
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index c3ff789..215212a 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -30,7 +30,8 @@
- global variables during system initialization (until we have set
- up the memory controller so that we can use RAM).
- Keep it *SMALL* and remember to set GENERATED_GBL_DATA_SIZE >
sizeof(gd_t)
- Keep it *SMALL* and remember to set GENERATED_GBL_DATA_SIZE >
*/
sizeof(gd_t) + EARLY_HEAP_SIZEtypedef struct global_data { @@ -86,6 +87,7 @@ typedef struct global_data { #endif } gd_t;
Ok well ... you know, this will need cleanup. But for RFC, this is OK.
/*
- Global Data Flags
*/ diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 500e216..81ee27b 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -52,6 +52,7 @@ #include <fdtdec.h> #include <post.h> #include <logbuff.h> +#include <earlymalloc.h>
Do we need early_malloc.h at all? malloc.h won't cut it?
#ifdef CONFIG_BITBANGMII #include <miiphy.h> @@ -273,6 +274,9 @@ void board_init_f(ulong bootflag)
memset((void *)gd, 0, sizeof(gd_t));
- early_malloc_init();
So this basically flips a bit, do it the other way and you don't need it.
- debug("Early malloc initialized.\n");
- gd->mon_len = _bss_end_ofs;
#ifdef CONFIG_OF_EMBED /* Get a pointer to the FDT */ @@ -452,12 +469,23 @@ void board_init_r(gd_t *id, ulong dest_addr) ulong flash_size; #endif
gd_t *old_gd = gd;
gd = id;
gd->flags |= GD_FLG_RELOC; /* tell others: relocation done */
monitor_flash_len = _end_ofs;
/* The Malloc area is immediately below the monitor copy in DRAM */
malloc_start = dest_addr - TOTAL_MALLOC_LEN;
mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
// TODO: DM Cores relocation goes here.
/* Disable early mallocator (effectively switch calls to the real
malloc). */
- early_malloc_disab((gde_t *)old_gd);
- /* Enable caches */ enable_caches();
@@ -485,10 +513,6 @@ void board_init_r(gd_t *id, ulong dest_addr) post_output_backlog(); #endif
- /* The Malloc area is immediately below the monitor copy in DRAM */
- malloc_start = dest_addr - TOTAL_MALLOC_LEN;
- mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
Can mem_malloc_init() not flip on the early_malloc bit for you here?
#if !defined(CONFIG_SYS_NO_FLASH) puts("Flash: ");
[...]
+#include <asm/global_data.h> /* for gd_t and gd */
+DECLARE_GLOBAL_DATA_PTR;
+static inline size_t early_malloc_align_up(phys_addr_t addr) +{
- if(!EARLY_MALLOC_IS_ALIGNED(addr)) {
- addr = EARLY_MALLOC_ALIGN_DOWN(addr);
- addr += EARLY_MALLOC_ALIGN_MULTIPLE;
- }
- return addr;
+}
+void early_malloc_init(void) +{
- ((gde_t *)gd)->em.flags = EARLY_MALLOC_INIT_FLAGS;
- ((gde_t *)gd)->em.size = 0;
+}
+int early_malloc_isactive(void) +{
- /* The early_malloc() is inactive after relocation or when the
flag says so. */- return ((EARLY_MALLOC_IS_ACTIVE(((gde_t *)gd)->em.flags)) ||
(gd->flags & GD_FLG_RELOC));
So ... gd->flags will be enough for the early malloc, you don't need any further flags
+}
+int early_free_isactive(void) +{
- /* The early free is inactive when the flag says so. */
- return (gd->flags & GD_FLG_RELOC);
+}
+void *early_malloc(size_t size)
No, we want this wrapped into malloc() call, so the drivers can be inited indifferent of time.
+{
- em_spc_t *em = &((gde_t *)gd)->em;
- /* Check flag active. */
- if(!EARLY_MALLOC_IS_ACTIVE(em->flags))
- return NULL;
Indent with tab please ... tools/checkpatch.pl will help here.
- /* Choose block beginning address. */
- phys_addr_t addr =
early_malloc_align_up(((phys_addr_t)&em->heap)+em->size);
- /* Check free space. */
- if(addr + size >= ((phys_addr_t)(&(em->heap))) + EARLY_HEAP_SIZE)
- return NULL;
- /* Calculate next free space. */
- em->size+=early_malloc_align_up(size);
- return (void *)addr;
+}
+int early_malloc_isaddress(void *addr) +{
- if((((phys_addr_t)addr) >= (phys_addr_t)(&(((gde_t *)gd)->em.heap)))&&
(((phys_addr_t)addr) < ((phys_addr_t)(&(((gde_t*)gd)->em.heap)))+EARLY_HEAP_SIZE))
See container_of() ... and you can really split the checks.
- return 1;
- else
- return 0;
+}
+void early_free(void *addr) +{
- /* NOOP - Early malloc does not support free(). */
This goes to free() call ...
+}
+void early_malloc_disab(gde_t *gd) +{
- EARLY_MALLOC_DISABLE(gd->em.flags);
+} diff --git a/include/common.h b/include/common.h index a2c6b27..486925f 100644 --- a/include/common.h +++ b/include/common.h @@ -171,6 +171,7 @@ typedef void (interrupt_handler_t)(void *);
#include <asm/u-boot.h> /* boot information for Linux kernel */ #include <asm/global_data.h> /* global data used for startup functions */ +#include <earlymalloc.h> /* Early mallocator data structure that extends global data. */
Drop this altogether. [...]
+typedef struct early_malloc_spc {
char flags;short size;char heap[EARLY_HEAP_SIZE];+} em_spc_t;
Drop the typedef please.
+/*
- Platform data extended for the early mallocator.
- This structure holds the original global data (and it starts at
- the very same address), but it is longer because it holds
- early malloc space in the end.
- This is a trick with data structures: This datastructure is
- pre-allocated by lib/asm-offsets.c but this structure is not to
- be relocated as whole because struct global_data is used during
- normal operations as well as during relocation phase.
- */
+typedef struct global_data_extended {
I'm not convinced about the typedef here either ... others?
gd_t gd; /* Original Global Datastructure */
em_spc_t em; /* Early-malloc space.*/ +} gde_t;
+/* Initial flags block. Active flag set. */ +#define EARLY_MALLOC_INIT_FLAGS 0x80
+/* Alignment size - number of bytes to align to it's mupliple. */ +#define EARLY_MALLOC_ALIGN_MULTIPLE (sizeof(phys_addr_t)) +#define EARLY_MALLOC_ALIGN_MASK (EARLY_MALLOC_ALIGN_MULTIPLE-1)
+#define EARLY_MALLOC_ALIGN_DOWN(addr) ((phys_addr_t)addr & (~EARLY_MALLOC_ALIGN_MASK)) +#define EARLY_MALLOC_IS_ALIGNED(addr) (!(((phys_addr_t)addr) & EARLY_MALLOC_ALIGN_MASK)) +#define EARLY_MALLOC_FLAG_ACTIVE 0x80 +#define EARLY_MALLOC_IS_ACTIVE(flags) (flags & EARLY_MALLOC_FLAG_ACTIVE) +#define EARLY_MALLOC_DISABLE(flags) (flags=(flags&(~EARLY_MALLOC_FLAG_ACTIVE)))
You won't need this whole file I think ...
+void early_malloc_init(void); +int early_malloc_isactive(void); +int early_free_isactive(void); +void *early_malloc(size_t size); +void early_free(void *addr); +void early_malloc_disab(gde_t *gd); +int early_malloc_isaddress(void *);
+#endif diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c index c88f5d4..6e39826 100644 --- a/lib/asm-offsets.c +++ b/lib/asm-offsets.c @@ -23,7 +23,7 @@ int main(void) { /* Round up to make sure size gives nice stack alignment */ DEFINE(GENERATED_GBL_DATA_SIZE,
(sizeof(struct global_data) + 15) & ~15);
(sizeof(struct global_data_extended) + 15) & ~15);
And if you adjusted the global data to be the "container of both, you won't need this here either.
DEFINE(GENERATED_BD_INFO_SIZE, (sizeof(struct bd_info) + 15) & ~15);
Hello Marek,
On Sat, Jul 28, 2012 at 10:36 PM, Marek Vasut marex@denx.de wrote:
I think we should still mark early patches as RFC.
Sure...
+#include <earlymalloc.h>
Do we need early_malloc.h at all? malloc.h won't cut it?
My intention was to keep the early_malloc in separate instance from real malloc. Although this is proof-of-concept / RFC for early_malloc, it does not contain adaptation layer for switching malloc/early_malloc, because it seriously break things. I wanted comments on the basic idea first.
- early_malloc_init();
So this basically flips a bit, do it the other way and you don't need it.
Well yes. I was told to support more stages of early_mallocator deactivation during DM core relocation. So this is why I have plenty of flags (currently using one). Now I know at least about two stages and three states. Actually intended lifecycle of the early_malloc is:
1) init: malloc() calls are switched to early_malloc(), other calls (= free(), realloc()...) are switched to dummy. 2) relocation done, real malloc initialized, before core relocation: malloc() switched to dlmalloc, other calls to dummy (this is special mode for core relocation, which prevents calling dlmaloc free() on space allocated by early_malloc(). 3) Full switch to dlmalloc.
Sure we can make it simpler, if it is OK with prospective core relocation function.
No, we want this wrapped into malloc() call, so the drivers can be inited indifferent of time.
Sure. My intention was not to hack current malloc but rather create adaptation layer. Maybe not a good idea... Anyway I can put simple switches like
if(!(gd->flags & GD_FLG_RELOC)) return early_malloc();
directly into malloc() functions.
+typedef struct early_malloc_spc {
char flags;short size;char heap[EARLY_HEAP_SIZE];+} em_spc_t;
Drop the typedef please.
Actually I don't follow what is the problem here with the typedef?
diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c index c88f5d4..6e39826 100644 --- a/lib/asm-offsets.c +++ b/lib/asm-offsets.c @@ -23,7 +23,7 @@ int main(void) { /* Round up to make sure size gives nice stack alignment */ DEFINE(GENERATED_GBL_DATA_SIZE,
(sizeof(struct global_data) + 15) & ~15);
(sizeof(struct global_data_extended) + 15) & ~15);And if you adjusted the global data to be the "container of both, you won't need this here either.
Sure. My intention was to keep early_malloc heap in separate structure (containing the GD structure) in order not to relocate it but rather relocate cores by ourselves, which was our plan. It would be simpler to have it in the end of standard GD structure, because we can drop multi-stage early_malloc turn-off and perhaps we are not going to need to do relocation of DM structures by hand.
Tomas
-- Tomáš Hlaváček tmshlvck@gmail.com
Dear Tomas Hlavacek,
Hello Marek,
On Sat, Jul 28, 2012 at 10:36 PM, Marek Vasut marex@denx.de wrote:
I think we should still mark early patches as RFC.
Sure...
+#include <earlymalloc.h>
Do we need early_malloc.h at all? malloc.h won't cut it?
My intention was to keep the early_malloc in separate instance from real malloc.
early_malloc.c is probably fine ... once you manage to properly wrap the calls to early_malloc()/dlmalloc() into malloc() and switch based on the GD flags. It's just the header file that I consider redundant.
Although this is proof-of-concept / RFC for early_malloc, it does not contain adaptation layer for switching malloc/early_malloc, because it seriously break things.
Why/how?
I wanted comments on the basic idea first.
- early_malloc_init();
So this basically flips a bit, do it the other way and you don't need it.
Well yes. I was told to support more stages of early_mallocator deactivation during DM core relocation.
Ok, add more flags maybe? Can you elaborate on the reason?
So this is why I have plenty of flags (currently using one).
Still, can you not wrap it into GD flags ?
Now I know at least about two stages and three states. Actually intended lifecycle of the early_malloc is:
- init:
What's "init" ... you mean board_init_f() ?
malloc() calls are switched to early_malloc()
Ok, but you must wrap this into malloc() call
other calls (=free(), realloc()...) are switched to dummy.
realloc() can work too, why not ? So can calloc() etc.
- relocation done
board_init_r() ?
real malloc initialized, before core relocation: malloc() switched to dlmalloc other calls to dummy (this is special mode for core relocation, which prevents calling dlmaloc free() on space allocated by early_malloc().
I see ... ok, se please check if the GD flags can't be used, I think they can.
- Full switch to dlmalloc.
Sure we can make it simpler, if it is OK with prospective core relocation function.
No, we want this wrapped into malloc() call, so the drivers can be inited indifferent of time.
Sure. My intention was not to hack current malloc but rather create adaptation layer. Maybe not a good idea... Anyway I can put simple switches like
if(!(gd->flags & GD_FLG_RELOC)) return early_malloc();
directly into malloc() functions.
You can actually switch the mallocator so the functions exported by it are prefixed with dl ... and then just create some stuff that's called malloc() free() etc. and does the switching.
+typedef struct early_malloc_spc {
char flags;short size;char heap[EARLY_HEAP_SIZE];+} em_spc_t;
Drop the typedef please.
Actually I don't follow what is the problem here with the typedef?
http://lwn.net/Articles/2241/ ... but I'd like to gain opinions from other people on this one.
diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c index c88f5d4..6e39826 100644 --- a/lib/asm-offsets.c +++ b/lib/asm-offsets.c @@ -23,7 +23,7 @@ int main(void)
{
/* Round up to make sure size gives nice stack alignment */ DEFINE(GENERATED_GBL_DATA_SIZE,
(sizeof(struct global_data) + 15) & ~15);
(sizeof(struct global_data_extended) + 15) & ~15);And if you adjusted the global data to be the "container of both, you won't need this here either.
Sure. My intention was to keep early_malloc heap in separate structure
Of course. My point was to do it the other way. But looking at it one more time, this seems correct.
(containing the GD structure) in order not to relocate it but rather relocate cores by ourselves, which was our plan. It would be simpler to have it in the end of standard GD structure, because we can drop multi-stage early_malloc turn-off and perhaps we are not going to need to do relocation of DM structures by hand.
Tomas
-- Tomáš Hlaváček tmshlvck@gmail.com
Best regards, Marek Vasut
participants (2)
-
Marek Vasut -
Tomas Hlavacek