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

On Thu, Jul 11, 2019 at 10:53 PM Julius Werner jwerner@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@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