[U-Boot] [PATCH 0/2] fit: FDT compression

older
[U-Boot] [PATCH v1 0/5] Kconfig...

Julius Werner

18 Apr 2019 18 Apr '19

3:22 a.m.

This patch series adds FDT compression support. The first patch adds the compression support itself, the second adds a new feature to compatible string matching that allows it to be useful with compressed FDTs.

Sandbox-tested with FIT images with and without compressed FDTs, with and without overlays, in both compatible string matching and direct config selection modes.

Julius Werner (2): fit: Support FDT compression fit: Support compat string property in configuration node

common/image-fit.c | 142 +++++++++++++++++++++++++++------------------ 1 file changed, 85 insertions(+), 57 deletions(-)

-- 2.20.1

Show replies by date

Julius Werner

18 Apr 18 Apr

3:22 a.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

This patch adds support for compressing FDT image nodes in a FIT image (equivalent to how kernel nodes can already be compressed). This can reduce the size of FIT images (and therefore improve boot times). FDTs will automatically get decompressed on load.

This patch does not support extracting compatible strings from compressed FDTs, so it's not very helpful in conjunction with CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments that select the configuration to load explicitly.

Signed-off-by: Julius Werner jwerner@chromium.org --- common/image-fit.c | 85 +++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 34 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c index ac901e131c..20e02d240b 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -22,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/

+#include <bootm.h> #include <image.h> #include <bootstage.h> #include <u-boot/crc.h> @@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt) kfdt_name); continue; } + + if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) { + debug("Can't extract compat from "%s" (compressed)\n", + kfdt_name); + continue; + } + /* * Get a pointer to this configuration's fdt. */ @@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr, const char *fit_uname_config; const char *fit_base_uname_config; const void *fit; - const void *buf; + void *buf; + void *loadbuf; size_t size; int type_ok, os_ok; - ulong load, data, len; - uint8_t os; + ulong load, load_end, data, len; + uint8_t os, comp; #ifndef USE_HOSTCC uint8_t os_arch; #endif @@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, images->os.arch = os_arch; #endif

- if (image_type == IH_TYPE_FLATDT && - !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) { - puts("FDT image is compressed"); - return -EPROTONOSUPPORT; - } - bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL); type_ok = fit_image_check_type(fit, noffset, image_type) || fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) || @@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);

/* get image data address and length */ - if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) { + if (fit_image_get_data_and_size(fit, noffset, + (const void **)&buf, &size)) { printf("Could not find %s subimage data!\n", prop_name); bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA); return -ENOENT; @@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,

#if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS) /* perform any post-processing on the image data */ - board_fit_image_post_process((void **)&buf, &size); + board_fit_image_post_process(&buf, &size); #endif

len = (ulong)size;

- /* verify that image data is a proper FDT blob */ - if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) { - puts("Subimage data is not a FDT"); - return -ENOEXEC; - } - bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);

- /* - * Work-around for eldk-4.2 which gives this warning if we try to - * cast in the unmap_sysmem() call: - * warning: initialization discards qualifiers from pointer target type - */ - { - void *vbuf = (void *)buf; - - data = map_to_sysmem(vbuf); - } - + data = map_to_sysmem(buf); + load = data; if (load_op == FIT_LOAD_IGNORED) { /* Don't load */ } else if (fit_image_get_load(fit, noffset, &load)) { @@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, } } else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) { ulong image_start, image_end; - ulong load_end; - void *dst;

/* * move image data to the load address, @@ -1993,14 +1980,44 @@ int fit_image_load(bootm_headers_t *images, ulong addr,

printf(" Loading %s from 0x%08lx to 0x%08lx\n", prop_name, data, load); + }

- dst = map_sysmem(load, len); - memmove(dst, buf, len); - data = load; + /* + * Decompress FDTs right here. Kernel images get decompressed later in + * bootm_host_load_image(). + */ + comp = IH_COMP_NONE; + loadbuf = buf; + if (image_type == IH_TYPE_FLATDT && + !fit_image_get_comp(fit, noffset, &comp) && + comp != IH_COMP_NONE) { + ulong max_decomp_len = len * 20; + if (load == data) { + loadbuf = malloc(max_decomp_len); + load = map_to_sysmem(loadbuf); + } else { + loadbuf = map_sysmem(load, max_decomp_len); + } + if (bootm_decomp_image(comp, load, data, IH_TYPE_FLATDT, + loadbuf, buf, len, max_decomp_len, &load_end)) { + printf("Error: Cannot decompress FDT image\n"); + return -ENOEXEC; + } + len = load_end - load; + } else if (load != data) { + loadbuf = map_sysmem(load, len); + memcpy(loadbuf, buf, len); } + + /* verify that image data is a proper FDT blob */ + if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) { + puts("Subimage data is not a FDT"); + return -ENOEXEC; + } + bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);

