[PATCH 1/3] spl: fit: Discard decompression if not supported

[PATCH 1/3] spl: fit: Discard decompression if not supported

[Loic Poulain]

And simplify further decompression testing.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 common/spl/spl_fit.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 70d8d5942d..1d42cb1d10 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -208,6 +208,20 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	return (data_size + info->bl_len - 1) / info->bl_len;
 }
 
+static inline bool spl_fit_decompression_supported(uint8_t comp)
+{
+	switch (comp) {
+	case IH_COMP_GZIP:
+		return IS_ENABLED(CONFIG_SPL_GZIP);
+	case IH_COMP_LZMA:
+		return IS_ENABLED(CONFIG_SPL_LZMA);
+	case IH_COMP_NONE:
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * load_simple_fit(): load the image described in a certain FIT node
  * @info:	points to information about the device to load data from
@@ -235,7 +249,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 	void *src;
 	ulong overhead;
 	int nr_sectors;
-	uint8_t image_comp = -1, type = -1;
+	uint8_t image_comp, type = -1;
 	const void *data;
 	const void *fit = ctx->fit;
 	bool external_data = false;
@@ -248,9 +262,11 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 			debug("%s ", genimg_get_type_name(type));
 	}
 
-	if (spl_decompression_enabled()) {
-		fit_image_get_comp(fit, node, &image_comp);
-		debug("%s ", genimg_get_comp_name(image_comp));
+	fit_image_get_comp(fit, node, &image_comp);
+	if (!spl_fit_decompression_supported(image_comp)) {
+		debug("Discard unsupported compression %s ",
+		      genimg_get_comp_name(image_comp));
+		image_comp = IH_COMP_NONE;
 	}
 
 	if (fit_image_get_load(fit, node, &load_addr)) {
@@ -283,8 +299,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 			return 0;
 		}
 
-		if (spl_decompression_enabled() &&
-		    (image_comp == IH_COMP_GZIP || image_comp == IH_COMP_LZMA))
+		if (image_comp != IH_COMP_NONE)
 			src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR, ARCH_DMA_MINALIGN), len);
 		else
 			src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
-- 
2.34.1

[PATCH 2/3] spl: fit: Add support for LZO compressed images

[Loic Poulain]

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 common/spl/spl_fit.c | 10 ++++++++++
 include/spl.h        |  4 +++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1d42cb1d10..08428660b0 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -18,6 +18,7 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <linux/libfdt.h>
+#include <linux/lzo.h>
 #include <linux/printk.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -215,6 +216,8 @@ static inline bool spl_fit_decompression_supported(uint8_t comp)
 		return IS_ENABLED(CONFIG_SPL_GZIP);
 	case IH_COMP_LZMA:
 		return IS_ENABLED(CONFIG_SPL_LZMA);
+	case IH_COMP_LZO:
+		return IS_ENABLED(CONFIG_SPL_LZO);
 	case IH_COMP_NONE:
 		return true;
 	}
@@ -357,6 +360,13 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 			return -EIO;
 		}
 		length = loadEnd - CONFIG_SYS_LOAD_ADDR;
+	} else if (IS_ENABLED(CONFIG_SPL_LZO) && image_comp == IH_COMP_LZO) {
+		size = CONFIG_SYS_BOOTM_LEN;
+		if (lzop_decompress(src, length, load_ptr, &size)) {
+			puts("Uncompressing error\n");
+			return -EIO;
+		}
+		length = size;
 	} else {
 		memcpy(load_ptr, src, length);
 	}
diff --git a/include/spl.h b/include/spl.h
index 8ff20adc28..e07092372a 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -1016,6 +1016,8 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
  */
 static inline bool spl_decompression_enabled(void)
 {
-	return IS_ENABLED(CONFIG_SPL_GZIP) || IS_ENABLED(CONFIG_SPL_LZMA);
+	return IS_ENABLED(CONFIG_SPL_GZIP) || IS_ENABLED(CONFIG_SPL_LZMA) ||
+		IS_ENABLED(CONFIG_SPL_LZO);
 }
+
 #endif
-- 
2.34.1

[PATCH 3/3] spl: fit: Load compressed blob to heap buffer

[Loic Poulain]

