[U-Boot] [PATCH PATCH v4 04/15] spl: fit: allocate a temporary buffer to load the overlays

[Simon Glass]

Hi Jean-Jacques,

On Mon, 5 Aug 2019 at 03:44, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>
> If the node describing an overlay does not specify a load address, it will
> be loaded at the address previously used.
> Fixing it by allocating a temporary 64kB region that will be used as a
> default load address.

How did you come to decide on 64KB? Might this be too large or too small?

>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>
> ---
>
> Changes in v4:
> - make sure that the temp buffer is freed in all cases
>
> Changes in v3: None
> Changes in v2: None
>
>  common/spl/spl_fit.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index ab47da5094..977074cd99 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -8,8 +8,9 @@
>  #include <errno.h>
>  #include <fpga.h>
>  #include <image.h>
> -#include <linux/libfdt.h>
> +#include <malloc.h>
>  #include <spl.h>
> +#include <linux/libfdt.h>
>
>  #ifndef CONFIG_SYS_BOOTM_LEN
>  #define CONFIG_SYS_BOOTM_LEN   (64 << 20)
> @@ -302,33 +303,50 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>         spl_image->fdt_addr = (void *)image_info.load_addr;
>  #if !CONFIG_IS_ENABLED(FIT_IMAGE_TINY)
>         if (CONFIG_IS_ENABLED(LOAD_FIT_APPLY_OVERLAY)) {
> +               void *tmpbuffer;
> +               /*
> +                * allocate 64KB of memory. This will be used to store the DT
> +                * overlay before it is applied. It may not be used depending on
> +                * how the overlay is stored, so don't fail yet if the
> +                * allocation failed.
> +                */
> +               tmpbuffer = malloc(64 * 1024);
> +               if (!tmpbuffer)
> +                       debug("%s: unable to allocate space for overlays\n",
> +                             __func__);

Can you adjust this to only allocate buf when you find it is needed?

> +
>                 for (; ; index++) {
>                         node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP,
>                                                       index);
>                         if (node < 0) {
>                                 debug("%s: No additional FDT node\n", __func__);
> -                               return 0;
> +                               break;
>                         }
>
> +                       image_info.load_addr = (ulong)tmpbuffer;

map_to_sysmem() or this won't work on sandbox.

>                         ret = spl_load_fit_image(info, sector, fit, base_offset,
>                                                  node, &image_info);
>                         if (ret < 0)
> -                               return ret;
> +                               break;
>
>                         /* Make room in FDT for changes from the overlay */
>                         ret = fdt_increase_size(spl_image->fdt_addr,
>                                                 image_info.size);
>                         if (ret < 0)
> -                               return ret;
> +                               break;
>
>                         ret = fdt_overlay_apply_verbose(spl_image->fdt_addr,
>                                                         (void *)image_info.load_addr);

map_sysmem()

>                         if (ret)
> -                               return ret;
> +                               break;
>
>                         debug("%s: DT overlay %s applied\n", __func__,
>                               fit_get_name(fit, node, NULL));
>                 }
> +               if (tmpbuffer)
> +                       free(tmpbuffer);
> +               if (ret)
> +                       return ret;
>         }
>         /* Try to make space, so we can inject details on the loadables */
>         ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
> --
> 2.17.1
>

Regads,
Simon