- *datap = data; + *datap = load; *lenp = len; if (fit_unamep) *fit_unamep = (char *)fit_uname;

-- 2.20.1

Simon Goldschmidt

10:29 a.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

Hi Julius,

On Thu, Apr 18, 2019 at 10:13 AM Julius Werner jwerner@chromium.org wrote:

...

This patch adds support for compressing FDT image nodes in a FIT image (equivalent to how kernel nodes can already be compressed). This can reduce the size of FIT images (and therefore improve boot times). FDTs will automatically get decompressed on load.

This patch does not support extracting compatible strings from compressed FDTs, so it's not very helpful in conjunction with CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments that select the configuration to load explicitly.

Signed-off-by: Julius Werner jwerner@chromium.org

I was working on a patch that allows *all* items loaded from a FIT image to be compressed. This is especially useful on my platform where FPGA images (quite big) are embedded in the FIT image.

I think it would make more sense to make the decompression more generic instead of adding yet another specific case for FDT decompression.

Unfortunately, I haven't found the time to finish my patch for this, yet...

Regards, Simon

...

common/image-fit.c | 85 +++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 34 deletions(-)

+#include <bootm.h> #include <image.h> #include <bootstage.h> #include <u-boot/crc.h> @@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt) kfdt_name); continue; }

          if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {

                  debug("Can't extract compat from \"%s\" (compressed)\n",

```
                        kfdt_name);
```
```
                  continue;
```
```
          }
```

          /*
           * Get a pointer to this configuration's fdt.
           */

@@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr, const char *fit_uname_config; const char *fit_base_uname_config; const void *fit;

```
  const void *buf;
```

```
  void *buf;
```

  void *loadbuf;
  size_t size;
  int type_ok, os_ok;

```
  ulong load, data, len;
```
```
  uint8_t os;
```

```
  ulong load, load_end, data, len;
```
```
  uint8_t os, comp;
```

#ifndef USE_HOSTCC uint8_t os_arch; #endif @@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, images->os.arch = os_arch; #endif

```
  if (image_type == IH_TYPE_FLATDT &&
```

      !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {

          puts("FDT image is compressed");

```
          return -EPROTONOSUPPORT;
```
```
  }
```

  bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
  type_ok = fit_image_check_type(fit, noffset, image_type) ||
            fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||

@@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);

    /* get image data address and length */

  if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) {

  if (fit_image_get_data_and_size(fit, noffset,

                                  (const void **)&buf, &size)) {
          printf("Could not find %s subimage data!\n", prop_name);
          bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
          return -ENOENT;

@@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,

#if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS) /* perform any post-processing on the image data */

  board_fit_image_post_process((void **)&buf, &size);

  board_fit_image_post_process(&buf, &size);

#endif

    len = (ulong)size;

  /* verify that image data is a proper FDT blob */

  if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {

          puts("Subimage data is not a FDT");

```
          return -ENOEXEC;
```
```
  }
```

  bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);

```
  /*
```

   * Work-around for eldk-4.2 which gives this warning if we try to

```
   * cast in the unmap_sysmem() call:
```

   * warning: initialization discards qualifiers from pointer target type

```
   */
```
```
  {
```
```
          void *vbuf = (void *)buf;
```
```
          data = map_to_sysmem(vbuf);
```
```
  }
```

```
  data = map_to_sysmem(buf);
```

  load = data;
  if (load_op == FIT_LOAD_IGNORED) {
          /* Don't load */
  } else if (fit_image_get_load(fit, noffset, &load)) {

@@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, } } else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) { ulong image_start, image_end;

```
          ulong load_end;
```

          void *dst;

          /*
           * move image data to the load address,

@@ -1993,14 +1980,44 @@ int fit_image_load(bootm_headers_t *images, ulong addr,

            printf("   Loading %s from 0x%08lx to 0x%08lx\n",
                   prop_name, data, load);

```
  }
```

```
          dst = map_sysmem(load, len);
```
```
          memmove(dst, buf, len);
```
```
          data = load;
```

```
  /*
```

   * Decompress FDTs right here. Kernel images get decompressed later in

```
   * bootm_host_load_image().
```
```
   */
```
```
  comp = IH_COMP_NONE;
```
```
  loadbuf = buf;
```
```
  if (image_type == IH_TYPE_FLATDT &&
```

      !fit_image_get_comp(fit, noffset, &comp) &&

```
      comp != IH_COMP_NONE) {
```

          ulong max_decomp_len = len * 20;

```
          if (load == data) {
```

                  loadbuf = malloc(max_decomp_len);

                  load = map_to_sysmem(loadbuf);

```
          } else {
```

                  loadbuf = map_sysmem(load, max_decomp_len);

```
          }
```

          if (bootm_decomp_image(comp, load, data, IH_TYPE_FLATDT,

                          loadbuf, buf, len, max_decomp_len, &load_end)) {

                  printf("Error: Cannot decompress FDT image\n");

```
                  return -ENOEXEC;
```
```
          }
```
```
          len = load_end - load;
```
```
  } else if (load != data) {
```

          loadbuf = map_sysmem(load, len);

          memcpy(loadbuf, buf, len);
  }

  /* verify that image data is a proper FDT blob */

  if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {

          puts("Subimage data is not a FDT");

```
          return -ENOEXEC;
```
```
  }
```

  bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);

```
  *datap = data;
```

  *datap = load;
  *lenp = len;
  if (fit_unamep)
          *fit_unamep = (char *)fit_uname;

-- 2.20.1

U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

Julius Werner

9:59 p.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

Hi Simon,

Is your approach similar to what I did here (decompressing transparently as part of fit_image_load())? I think I could easily expand this to other image types, I just don't always know how to test those. Really, the only thing that can't be decompressed there is the kernel image (because that is done later), so rather than checking for IH_TYPE_FLATDT I could check for !(IH_TYPE_KERNEL || IH_TYPE_KERNEL_NOLOAD)?

Alternatively, maybe we could take this as a first step and it can later be expanded to support other types when someone has time to work on that?

Simon Goldschmidt

10:06 p.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

On 18.04.19 21:59, Julius Werner wrote:

...

Hi Simon,

Is your approach similar to what I did here (decompressing transparently as part of fit_image_load())? I think I could easily expand this to other image types, I just don't always know how to test those. Really, the only thing that can't be decompressed there is the kernel image (because that is done later), so rather than checking for IH_TYPE_FLATDT I could check for !(IH_TYPE_KERNEL || IH_TYPE_KERNEL_NOLOAD)?

My approach was to uncompress all compressed images on-the-fly in fit_image_load(). I also hadn't fixed the kernel issue, so maybe deferring uncompression of kernel would be a good enough fix.

We had discussed that back then (around october'18) and I think we got to the point that it would be OK to use the "compressed" flag as a hint for U-Boot to uncompress things (meaning that a compressed initrd that should be passed to kernel compressed should not have the "compressed" flag in the FIT).

...

Alternatively, maybe we could take this as a first step and it can later be expanded to support other types when someone has time to work on that?

Or I could dig up my patches from October and we'll see how far you get with those?

Regards, Simon

Julius Werner

10:36 p.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

...

My approach was to uncompress all compressed images on-the-fly in fit_image_load().

Right, that's essentially what this patch is doing too.

...

Or I could dig up my patches from October and we'll see how far you get with those?

I think I found your patch: https://lists.denx.de/pipermail/u-boot/2018-October/344673.html

It seems to be doing something very close to what my patch does anyway. My patch goes a little further by also solving the case when no load address is given (in that case it will malloc() a buffer to decompress into with an upper bound guess for the required size). If there is a load size given, the two should end up doing the same thing. Also your patch works on all image types, which as you said there breaks it for kernels. I think the option of doing it for all image types except kernels would be a good solution for everyone. (Ultimately, I think it might be nicer if the kernel decompression would also be handled there and not as a special case, but I'd rather not tackle everything at once. This can always be iterated upon in the future.)

Simon Goldschmidt

10:48 p.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

On 18.04.19 22:36, Julius Werner wrote:

...

...
My approach was to uncompress all compressed images on-the-fly in fit_image_load().

Right, that's essentially what this patch is doing too.

Cool. I'm sorry I haven't found the time to dig into your patch for details (too much day-to-day work right now).

...

...
Or I could dig up my patches from October and we'll see how far you get with those?

I think I found your patch: https://lists.denx.de/pipermail/u-boot/2018-October/344673.html

Exactly. I do have some further versions locally, but no real breakthrough I think.

...

It seems to be doing something very close to what my patch does anyway. My patch goes a little further by also solving the case when no load address is given (in that case it will malloc() a buffer to decompress into with an upper bound guess for the required size).

Hmm, I think we might want to use the lmb functions here to allocate a buffer instead of relyling on malloc? The malloc pool might be large enough for an uncompressed devicetree, but not for an 8 MByte FPGA image...

But starting with malloc might be ok.

...

If there is a load size given, the two should end up doing the same thing. Also your patch works on all image types, which as you said there breaks it for kernels. I think the option of doing it for all image types except kernels would be a good solution for everyone. (Ultimately, I think it might be nicer if the kernel decompression would also be handled there and not as a special case, but I'd rather not tackle everything at once. This can always be iterated upon in the future.)

The reason I discontinued that patch was that I started adding a feature to mkimage to add a property for the uncompressed size. This is still pending work I haven't sent to the ML, but I do want to continue it once I find the time.

So maybe we could move on with a v2 of your patch that uncompresses everything but the kernel? I'd like to test that with my compressed FPGA images then.

Regards, Simon

Julius Werner

11:09 p.m.

New subject: [U-Boot] [PATCH 1/2] fit: Support FDT compression

...

Hmm, I think we might want to use the lmb functions here to allocate a buffer instead of relyling on malloc? The malloc pool might be large enough for an uncompressed devicetree, but not for an 8 MByte FPGA image...

But starting with malloc might be ok.

Okay, that sounds like a good possible improvement for later. For now, you can always just specify a load address for the FPGA image so that it doesn't need to allocate a separate buffer.

...

The reason I discontinued that patch was that I started adding a feature to mkimage to add a property for the uncompressed size. This is still pending work I haven't sent to the ML, but I do want to continue it once I find the time.

Yeah, that would make all of this a lot cleaner.

...

So maybe we could move on with a v2 of your patch that uncompresses everything but the kernel? I'd like to test that with my compressed FPGA images then.

Sounds good, v2 sent out.

Julius Werner

3:22 a.m.

New subject: [U-Boot] [PATCH 2/2] fit: Support compat string property in configuration node

This patch adds support for an optional optimization to compatible string matching where the compatible string property from the root node of the kernel FDT can be copied into the configuration node of the FIT image. This is most useful when using compressed FDTs or when using FDT overlays, where the traditional extraction of the compatible string from the kernel FDT itself is not easily possible.

Signed-off-by: Julius Werner jwerner@chromium.org --- common/image-fit.c | 67 +++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c index 20e02d240b..dec55ffaec 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1522,6 +1522,10 @@ int fit_check_format(const void *fit) * compatible list, "foo,bar", matches a compatible string in the root of fdt1. * "bim,bam" in fdt2 matches the second string which isn't as good as fdt1. * + * As an optimization, the compatible property from the FDT's root node can be + * copied into the configuration node in the FIT image. This is required to + * match configurations with compressed FDTs. + * * returns: * offset to the configuration to use if one was found * -1 otherwise @@ -1554,55 +1558,62 @@ int fit_conf_find_compat(const void *fit, const void *fdt) for (noffset = fdt_next_node(fit, confs_noffset, &ndepth); (noffset >= 0) && (ndepth > 0); noffset = fdt_next_node(fit, noffset, &ndepth)) { - const void *kfdt; + const void *fdt; const char *kfdt_name; - int kfdt_noffset; + int kfdt_noffset, compat_noffset; const char *cur_fdt_compat; int len; - size_t size; + size_t sz; int i;

if (ndepth > 1) continue;

- kfdt_name = fdt_getprop(fit, noffset, "fdt", &len); - if (!kfdt_name) { - debug("No fdt property found.\n"); - continue; - } - kfdt_noffset = fdt_subnode_offset(fit, images_noffset, - kfdt_name); - if (kfdt_noffset < 0) { - debug("No image node named "%s" found.\n", - kfdt_name); - continue; - } + /* If there's a compat property in the config node, use that. */ + if (fdt_getprop(fit, noffset, "compatible", NULL)) { + fdt = fit; /* search in FIT image */ + compat_noffset = noffset; /* search under config node */ + } else { /* Otherwise extract it from the kernel FDT. */ + kfdt_name = fdt_getprop(fit, noffset, "fdt", &len); + if (!kfdt_name) { + debug("No fdt property found.\n"); + continue; + } + kfdt_noffset = fdt_subnode_offset(fit, images_noffset, + kfdt_name); + if (kfdt_noffset < 0) { + debug("No image node named "%s" found.\n", + kfdt_name); + continue; + }

- if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) { - debug("Can't extract compat from "%s" (compressed)\n", - kfdt_name); - continue; - } + if (!fit_image_check_comp(fit, kfdt_noffset, + IH_COMP_NONE)) { + debug("Can't extract compat from "%s" " + "(compressed)\n", kfdt_name); + continue; + }

- /* - * Get a pointer to this configuration's fdt. - */ - if (fit_image_get_data(fit, kfdt_noffset, &kfdt, &size)) { - debug("Failed to get fdt "%s".\n", kfdt_name); - continue; + /* search in this config's kernel FDT */ + if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) { + debug("Failed to get fdt "%s".\n", kfdt_name); + continue; + } + + compat_noffset = 0; /* search kFDT under root node */ }

len = fdt_compat_len; cur_fdt_compat = fdt_compat; /* * Look for a match for each U-Boot compatibility string in - * turn in this configuration's fdt. + * turn in the compat string property. */ for (i = 0; len > 0 && (!best_match_offset || best_match_pos > i); i++) { int cur_len = strlen(cur_fdt_compat) + 1;

- if (!fdt_node_check_compatible(kfdt, 0, + if (!fdt_node_check_compatible(fdt, compat_noffset, cur_fdt_compat)) { best_match_offset = noffset; best_match_pos = i;

-- 2.20.1

2213

Age (days ago)

2213

Last active (days ago)

List overview

Download

8 comments

2 participants

tags (0)

participants (2)

Julius Werner
Simon Goldschmidt