CONFIG_SYS_LOAD_ADDR is usually configured as the address where
the kernel should be loaded at. It can be problematic to use it
as a generic temporary buffer for FIT compressed blobs.

An example is when the image is a compressed kernel with load
address equal to CONFIG_SYS_LOAD_ADDR, this causes (inplace)
decompression to fail.

Instead we can simply allocate a temporary buffer in the heap

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 common/spl/spl_fit.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 08428660b0..8a807db5ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -249,7 +249,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 	ulong size;
 	ulong load_addr;
 	void *load_ptr;
-	void *src;
+	void *src, *src_ptr;
 	ulong overhead;
 	int nr_sectors;
 	uint8_t image_comp, type = -1;
@@ -289,8 +289,6 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 	}
 
 	if (external_data) {
-		void *src_ptr;
-
 		/* External data */
 		if (fit_image_get_data_size(fit, node, &len))
 			return -ENOENT;
@@ -302,10 +300,13 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 			return 0;
 		}
 
-		if (image_comp != IH_COMP_NONE)
-			src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR, ARCH_DMA_MINALIGN), len);
-		else
+		if (image_comp != IH_COMP_NONE) {
+			src_ptr = malloc_cache_aligned(len + 2 * info->bl_len);
+			if (!src_ptr)
+				return -ENOMEM;
+		} else {
 			src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
+		}
 		length = len;
 
 		overhead = get_aligned_image_overhead(info, offset);
@@ -383,6 +384,9 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
 			image_info->entry_point = FDT_ERROR;
 	}
 
+	if (external_data && image_comp != IH_COMP_NONE)
+		free(src_ptr);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 1/3] spl: fit: Discard decompression if not supported

[Sean Anderson]

On 11/3/23 10:34, Loic Poulain wrote:
> And simplify further decompression testing.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   common/spl/spl_fit.c | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 70d8d5942d..1d42cb1d10 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -208,6 +208,20 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>   	return (data_size + info->bl_len - 1) / info->bl_len;
>   }
>   
> +static inline bool spl_fit_decompression_supported(uint8_t comp)
> +{
> +	switch (comp) {
> +	case IH_COMP_GZIP:
> +		return IS_ENABLED(CONFIG_SPL_GZIP);
> +	case IH_COMP_LZMA:
> +		return IS_ENABLED(CONFIG_SPL_LZMA);
> +	case IH_COMP_NONE:
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>   /**
>    * load_simple_fit(): load the image described in a certain FIT node
>    * @info:	points to information about the device to load data from
> @@ -235,7 +249,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   	void *src;
>   	ulong overhead;
>   	int nr_sectors;
> -	uint8_t image_comp = -1, type = -1;
> +	uint8_t image_comp, type = -1;
>   	const void *data;
>   	const void *fit = ctx->fit;
>   	bool external_data = false;
> @@ -248,9 +262,11 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   			debug("%s ", genimg_get_type_name(type));
>   	}
>   
> -	if (spl_decompression_enabled()) {
> -		fit_image_get_comp(fit, node, &image_comp);
> -		debug("%s ", genimg_get_comp_name(image_comp));
> +	fit_image_get_comp(fit, node, &image_comp);
> +	if (!spl_fit_decompression_supported(image_comp)) {
> +		debug("Discard unsupported compression %s ",
> +		      genimg_get_comp_name(image_comp));
> +		image_comp = IH_COMP_NONE;
>   	}
>   
>   	if (fit_image_get_load(fit, node, &load_addr)) {
> @@ -283,8 +299,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   			return 0;
>   		}
>   
> -		if (spl_decompression_enabled() &&
> -		    (image_comp == IH_COMP_GZIP || image_comp == IH_COMP_LZMA))
> +		if (image_comp != IH_COMP_NONE)
>   			src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR, ARCH_DMA_MINALIGN), len);
>   		else
>   			src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);

Why doesn't spl_decompression_enabled work here?

--Sean

Re: [PATCH 3/3] spl: fit: Load compressed blob to heap buffer

[Sean Anderson]

On 11/3/23 10:34, Loic Poulain wrote:
> CONFIG_SYS_LOAD_ADDR is usually configured as the address where
> the kernel should be loaded at. It can be problematic to use it
> as a generic temporary buffer for FIT compressed blobs.
> 
> An example is when the image is a compressed kernel with load
> address equal to CONFIG_SYS_LOAD_ADDR, this causes (inplace)
> decompression to fail.
> 
> Instead we can simply allocate a temporary buffer in the heap
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   common/spl/spl_fit.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 08428660b0..8a807db5ba 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -249,7 +249,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   	ulong size;
>   	ulong load_addr;
>   	void *load_ptr;
> -	void *src;
> +	void *src, *src_ptr;
>   	ulong overhead;
>   	int nr_sectors;
>   	uint8_t image_comp, type = -1;
> @@ -289,8 +289,6 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   	}
>   
>   	if (external_data) {
> -		void *src_ptr;
> -
>   		/* External data */
>   		if (fit_image_get_data_size(fit, node, &len))
>   			return -ENOENT;
> @@ -302,10 +300,13 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   			return 0;
>   		}
>   
> -		if (image_comp != IH_COMP_NONE)
> -			src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR, ARCH_DMA_MINALIGN), len);
> -		else
> +		if (image_comp != IH_COMP_NONE) {
> +			src_ptr = malloc_cache_aligned(len + 2 * info->bl_len);
> +			if (!src_ptr)
> +				return -ENOMEM;
> +		} else {
>   			src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
> +		}
>   		length = len;
>   
>   		overhead = get_aligned_image_overhead(info, offset);
> @@ -383,6 +384,9 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   			image_info->entry_point = FDT_ERROR;
>   	}
>   
> +	if (external_data && image_comp != IH_COMP_NONE)
> +		free(src_ptr);
> +
>   	return 0;
>   }
>   

