[U-Boot] [U-Boot-DM] [PATCH 1/1] early_malloc() introduced to ARM architecture

[Marek Vasut <marex@denx.de>]

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