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_SIZE
    

  */

typedef 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 Data
  

structure */

•    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);