Re: [U-Boot] [PATCH PATCH v4 04/15] spl: fit: allocate a temporary buffer to load the overlays
Hi Simon, On 17/09/2019 07:48, Simon Glass wrote: Hi Jean-Jacques, On Wed, 11 Sep 2019 at 06:32, Jean-Jacques Hiblot wrote: 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 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. You should be able to check the header to see the uncompressed size. I looked into that, and there is no uncompressed size in the GZIP header. We cannot know the size in advance. JJ Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH PATCH v4 04/15] spl: fit: allocate a temporary buffer to load the overlays
Hi Jean-Jacques, On Wed, 11 Sep 2019 at 06:32, Jean-Jacques Hiblot wrote: > > 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 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. You should be able to check the header to see the uncompressed size. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH PATCH v4 04/15] spl: fit: allocate a temporary buffer to load the overlays
On 13/08/2019 11:33, Simon Glass wrote:
Hi Jean-Jacques,
On Mon, 5 Aug 2019 at 03:44, Jean-Jacques Hiblot 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
---
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
#include
#include
-#include
+#include
#include
+#include
#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.
This makes me think that loading u-boot from a FIT image in SPL probably
doesn't work on sandbox, otherwise there would be some call to
map-sysmem() in spl_load_fit_image().
This seems like a big task and honestly I don't have time to work on it.
So I'll forgo the map_system()/map_to_sysmem(); adding them will only
trick people into thinking that this work on sandbox.
ret = spl_load_fit_image(info, sector, fit,
base_offset,
node, _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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
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 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
---
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
#include
#include
-#include
+#include
#include
+#include
#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, _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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH PATCH v4 04/15] spl: fit: allocate a temporary buffer to load the overlays
Hi Jean-Jacques,
On Mon, 5 Aug 2019 at 03:44, Jean-Jacques Hiblot 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
>
> ---
>
> 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
> #include
> #include
> -#include
> +#include
> #include
> +#include
>
> #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, _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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
