[U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())

Simon Goldschmidt simon.k.r.goldschmidt at gmail.com
Fri Jul 12 06:09:32 UTC 2019


On Thu, Jul 11, 2019 at 10:53 PM Julius Werner <jwerner at chromium.org> wrote:
>
> Upcoming patches want to add decompression to use cases that are no
> longer directly related to booting. It makes sense to retain a single
> decompression routine, but it should no longer be in bootm.c (which is
> not compiled for all configurations). This patch moves
> bootm_decomp_image() to image.c and renames it to image_decomp() in
> preparation of those upcoming patches.

I'd like to review this, but maybe you can help me speed up the process and tell
us if the move has been a 1:1 code move or if you had to adapt things to the new
location (other than the function being renamed)?

Thanks,
Simon

>
> Signed-off-by: Julius Werner <jwerner at chromium.org>
> ---
>  - First version: v5
>
>  common/bootm.c     | 148 ++++++---------------------------------------
>  common/image.c     | 111 ++++++++++++++++++++++++++++++++++
>  include/bootm.h    |  17 ------
>  include/image.h    |  17 ++++++
>  test/compression.c |  24 ++++----
>  5 files changed, 160 insertions(+), 157 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index d193751647..2e64140df6 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -7,17 +7,12 @@
>  #ifndef USE_HOSTCC
>  #include <common.h>
>  #include <bootstage.h>
> -#include <bzlib.h>
>  #include <errno.h>
>  #include <fdt_support.h>
>  #include <lmb.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <asm/io.h>
> -#include <linux/lzo.h>
> -#include <lzma/LzmaTypes.h>
> -#include <lzma/LzmaDec.h>
> -#include <lzma/LzmaTools.h>
>  #if defined(CONFIG_CMD_USB)
>  #include <usb.h>
>  #endif
> @@ -299,23 +294,6 @@ static int bootm_find_other(cmd_tbl_t *cmdtp, int flag, int argc,
>  }
>  #endif /* USE_HOSTC */
>
> -/**
> - * print_decomp_msg() - Print a suitable decompression/loading message
> - *
> - * @type:      OS type (IH_OS_...)
> - * @comp_type: Compression type being used (IH_COMP_...)
> - * @is_xip:    true if the load address matches the image start
> - */
> -static void print_decomp_msg(int comp_type, int type, bool is_xip)
> -{
> -       const char *name = genimg_get_type_name(type);
> -
> -       if (comp_type == IH_COMP_NONE)
> -               printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
> -       else
> -               printf("   Uncompressing %s ... ", name);
> -}
> -
>  /**
>   * handle_decomp_error() - display a decompression error
>   *
> @@ -325,16 +303,18 @@ static void print_decomp_msg(int comp_type, int type, bool is_xip)
>   *
>   * @comp_type:         Compression type being used (IH_COMP_...)
>   * @uncomp_size:       Number of bytes uncompressed
> - * @unc_len:           Amount of space available for decompression
> - * @ret:               Error code to report
> - * @return BOOTM_ERR_RESET, indicating that the board must be reset
> + * @ret:               errno error code received from compression library
> + * @return Appropriate BOOTM_ERR_ error code
>   */
> -static int handle_decomp_error(int comp_type, size_t uncomp_size,
> -                              size_t unc_len, int ret)
> +static int handle_decomp_error(int comp_type, size_t uncomp_size, int ret)
>  {
>         const char *name = genimg_get_comp_name(comp_type);
>
> -       if (uncomp_size >= unc_len)
> +       /* ENOSYS means unimplemented compression type, don't reset. */
> +       if (ret == -ENOSYS)
> +               return BOOTM_ERR_UNIMPLEMENTED;
> +
> +       if (uncomp_size >= CONFIG_SYS_BOOTM_LEN)
>                 printf("Image too large: increase CONFIG_SYS_BOOTM_LEN\n");
>         else
>                 printf("%s: uncompress error %d\n", name, ret);
> @@ -352,93 +332,6 @@ static int handle_decomp_error(int comp_type, size_t uncomp_size,
>         return BOOTM_ERR_RESET;
>  }
>
> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> -                      void *load_buf, void *image_buf, ulong image_len,
> -                      uint unc_len, ulong *load_end)
> -{
> -       int ret = 0;
> -
> -       *load_end = load;
> -       print_decomp_msg(comp, type, load == image_start);
> -
> -       /*
> -        * Load the image to the right place, decompressing if needed. After
> -        * this, image_len will be set to the number of uncompressed bytes
> -        * loaded, ret will be non-zero on error.
> -        */
> -       switch (comp) {
> -       case IH_COMP_NONE:
> -               if (load == image_start)
> -                       break;
> -               if (image_len <= unc_len)
> -                       memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
> -               else
> -                       ret = 1;
> -               break;
> -#ifdef CONFIG_GZIP
> -       case IH_COMP_GZIP: {
> -               ret = gunzip(load_buf, unc_len, image_buf, &image_len);
> -               break;
> -       }
> -#endif /* CONFIG_GZIP */
> -#ifdef CONFIG_BZIP2
> -       case IH_COMP_BZIP2: {
> -               uint size = unc_len;
> -
> -               /*
> -                * If we've got less than 4 MB of malloc() space,
> -                * use slower decompression algorithm which requires
> -                * at most 2300 KB of memory.
> -                */
> -               ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
> -                       image_buf, image_len,
> -                       CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_BZIP2 */
> -#ifdef CONFIG_LZMA
> -       case IH_COMP_LZMA: {
> -               SizeT lzma_len = unc_len;
> -
> -               ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
> -                                              image_buf, image_len);
> -               image_len = lzma_len;
> -               break;
> -       }
> -#endif /* CONFIG_LZMA */
> -#ifdef CONFIG_LZO
> -       case IH_COMP_LZO: {
> -               size_t size = unc_len;
> -
> -               ret = lzop_decompress(image_buf, image_len, load_buf, &size);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_LZO */
> -#ifdef CONFIG_LZ4
> -       case IH_COMP_LZ4: {
> -               size_t size = unc_len;
> -
> -               ret = ulz4fn(image_buf, image_len, load_buf, &size);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_LZ4 */
> -       default:
> -               printf("Unimplemented compression type %d\n", comp);
> -               return BOOTM_ERR_UNIMPLEMENTED;
> -       }
> -
> -       if (ret)
> -               return handle_decomp_error(comp, image_len, unc_len, ret);
> -       *load_end = load + image_len;
> -
> -       puts("OK\n");
> -
> -       return 0;
> -}
> -
>  #ifndef USE_HOSTCC
>  static int bootm_load_os(bootm_headers_t *images, int boot_progress)
>  {
> @@ -456,10 +349,11 @@ static int bootm_load_os(bootm_headers_t *images, int boot_progress)
>
>         load_buf = map_sysmem(load, 0);
>         image_buf = map_sysmem(os.image_start, image_len);
> -       err = bootm_decomp_image(os.comp, load, os.image_start, os.type,
> -                                load_buf, image_buf, image_len,
> -                                CONFIG_SYS_BOOTM_LEN, &load_end);
> +       err = image_decomp(os.comp, load, os.image_start, os.type,
> +                          load_buf, image_buf, image_len,
> +                          CONFIG_SYS_BOOTM_LEN, &load_end);
>         if (err) {
> +               err = handle_decomp_error(os.comp, load_end - load, err);
>                 bootstage_error(BOOTSTAGE_ID_DECOMP_IMAGE);
>                 return err;
>         }
> @@ -919,11 +813,6 @@ void __weak switch_to_non_secure_mode(void)
>
>  #else /* USE_HOSTCC */
>
> -void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
> -{
> -       memmove(to, from, len);
> -}
> -
>  #if defined(CONFIG_FIT_SIGNATURE)
>  static int bootm_host_load_image(const void *fit, int req_image_type)
>  {
> @@ -957,13 +846,16 @@ static int bootm_host_load_image(const void *fit, int req_image_type)
>
>         /* Allow the image to expand by a factor of 4, should be safe */
>         load_buf = malloc((1 << 20) + len * 4);
> -       ret = bootm_decomp_image(imape_comp, 0, data, image_type, load_buf,
> -                                (void *)data, len, CONFIG_SYS_BOOTM_LEN,
> -                                &load_end);
> +       ret = image_decomp(imape_comp, 0, data, image_type, load_buf,
> +                          (void *)data, len, CONFIG_SYS_BOOTM_LEN,
> +                          &load_end);
>         free(load_buf);
>
> -       if (ret && ret != BOOTM_ERR_UNIMPLEMENTED)
> -               return ret;
> +       if (ret) {
> +               ret = handle_decomp_error(imape_comp, load_end - 0, ret);
> +               if (ret != BOOTM_ERR_UNIMPLEMENTED)
> +                       return ret;
> +       }
>
>         return 0;
>  }
> diff --git a/common/image.c b/common/image.c
> index 75b84d5009..04da002fcc 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -32,6 +32,12 @@
>  #include <linux/errno.h>
>  #include <asm/io.h>
>
> +#include <bzlib.h>
> +#include <linux/lzo.h>
> +#include <lzma/LzmaTypes.h>
> +#include <lzma/LzmaDec.h>
> +#include <lzma/LzmaTools.h>
> +
>  #ifdef CONFIG_CMD_BDI
>  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>  #endif
> @@ -375,6 +381,106 @@ void image_print_contents(const void *ptr)
>         }
>  }
>
> +/**
> + * print_decomp_msg() - Print a suitable decompression/loading message
> + *
> + * @type:      OS type (IH_OS_...)
> + * @comp_type: Compression type being used (IH_COMP_...)
> + * @is_xip:    true if the load address matches the image start
> + */
> +static void print_decomp_msg(int comp_type, int type, bool is_xip)
> +{
> +       const char *name = genimg_get_type_name(type);
> +
> +       if (comp_type == IH_COMP_NONE)
> +               printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
> +       else
> +               printf("   Uncompressing %s ... ", name);
> +}
> +
> +int image_decomp(int comp, ulong load, ulong image_start, int type,
> +                void *load_buf, void *image_buf, ulong image_len,
> +                uint unc_len, ulong *load_end)
> +{
> +       int ret = 0;
> +
> +       *load_end = load;
> +       print_decomp_msg(comp, type, load == image_start);
> +
> +       /*
> +        * Load the image to the right place, decompressing if needed. After
> +        * this, image_len will be set to the number of uncompressed bytes
> +        * loaded, ret will be non-zero on error.
> +        */
> +       switch (comp) {
> +       case IH_COMP_NONE:
> +               if (load == image_start)
> +                       break;
> +               if (image_len <= unc_len)
> +                       memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
> +               else
> +                       ret = -ENOSPC;
> +               break;
> +#ifdef CONFIG_GZIP
> +       case IH_COMP_GZIP: {
> +               ret = gunzip(load_buf, unc_len, image_buf, &image_len);
> +               break;
> +       }
> +#endif /* CONFIG_GZIP */
> +#ifdef CONFIG_BZIP2
> +       case IH_COMP_BZIP2: {
> +               uint size = unc_len;
> +
> +               /*
> +                * If we've got less than 4 MB of malloc() space,
> +                * use slower decompression algorithm which requires
> +                * at most 2300 KB of memory.
> +                */
> +               ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
> +                       image_buf, image_len,
> +                       CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_BZIP2 */
> +#ifdef CONFIG_LZMA
> +       case IH_COMP_LZMA: {
> +               SizeT lzma_len = unc_len;
> +
> +               ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
> +                                              image_buf, image_len);
> +               image_len = lzma_len;
> +               break;
> +       }
> +#endif /* CONFIG_LZMA */
> +#ifdef CONFIG_LZO
> +       case IH_COMP_LZO: {
> +               size_t size = unc_len;
> +
> +               ret = lzop_decompress(image_buf, image_len, load_buf, &size);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_LZO */
> +#ifdef CONFIG_LZ4
> +       case IH_COMP_LZ4: {
> +               size_t size = unc_len;
> +
> +               ret = ulz4fn(image_buf, image_len, load_buf, &size);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_LZ4 */
> +       default:
> +               printf("Unimplemented compression type %d\n", comp);
> +               return -ENOSYS;
> +       }
> +
> +       *load_end = load + image_len;
> +
> +       return ret;
> +}
> +
>
>  #ifndef USE_HOSTCC
>  #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
> @@ -551,6 +657,11 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
>         memmove(to, from, len);
>  #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
>  }
> +#else  /* USE_HOSTCC */
> +void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
> +{
> +       memmove(to, from, len);
> +}
>  #endif /* !USE_HOSTCC */
>
>  void genimg_print_size(uint32_t size)
> diff --git a/include/bootm.h b/include/bootm.h
> index f771b733f5..edeeacb0df 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -59,23 +59,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>
>  void arch_preboot_os(void);
>
> -/**
> - * bootm_decomp_image() - decompress the operating system
> - *
> - * @comp:      Compression algorithm that is used (IH_COMP_...)
> - * @load:      Destination load address in U-Boot memory
> - * @image_start Image start address (where we are decompressing from)
> - * @type:      OS type (IH_OS_...)
> - * @load_bug:  Place to decompress to
> - * @image_buf: Address to decompress from
> - * @image_len: Number of bytes in @image_buf to decompress
> - * @unc_len:   Available space for decompression
> - * @return 0 if OK, -ve on error (BOOTM_ERR_...)
> - */
> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> -                      void *load_buf, void *image_buf, ulong image_len,
> -                      uint unc_len, ulong *load_end);
> -
>  /*
>   * boards should define this to disable devices when EFI exits from boot
>   * services.
> diff --git a/include/image.h b/include/image.h
> index bb7089ef5d..3bdb9f0a97 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -849,6 +849,23 @@ static inline int image_check_target_arch(const image_header_t *hdr)
>  }
>  #endif /* USE_HOSTCC */
>
> +/**
> + * image_decomp() - decompress an image
> + *
> + * @comp:      Compression algorithm that is used (IH_COMP_...)
> + * @load:      Destination load address in U-Boot memory
> + * @image_start Image start address (where we are decompressing from)
> + * @type:      OS type (IH_OS_...)
> + * @load_bug:  Place to decompress to
> + * @image_buf: Address to decompress from
> + * @image_len: Number of bytes in @image_buf to decompress
> + * @unc_len:   Available space for decompression
> + * @return 0 if OK, -ve on error (BOOTM_ERR_...)
> + */
> +int image_decomp(int comp, ulong load, ulong image_start, int type,
> +                void *load_buf, void *image_buf, ulong image_len,
> +                uint unc_len, ulong *load_end);
> +
>  /**
>   * Set up properties in the FDT
>   *
> diff --git a/test/compression.c b/test/compression.c
> index 7bc0f73e09..dc5e94684f 100644
> --- a/test/compression.c
> +++ b/test/compression.c
> @@ -471,15 +471,15 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
>         unc_len = strlen(plain);
>         compress(uts, (void *)plain, unc_len, compress_buff, compress_size,
>                  &compress_size);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, unc_len,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, unc_len,
> +                          &load_end);
>         ut_assertok(err);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, unc_len - 1,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, unc_len - 1,
> +                          &load_end);
>         ut_assert(err);
>
>         /* We can't detect corruption when not decompressing */
> @@ -487,10 +487,10 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
>                 return 0;
>         memset(compress_buff + compress_size / 2, '\x49',
>                compress_size / 2);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, 0x10000,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, 0x10000,
> +                          &load_end);
>         ut_assert(err);
>
>         return 0;
> --
> 2.20.1
>


More information about the U-Boot mailing list