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

Hi Simon,

On 13/08/2019 11:33, Simon Glass wrote:

Hi Jean-Jacques,

On Mon, 5 Aug 2019 at 03:44, Jean-Jacques Hiblot jjhiblot@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?

It seemed big enough to me to accommodate a DTB overlay.

There is no easy way to know how big the buffer must be without reading it first because it can be compressed.

...

Signed-off-by: Jean-Jacques Hiblot jjhiblot@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?

could be done.

...

•            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.

ok

...

                     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