Why not use spl_simple_fit_read?

--Sean

Re: [PATCH 2/3] spl: fit: Add support for LZO compressed images

[Sean Anderson]

On 11/3/23 10:34, Loic Poulain wrote:
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Please add an appropriate commit message.

> ---
>   common/spl/spl_fit.c | 10 ++++++++++
>   include/spl.h        |  4 +++-
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 1d42cb1d10..08428660b0 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -18,6 +18,7 @@
>   #include <asm/global_data.h>
>   #include <asm/io.h>
>   #include <linux/libfdt.h>
> +#include <linux/lzo.h>
>   #include <linux/printk.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
> @@ -215,6 +216,8 @@ static inline bool spl_fit_decompression_supported(uint8_t comp)
>   		return IS_ENABLED(CONFIG_SPL_GZIP);
>   	case IH_COMP_LZMA:
>   		return IS_ENABLED(CONFIG_SPL_LZMA);
> +	case IH_COMP_LZO:
> +		return IS_ENABLED(CONFIG_SPL_LZO);
>   	case IH_COMP_NONE:
>   		return true;
>   	}
> @@ -357,6 +360,13 @@ static int load_simple_fit(struct spl_load_info *info, ulong sector,
>   			return -EIO;
>   		}
>   		length = loadEnd - CONFIG_SYS_LOAD_ADDR;
> +	} else if (IS_ENABLED(CONFIG_SPL_LZO) && image_comp == IH_COMP_LZO) {
> +		size = CONFIG_SYS_BOOTM_LEN;
> +		if (lzop_decompress(src, length, load_ptr, &size)) {
> +			puts("Uncompressing error\n");
> +			return -EIO;
> +		}
> +		length = size;
>   	} else {
>   		memcpy(load_ptr, src, length);
>   	}
> diff --git a/include/spl.h b/include/spl.h
> index 8ff20adc28..e07092372a 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -1016,6 +1016,8 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
>    */
>   static inline bool spl_decompression_enabled(void)
>   {
> -	return IS_ENABLED(CONFIG_SPL_GZIP) || IS_ENABLED(CONFIG_SPL_LZMA);
> +	return IS_ENABLED(CONFIG_SPL_GZIP) || IS_ENABLED(CONFIG_SPL_LZMA) ||
> +		IS_ENABLED(CONFIG_SPL_LZO);
>   }
> +
>   #endif

Please also add a decompression test. I think spl_test_image should be extended to
test all decompression types (see do_spl_test_load for inspiration). do_spl_test_load
should also be extended to test LZMA FITs.

--Sean