[PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Jules Maselbas]

Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
which include spelling fixes and the new ARM64 BCJ decoder which was recently
introduced into .xz file format version 1.1.0 (2022-12-11) [2].

[1] https://git.tukaani.org/?p=xz-embedded.git
[2] https://tukaani.org/xz/xz-file-format-1.1.0.txt

Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
---
 lib/Kconfig           |  4 ++++
 lib/decompress_unxz.c |  3 +++
 lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
 lib/xz/xz_private.h   |  3 +++
 4 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 758197b608..9291e5a8ff 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -42,6 +42,7 @@ config XZ_DECOMPRESS
 	select XZ_DEC_ARM
 	select XZ_DEC_ARMTHUMB
 	select XZ_DEC_SPARC
+	select XZ_DEC_ARM64
 
 config XZ_DEC_X86
         bool
@@ -61,6 +62,9 @@ config XZ_DEC_ARMTHUMB
 config XZ_DEC_SPARC
         bool
 
+config XZ_DEC_ARM64
+	bool "ARM64 BCJ filter decoder"
+
 config REED_SOLOMON
 	bool
 
diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
index a7e2d331ab..5c906932f8 100644
--- a/lib/decompress_unxz.c
+++ b/lib/decompress_unxz.c
@@ -132,6 +132,9 @@
 #endif
 #ifdef CONFIG_ARM
 #	define XZ_DEC_ARM
+#	ifdef CONFIG_CPU_64
+#		define XZ_DEC_ARM64
+#	endif
 #endif
 #ifdef CONFIG_IA64
 #	define XZ_DEC_IA64
diff --git a/lib/xz/xz_dec_bcj.c b/lib/xz/xz_dec_bcj.c
index d268adbc65..d40fae3416 100644
--- a/lib/xz/xz_dec_bcj.c
+++ b/lib/xz/xz_dec_bcj.c
@@ -2,7 +2,7 @@
  * Branch/Call/Jump (BCJ) filter decoders
  *
  * Authors: Lasse Collin <lasse.collin@tukaani.org>
- *          Igor Pavlov <http://7-zip.org/>
+ *          Igor Pavlov <https://7-zip.org/>
  *
  * This file has been put into the public domain.
  * You can do whatever you want with this file.
@@ -24,7 +24,8 @@ struct xz_dec_bcj {
 		BCJ_IA64 = 6,       /* Big or little endian */
 		BCJ_ARM = 7,        /* Little endian only */
 		BCJ_ARMTHUMB = 8,   /* Little endian only */
-		BCJ_SPARC = 9       /* Big or little endian */
+		BCJ_SPARC = 9,      /* Big or little endian */
+		BCJ_ARM64 = 10      /* AArch64 */
 	} type;
 
 	/*
@@ -334,6 +335,45 @@ static size_t bcj_sparc(struct xz_dec_bcj *s, uint8_t *buf, size_t size)
 }
 #endif
 
+#ifdef XZ_DEC_ARM64
+static size_t bcj_arm64(struct xz_dec_bcj *s, uint8_t *buf, size_t size)
+{
+	size_t i;
+	uint32_t instr;
+	uint32_t addr;
+
+	for (i = 0; i + 4 <= size; i += 4) {
+		instr = get_unaligned_le32(buf + i);
+
+		if ((instr >> 26) == 0x25) {
+			/* BL instruction */
+			addr = instr - ((s->pos + (uint32_t)i) >> 2);
+			instr = 0x94000000 | (addr & 0x03FFFFFF);
+			put_unaligned_le32(instr, buf + i);
+
+		} else if ((instr & 0x9F000000) == 0x90000000) {
+			/* ADRP instruction */
+			addr = ((instr >> 29) & 3) | ((instr >> 3) & 0x1FFFFC);
+
+			/* Only convert values in the range +/-512 MiB. */
+			if ((addr + 0x020000) & 0x1C0000)
+				continue;
+
+			addr -= (s->pos + (uint32_t)i) >> 12;
+
+			instr &= 0x9000001F;
+			instr |= (addr & 3) << 29;
+			instr |= (addr & 0x03FFFC) << 3;
+			instr |= (0U - (addr & 0x020000)) & 0xE00000;
+
+			put_unaligned_le32(instr, buf + i);
+		}
+	}
+
+	return i;
+}
+#endif
+
 /*
  * Apply the selected BCJ filter. Update *pos and s->pos to match the amount
  * of data that got filtered.
@@ -380,6 +420,11 @@ static void bcj_apply(struct xz_dec_bcj *s,
 	case BCJ_SPARC:
 		filtered = bcj_sparc(s, buf, size);
 		break;
+#endif
+#ifdef XZ_DEC_ARM64
+	case BCJ_ARM64:
+		filtered = bcj_arm64(s, buf, size);
+		break;
 #endif
 	default:
 		/* Never reached but silence compiler warnings. */
@@ -422,7 +467,7 @@ XZ_EXTERN enum xz_ret xz_dec_bcj_run(struct xz_dec_bcj *s,
 
 	/*
 	 * Flush pending already filtered data to the output buffer. Return
-	 * immediatelly if we couldn't flush everything, or if the next
+	 * immediately if we couldn't flush everything, or if the next
 	 * filter in the chain had already returned XZ_STREAM_END.
 	 */
 	if (s->temp.filtered > 0) {
@@ -553,6 +598,9 @@ XZ_EXTERN enum xz_ret xz_dec_bcj_reset(struct xz_dec_bcj *s, uint8_t id)
 #endif
 #ifdef XZ_DEC_SPARC
 	case BCJ_SPARC:
+#endif
+#ifdef XZ_DEC_ARM64
+	case BCJ_ARM64:
 #endif
 		break;
 
diff --git a/lib/xz/xz_private.h b/lib/xz/xz_private.h
index 85f79635f0..9d6bd7c905 100644
--- a/lib/xz/xz_private.h
+++ b/lib/xz/xz_private.h
@@ -37,6 +37,9 @@
 #		ifdef CONFIG_XZ_DEC_SPARC
 #			define XZ_DEC_SPARC
 #		endif
+#		ifdef CONFIG_XZ_DEC_ARM64
+#			define XZ_DEC_ARM64
+#		endif
 #		define memeq(a, b, size) (memcmp(a, b, size) == 0)
 #		define memzero(buf, size) memset(buf, 0, size)
 #		define FREE free
-- 
2.41.0

[PATCH 2/3] scripts: Select XZ --arm64 BCJ filter for 64-bit arm

[Jules Maselbas]

This change will select the arm64 BCJ filter for aarch64 instead of
the arm filter which was always selected for arm based architectures.

The selected filter is also tested, by looking if it is present in
the long help message of xz, this is because the arm64 BCJ filter is
a recent addition to xz.

Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
---
 scripts/xz_wrap.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/xz_wrap.sh b/scripts/xz_wrap.sh
index 7a2d372f48..5b5f3adcff 100755
--- a/scripts/xz_wrap.sh
+++ b/scripts/xz_wrap.sh
@@ -16,8 +16,11 @@ case $SRCARCH in
 	x86)            BCJ=--x86 ;;
 	powerpc)        BCJ=--powerpc ;;
 	ia64)           BCJ=--ia64; LZMA2OPTS=pb=4 ;;
-	arm)            BCJ=--arm ;;
+	arm)            BCJ=--arm$S64 ;;
 	sparc)          BCJ=--sparc ;;
 esac
 
+# clear BCJ filter if unsupported
+xz -H | grep -q -- $BCJ || BCJ=
+
 exec xz --check=crc32 $BCJ --lzma2=$LZMA2OPTS,dict=32MiB
-- 
2.41.0

[PATCH 3/3] kbuild: remove duplicated xz compressions target

[Jules Maselbas]

Both cmd_xzkern and cmd_xzmisc where already defined in the same file

Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
---
 scripts/Makefile.lib | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f04c09f9e2..6baa618f15 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -470,34 +470,6 @@ cmd_lzo = (cat $(filter-out FORCE,$^) | \
 %.lzo: %
 	$(call if_changed,lzo)
 
-# XZ
-# ---------------------------------------------------------------------------
-# Use xzkern to compress the kernel image and xzmisc to compress other things.
-#
-# xzkern uses a big LZMA2 dictionary since it doesn't increase memory usage
-# of the kernel decompressor. A BCJ filter is used if it is available for
-# the target architecture. xzkern also appends uncompressed size of the data
-# using size_append. The .xz format has the size information available at
-# the end of the file too, but it's in more complex format and it's good to
-# avoid changing the part of the boot code that reads the uncompressed size.
-# Note that the bytes added by size_append will make the xz tool think that
-# the file is corrupt. This is expected.
-#
-# xzmisc doesn't use size_append, so it can be used to create normal .xz
-# files. xzmisc uses smaller LZMA2 dictionary than xzkern, because a very
-# big dictionary would increase the memory usage too much in the multi-call
-# decompression mode. A BCJ filter isn't used either.
-quiet_cmd_xzkern = XZKERN  $@
-cmd_xzkern = (cat $(filter-out FORCE,$^) | \
-	sh $(srctree)/scripts/xz_wrap.sh && \
-	$(call size_append, $(filter-out FORCE,$^))) > $@ || \
-	(rm -f $@ ; false)
-
-quiet_cmd_xzmisc = XZMISC  $@
-cmd_xzmisc = (cat $(filter-out FORCE,$^) | \
-	xz --check=crc32 --lzma2=dict=1MiB) > $@ || \
-	(rm -f $@ ; false)
-
 # lz4
 # ---------------------------------------------------------------------------
 
-- 
2.41.0

Re: [PATCH 3/3] kbuild: remove duplicated xz compressions target

[Ahmad Fatoum]

On 06.09.23 17:08, Jules Maselbas wrote:
> Both cmd_xzkern and cmd_xzmisc where already defined in the same file
> 
> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  scripts/Makefile.lib | 28 ----------------------------
>  1 file changed, 28 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index f04c09f9e2..6baa618f15 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -470,34 +470,6 @@ cmd_lzo = (cat $(filter-out FORCE,$^) | \
>  %.lzo: %
>  	$(call if_changed,lzo)
>  
> -# XZ
> -# ---------------------------------------------------------------------------
> -# Use xzkern to compress the kernel image and xzmisc to compress other things.
> -#
> -# xzkern uses a big LZMA2 dictionary since it doesn't increase memory usage
> -# of the kernel decompressor. A BCJ filter is used if it is available for
> -# the target architecture. xzkern also appends uncompressed size of the data
> -# using size_append. The .xz format has the size information available at
> -# the end of the file too, but it's in more complex format and it's good to
> -# avoid changing the part of the boot code that reads the uncompressed size.
> -# Note that the bytes added by size_append will make the xz tool think that
> -# the file is corrupt. This is expected.
> -#
> -# xzmisc doesn't use size_append, so it can be used to create normal .xz
> -# files. xzmisc uses smaller LZMA2 dictionary than xzkern, because a very
> -# big dictionary would increase the memory usage too much in the multi-call
> -# decompression mode. A BCJ filter isn't used either.
> -quiet_cmd_xzkern = XZKERN  $@
> -cmd_xzkern = (cat $(filter-out FORCE,$^) | \
> -	sh $(srctree)/scripts/xz_wrap.sh && \
> -	$(call size_append, $(filter-out FORCE,$^))) > $@ || \
> -	(rm -f $@ ; false)
> -
> -quiet_cmd_xzmisc = XZMISC  $@
> -cmd_xzmisc = (cat $(filter-out FORCE,$^) | \
> -	xz --check=crc32 --lzma2=dict=1MiB) > $@ || \
> -	(rm -f $@ ; false)
> -
>  # lz4
>  # ---------------------------------------------------------------------------
>  

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/3] scripts: Select XZ --arm64 BCJ filter for 64-bit arm

[Ahmad Fatoum]

On 06.09.23 17:08, Jules Maselbas wrote:
> This change will select the arm64 BCJ filter for aarch64 instead of
> the arm filter which was always selected for arm based architectures.
> 
> The selected filter is also tested, by looking if it is present in
> the long help message of xz, this is because the arm64 BCJ filter is
> a recent addition to xz.
> 
> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>

How much of an improvement is BCJ=--arm over BCJ=""?

I wonder if we should fallback from --arm64 to --arm.

Cheers,
Ahmad

> ---
>  scripts/xz_wrap.sh | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/xz_wrap.sh b/scripts/xz_wrap.sh
> index 7a2d372f48..5b5f3adcff 100755
> --- a/scripts/xz_wrap.sh
> +++ b/scripts/xz_wrap.sh
> @@ -16,8 +16,11 @@ case $SRCARCH in
>  	x86)            BCJ=--x86 ;;
>  	powerpc)        BCJ=--powerpc ;;
>  	ia64)           BCJ=--ia64; LZMA2OPTS=pb=4 ;;
> -	arm)            BCJ=--arm ;;
> +	arm)            BCJ=--arm$S64 ;;
>  	sparc)          BCJ=--sparc ;;
>  esac
>  
> +# clear BCJ filter if unsupported
> +xz -H | grep -q -- $BCJ || BCJ=
> +
>  exec xz --check=crc32 $BCJ --lzma2=$LZMA2OPTS,dict=32MiB

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Ahmad Fatoum]

On 06.09.23 17:08, Jules Maselbas wrote:
> Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
> which include spelling fixes and the new ARM64 BCJ decoder which was recently
> introduced into .xz file format version 1.1.0 (2022-12-11) [2].
> 
> [1] https://git.tukaani.org/?p=xz-embedded.git
> [2] https://tukaani.org/xz/xz-file-format-1.1.0.txt
> 
> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> ---
>  lib/Kconfig           |  4 ++++
>  lib/decompress_unxz.c |  3 +++
>  lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
>  lib/xz/xz_private.h   |  3 +++
>  4 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 758197b608..9291e5a8ff 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -42,6 +42,7 @@ config XZ_DECOMPRESS
>  	select XZ_DEC_ARM
>  	select XZ_DEC_ARMTHUMB
>  	select XZ_DEC_SPARC
> +	select XZ_DEC_ARM64

Hmm, maybe for PBL, we should only support the XZ BCJ filter that is chosen
for barebox proper? If you are trying to get barebox binary size down,
maybe you are inclined to check? :-)

Thanks,
Ahmad 

>  
>  config XZ_DEC_X86
>          bool
> @@ -61,6 +62,9 @@ config XZ_DEC_ARMTHUMB
>  config XZ_DEC_SPARC
>          bool
>  
> +config XZ_DEC_ARM64
> +	bool "ARM64 BCJ filter decoder"
> +
>  config REED_SOLOMON
>  	bool
>  
> diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
> index a7e2d331ab..5c906932f8 100644
> --- a/lib/decompress_unxz.c
> +++ b/lib/decompress_unxz.c
> @@ -132,6 +132,9 @@
>  #endif
>  #ifdef CONFIG_ARM
>  #	define XZ_DEC_ARM
> +#	ifdef CONFIG_CPU_64
> +#		define XZ_DEC_ARM64
> +#	endif
>  #endif
>  #ifdef CONFIG_IA64
>  #	define XZ_DEC_IA64
> diff --git a/lib/xz/xz_dec_bcj.c b/lib/xz/xz_dec_bcj.c
> index d268adbc65..d40fae3416 100644
> --- a/lib/xz/xz_dec_bcj.c
> +++ b/lib/xz/xz_dec_bcj.c
> @@ -2,7 +2,7 @@
>   * Branch/Call/Jump (BCJ) filter decoders
>   *
>   * Authors: Lasse Collin <lasse.collin@tukaani.org>
> - *          Igor Pavlov <http://7-zip.org/>
> + *          Igor Pavlov <https://7-zip.org/>
>   *
>   * This file has been put into the public domain.
>   * You can do whatever you want with this file.
> @@ -24,7 +24,8 @@ struct xz_dec_bcj {
>  		BCJ_IA64 = 6,       /* Big or little endian */
>  		BCJ_ARM = 7,        /* Little endian only */
>  		BCJ_ARMTHUMB = 8,   /* Little endian only */
> -		BCJ_SPARC = 9       /* Big or little endian */
> +		BCJ_SPARC = 9,      /* Big or little endian */
> +		BCJ_ARM64 = 10      /* AArch64 */
>  	} type;
>  
>  	/*
> @@ -334,6 +335,45 @@ static size_t bcj_sparc(struct xz_dec_bcj *s, uint8_t *buf, size_t size)
>  }
>  #endif
>  
> +#ifdef XZ_DEC_ARM64
> +static size_t bcj_arm64(struct xz_dec_bcj *s, uint8_t *buf, size_t size)
> +{
> +	size_t i;
> +	uint32_t instr;
> +	uint32_t addr;
> +
> +	for (i = 0; i + 4 <= size; i += 4) {
> +		instr = get_unaligned_le32(buf + i);
> +
> +		if ((instr >> 26) == 0x25) {
> +			/* BL instruction */
> +			addr = instr - ((s->pos + (uint32_t)i) >> 2);
> +			instr = 0x94000000 | (addr & 0x03FFFFFF);
> +			put_unaligned_le32(instr, buf + i);
> +
> +		} else if ((instr & 0x9F000000) == 0x90000000) {
> +			/* ADRP instruction */
> +			addr = ((instr >> 29) & 3) | ((instr >> 3) & 0x1FFFFC);
> +
> +			/* Only convert values in the range +/-512 MiB. */
> +			if ((addr + 0x020000) & 0x1C0000)
> +				continue;
> +
> +			addr -= (s->pos + (uint32_t)i) >> 12;
> +
> +			instr &= 0x9000001F;
> +			instr |= (addr & 3) << 29;
> +			instr |= (addr & 0x03FFFC) << 3;
> +			instr |= (0U - (addr & 0x020000)) & 0xE00000;
> +
> +			put_unaligned_le32(instr, buf + i);
> +		}
> +	}
> +
> +	return i;
> +}
> +#endif
> +
>  /*
>   * Apply the selected BCJ filter. Update *pos and s->pos to match the amount
>   * of data that got filtered.
> @@ -380,6 +420,11 @@ static void bcj_apply(struct xz_dec_bcj *s,
>  	case BCJ_SPARC:
>  		filtered = bcj_sparc(s, buf, size);
>  		break;
> +#endif
> +#ifdef XZ_DEC_ARM64
> +	case BCJ_ARM64:
> +		filtered = bcj_arm64(s, buf, size);
> +		break;
>  #endif
>  	default:
>  		/* Never reached but silence compiler warnings. */
> @@ -422,7 +467,7 @@ XZ_EXTERN enum xz_ret xz_dec_bcj_run(struct xz_dec_bcj *s,
>  
>  	/*
>  	 * Flush pending already filtered data to the output buffer. Return
> -	 * immediatelly if we couldn't flush everything, or if the next
> +	 * immediately if we couldn't flush everything, or if the next
>  	 * filter in the chain had already returned XZ_STREAM_END.
>  	 */
>  	if (s->temp.filtered > 0) {
> @@ -553,6 +598,9 @@ XZ_EXTERN enum xz_ret xz_dec_bcj_reset(struct xz_dec_bcj *s, uint8_t id)
>  #endif
>  #ifdef XZ_DEC_SPARC
>  	case BCJ_SPARC:
> +#endif
> +#ifdef XZ_DEC_ARM64
> +	case BCJ_ARM64:
>  #endif
>  		break;
>  
> diff --git a/lib/xz/xz_private.h b/lib/xz/xz_private.h
> index 85f79635f0..9d6bd7c905 100644
> --- a/lib/xz/xz_private.h
> +++ b/lib/xz/xz_private.h
> @@ -37,6 +37,9 @@
>  #		ifdef CONFIG_XZ_DEC_SPARC
>  #			define XZ_DEC_SPARC
>  #		endif
> +#		ifdef CONFIG_XZ_DEC_ARM64
> +#			define XZ_DEC_ARM64
> +#		endif
>  #		define memeq(a, b, size) (memcmp(a, b, size) == 0)
>  #		define memzero(buf, size) memset(buf, 0, size)
>  #		define FREE free

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/3] scripts: Select XZ --arm64 BCJ filter for 64-bit arm

[Jules Maselbas]

On Thu, Sep 07, 2023 at 10:56:21AM +0200, Ahmad Fatoum wrote:
> On 06.09.23 17:08, Jules Maselbas wrote:
> > This change will select the arm64 BCJ filter for aarch64 instead of
> > the arm filter which was always selected for arm based architectures.
> > 
> > The selected filter is also tested, by looking if it is present in
> > the long help message of xz, this is because the arm64 BCJ filter is
> > a recent addition to xz.
> > 
> > Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> 
> How much of an improvement is BCJ=--arm over BCJ=""?
In my case using the --arm BCJ on a aarch64 barebox is slightly worst
than no BCJ at all.

$ xz -vkf --check=crc32 --lzma2=dict=32MiB barebox.bin
  100 %        84.7 KiB / 236.9 KiB = 0.357
$ xz -vkf --check=crc32 --arm --lzma2=dict=32MiB barebox.bin
  100 %        85.5 KiB / 236.9 KiB = 0.361
$ xz -vkf --check=crc32 --arm64 --lzma2=dict=32MiB barebox.bin
  100 %        81.0 KiB / 236.9 KiB = 0.342


> I wonder if we should fallback from --arm64 to --arm.
>  
> Cheers,
> Ahmad
> 
> > ---
> >  scripts/xz_wrap.sh | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/xz_wrap.sh b/scripts/xz_wrap.sh
> > index 7a2d372f48..5b5f3adcff 100755
> > --- a/scripts/xz_wrap.sh
> > +++ b/scripts/xz_wrap.sh
> > @@ -16,8 +16,11 @@ case $SRCARCH in
> >  	x86)            BCJ=--x86 ;;
> >  	powerpc)        BCJ=--powerpc ;;
> >  	ia64)           BCJ=--ia64; LZMA2OPTS=pb=4 ;;
> > -	arm)            BCJ=--arm ;;
> > +	arm)            BCJ=--arm$S64 ;;
> >  	sparc)          BCJ=--sparc ;;
> >  esac
> >  
> > +# clear BCJ filter if unsupported
> > +xz -H | grep -q -- $BCJ || BCJ=
> > +
> >  exec xz --check=crc32 $BCJ --lzma2=$LZMA2OPTS,dict=32MiB
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

Re: [PATCH 2/3] scripts: Select XZ --arm64 BCJ filter for 64-bit arm

[Ahmad Fatoum]

On 07.09.23 11:48, Jules Maselbas wrote:
> On Thu, Sep 07, 2023 at 10:56:21AM +0200, Ahmad Fatoum wrote:
>> On 06.09.23 17:08, Jules Maselbas wrote:
>>> This change will select the arm64 BCJ filter for aarch64 instead of
>>> the arm filter which was always selected for arm based architectures.
>>>
>>> The selected filter is also tested, by looking if it is present in
>>> the long help message of xz, this is because the arm64 BCJ filter is
>>> a recent addition to xz.
>>>
>>> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
>>
>> How much of an improvement is BCJ=--arm over BCJ=""?
> In my case using the --arm BCJ on a aarch64 barebox is slightly worst
> than no BCJ at all.

OK, that answers that question... :D

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> 
> $ xz -vkf --check=crc32 --lzma2=dict=32MiB barebox.bin
>   100 %        84.7 KiB / 236.9 KiB = 0.357
> $ xz -vkf --check=crc32 --arm --lzma2=dict=32MiB barebox.bin
>   100 %        85.5 KiB / 236.9 KiB = 0.361
> $ xz -vkf --check=crc32 --arm64 --lzma2=dict=32MiB barebox.bin
>   100 %        81.0 KiB / 236.9 KiB = 0.342
> 
> 
>> I wonder if we should fallback from --arm64 to --arm.
>>  
>> Cheers,
>> Ahmad
>>
>>> ---
>>>  scripts/xz_wrap.sh | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/xz_wrap.sh b/scripts/xz_wrap.sh
>>> index 7a2d372f48..5b5f3adcff 100755
>>> --- a/scripts/xz_wrap.sh
>>> +++ b/scripts/xz_wrap.sh
>>> @@ -16,8 +16,11 @@ case $SRCARCH in
>>>  	x86)            BCJ=--x86 ;;
>>>  	powerpc)        BCJ=--powerpc ;;
>>>  	ia64)           BCJ=--ia64; LZMA2OPTS=pb=4 ;;
>>> -	arm)            BCJ=--arm ;;
>>> +	arm)            BCJ=--arm$S64 ;;
>>>  	sparc)          BCJ=--sparc ;;
>>>  esac
>>>  
>>> +# clear BCJ filter if unsupported
>>> +xz -H | grep -q -- $BCJ || BCJ=
>>> +
>>>  exec xz --check=crc32 $BCJ --lzma2=$LZMA2OPTS,dict=32MiB
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Jules Maselbas]

On Thu, Sep 07, 2023 at 11:00:05AM +0200, Ahmad Fatoum wrote:
> On 06.09.23 17:08, Jules Maselbas wrote:
> > Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
> > which include spelling fixes and the new ARM64 BCJ decoder which was recently
> > introduced into .xz file format version 1.1.0 (2022-12-11) [2].
> > 
> > [1] https://git.tukaani.org/?p=xz-embedded.git
> > [2] https://tukaani.org/xz/xz-file-format-1.1.0.txt
> > 
> > Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> > ---
> >  lib/Kconfig           |  4 ++++
> >  lib/decompress_unxz.c |  3 +++
> >  lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
> >  lib/xz/xz_private.h   |  3 +++
> >  4 files changed, 61 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 758197b608..9291e5a8ff 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -42,6 +42,7 @@ config XZ_DECOMPRESS
> >  	select XZ_DEC_ARM
> >  	select XZ_DEC_ARMTHUMB
> >  	select XZ_DEC_SPARC
> > +	select XZ_DEC_ARM64
> 
> Hmm, maybe for PBL, we should only support the XZ BCJ filter that is chosen
> for barebox proper? If you are trying to get barebox binary size down,
> maybe you are inclined to check? :-)

indeed, i thought it was already the case. How would do that ? have two version
of the unxz binary object ? one for pbl and the other for barebox proper ?
Or maybe only select the relevent decoder ?

snip

> > diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
> > index a7e2d331ab..5c906932f8 100644
> > --- a/lib/decompress_unxz.c
> > +++ b/lib/decompress_unxz.c
> > @@ -132,6 +132,9 @@
> >  #endif
> >  #ifdef CONFIG_ARM
> >  #	define XZ_DEC_ARM
> > +#	ifdef CONFIG_CPU_64
> > +#		define XZ_DEC_ARM64
> > +#	endif
> >  #endif
I though this was used to _only_ have the decoder for the current architecture.

Re: [PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Ahmad Fatoum]

Hello Jules,

On 07.09.23 11:57, Jules Maselbas wrote:
> On Thu, Sep 07, 2023 at 11:00:05AM +0200, Ahmad Fatoum wrote:
>> On 06.09.23 17:08, Jules Maselbas wrote:
>>> Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
>>> which include spelling fixes and the new ARM64 BCJ decoder which was recently
>>> introduced into .xz file format version 1.1.0 (2022-12-11) [2].
>>>
>>> [1] https://git.tukaani.org/?p=xz-embedded.git
>>> [2] https://tukaani.org/xz/xz-file-format-1.1.0.txt
>>>
>>> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
>>> ---
>>>  lib/Kconfig           |  4 ++++
>>>  lib/decompress_unxz.c |  3 +++
>>>  lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
>>>  lib/xz/xz_private.h   |  3 +++
>>>  4 files changed, 61 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>> index 758197b608..9291e5a8ff 100644
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -42,6 +42,7 @@ config XZ_DECOMPRESS
>>>  	select XZ_DEC_ARM
>>>  	select XZ_DEC_ARMTHUMB
>>>  	select XZ_DEC_SPARC
>>> +	select XZ_DEC_ARM64
>>
>> Hmm, maybe for PBL, we should only support the XZ BCJ filter that is chosen
>> for barebox proper? If you are trying to get barebox binary size down,
>> maybe you are inclined to check? :-)
> 
> indeed, i thought it was already the case. How would do that ? have two version
> of the unxz binary object ? one for pbl and the other for barebox proper ?
> Or maybe only select the relevent decoder ?

You already have two objects (.o and .o.pbl). What you'd do is ignore the Kconfig
option in the PBL case. Looking into the code, that's indeed happening
CONFIG_XZ_DEC_ARM is only consulted when !defined(XZ_PREBOOT) and that symbol
is only defined for PBL. This has one small implication for your code though:

> 
> snip
> 
>>> diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
>>> index a7e2d331ab..5c906932f8 100644
>>> --- a/lib/decompress_unxz.c
>>> +++ b/lib/decompress_unxz.c
>>> @@ -132,6 +132,9 @@
>>>  #endif
>>>  #ifdef CONFIG_ARM
>>>  #	define XZ_DEC_ARM
>>> +#	ifdef CONFIG_CPU_64
>>> +#		define XZ_DEC_ARM64

You define XZ_DEC_ARM in the aarch64 case, which wastes space unnecessarily.

Cheers,
Ahmad

>>> +#	endif
>>>  #endif
> I though this was used to _only_ have the decoder for the current architecture.
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Jules Maselbas]

Hi Ahmad,

On Thu, Sep 07, 2023 at 12:08:34PM +0200, Ahmad Fatoum wrote:
> Hello Jules,
> 
> On 07.09.23 11:57, Jules Maselbas wrote:
> > On Thu, Sep 07, 2023 at 11:00:05AM +0200, Ahmad Fatoum wrote:
> >> On 06.09.23 17:08, Jules Maselbas wrote:
> >>> Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
> >>> which include spelling fixes and the new ARM64 BCJ decoder which was recently
> >>> introduced into .xz file format version 1.1.0 (2022-12-11) [2].
> >>>
> >>> [1] https://git.tukaani.org/?p=xz-embedded.git
> >>> [2] https://tukaani.org/xz/xz-file-format-1.1.0.txt
> >>>
> >>> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> >>> ---
> >>>  lib/Kconfig           |  4 ++++
> >>>  lib/decompress_unxz.c |  3 +++
> >>>  lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
> >>>  lib/xz/xz_private.h   |  3 +++
> >>>  4 files changed, 61 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/lib/Kconfig b/lib/Kconfig
> >>> index 758197b608..9291e5a8ff 100644
> >>> --- a/lib/Kconfig
> >>> +++ b/lib/Kconfig
> >>> @@ -42,6 +42,7 @@ config XZ_DECOMPRESS
> >>>  	select XZ_DEC_ARM
> >>>  	select XZ_DEC_ARMTHUMB
> >>>  	select XZ_DEC_SPARC
> >>> +	select XZ_DEC_ARM64
> >>
> >> Hmm, maybe for PBL, we should only support the XZ BCJ filter that is chosen
> >> for barebox proper? If you are trying to get barebox binary size down,
> >> maybe you are inclined to check? :-)
> > 
> > indeed, i thought it was already the case. How would do that ? have two version
> > of the unxz binary object ? one for pbl and the other for barebox proper ?
> > Or maybe only select the relevent decoder ?
> 
> You already have two objects (.o and .o.pbl). What you'd do is ignore the Kconfig
> option in the PBL case. Looking into the code, that's indeed happening
> CONFIG_XZ_DEC_ARM is only consulted when !defined(XZ_PREBOOT) and that symbol
> is only defined for PBL. This has one small implication for your code though:

I didn't found the .o.pbl, but this seems to be compiled in pbl/decompress.o,
which directly includes decompress_unxz.c after defining STATIC (which will makes
XZ_PREBOOT being defined).

After digging a bit, it seems that pbl already only contains the relevent decoder.

> > 
> > snip
> > 
> >>> diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
> >>> index a7e2d331ab..5c906932f8 100644
> >>> --- a/lib/decompress_unxz.c
> >>> +++ b/lib/decompress_unxz.c
> >>> @@ -132,6 +132,9 @@
> >>>  #endif
> >>>  #ifdef CONFIG_ARM
> >>>  #	define XZ_DEC_ARM
> >>> +#	ifdef CONFIG_CPU_64
> >>> +#		define XZ_DEC_ARM64
> 
> You define XZ_DEC_ARM in the aarch64 case, which wastes space unnecessarily.
Yeah, i was just a bit worried about "backward" compatibility where a newer pbl
would be used to boot an older barebox proper... compressed with the arm BCJ.
On a second thought, this doesn't makes much sens, barebox proper and pbl are
tightly integrated, so this should not be an issue.

Thanks for pointing this out!

Cheers,

Re: [PATCH 1/3] decompressors: Update xz to include ARM64 BCJ decoder

[Ahmad Fatoum]

Hello Jules,

On 07.09.23 12:42, Jules Maselbas wrote:
> Hi Ahmad,
> 
> On Thu, Sep 07, 2023 at 12:08:34PM +0200, Ahmad Fatoum wrote:
>> Hello Jules,
>>
>> On 07.09.23 11:57, Jules Maselbas wrote:
>>> On Thu, Sep 07, 2023 at 11:00:05AM +0200, Ahmad Fatoum wrote:
>>>> On 06.09.23 17:08, Jules Maselbas wrote:
>>>>> Update lib/xz/xz_dec_bcj.c and lib/xz/xz_private.h files from xz-embedded [1],
>>>>> which include spelling fixes and the new ARM64 BCJ decoder which was recently
>>>>> introduced into .xz file format version 1.1.0 (2022-12-11) [2].
>>>>>
>>>>> [1] https://git.tukaani.org/?p=xz-embedded.git
>>>>> [2] https://tukaani.org/xz/xz-file-format-1.1.0.txt
>>>>>
>>>>> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
>>>>> ---
>>>>>  lib/Kconfig           |  4 ++++
>>>>>  lib/decompress_unxz.c |  3 +++
>>>>>  lib/xz/xz_dec_bcj.c   | 54 ++++++++++++++++++++++++++++++++++++++++---
>>>>>  lib/xz/xz_private.h   |  3 +++
>>>>>  4 files changed, 61 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>>>> index 758197b608..9291e5a8ff 100644
>>>>> --- a/lib/Kconfig
>>>>> +++ b/lib/Kconfig
>>>>> @@ -42,6 +42,7 @@ config XZ_DECOMPRESS
>>>>>  	select XZ_DEC_ARM
>>>>>  	select XZ_DEC_ARMTHUMB
>>>>>  	select XZ_DEC_SPARC
>>>>> +	select XZ_DEC_ARM64
>>>>
>>>> Hmm, maybe for PBL, we should only support the XZ BCJ filter that is chosen
>>>> for barebox proper? If you are trying to get barebox binary size down,
>>>> maybe you are inclined to check? :-)
>>>
>>> indeed, i thought it was already the case. How would do that ? have two version
>>> of the unxz binary object ? one for pbl and the other for barebox proper ?
>>> Or maybe only select the relevent decoder ?
>>
>> You already have two objects (.o and .o.pbl). What you'd do is ignore the Kconfig
>> option in the PBL case. Looking into the code, that's indeed happening
>> CONFIG_XZ_DEC_ARM is only consulted when !defined(XZ_PREBOOT) and that symbol
>> is only defined for PBL. This has one small implication for your code though:
> 
> I didn't found the .o.pbl, but this seems to be compiled in pbl/decompress.o,
> which directly includes decompress_unxz.c after defining STATIC (which will makes
> XZ_PREBOOT being defined).

Ah, right. The decompressors are a bit "special" in that regard.

> 
> After digging a bit, it seems that pbl already only contains the relevent decoder.
> 
>>>
>>> snip
>>>
>>>>> diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
>>>>> index a7e2d331ab..5c906932f8 100644
>>>>> --- a/lib/decompress_unxz.c
>>>>> +++ b/lib/decompress_unxz.c
>>>>> @@ -132,6 +132,9 @@
>>>>>  #endif
>>>>>  #ifdef CONFIG_ARM
>>>>>  #	define XZ_DEC_ARM
>>>>> +#	ifdef CONFIG_CPU_64
>>>>> +#		define XZ_DEC_ARM64
>>
>> You define XZ_DEC_ARM in the aarch64 case, which wastes space unnecessarily.
> Yeah, i was just a bit worried about "backward" compatibility where a newer pbl
> would be used to boot an older barebox proper... compressed with the arm BCJ.
> On a second thought, this doesn't makes much sens, barebox proper and pbl are
> tightly integrated, so this should not be an issue.

Ye, booting mismatched barebox PBLs and proper binaries isn't something
that we guarantee to work. For this reason, e.g. i.MX8M enters PBL twice:
Once while running in SRAM and once after relocating whole barebox binary
(including PBL) to DRAM.

Cheers,
Ahmad

> 
> Thanks for pointing this out!
> 
> Cheers,
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] fixup! decompressors: Update xz to include ARM64 BCJ decoder

[Jules Maselbas]

This saves ~1.2KB from pbl binary size

Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
---
 lib/decompress_unxz.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
index 5c906932f8..132ab4a239 100644
--- a/lib/decompress_unxz.c
+++ b/lib/decompress_unxz.c
@@ -131,9 +131,10 @@
 #	define XZ_DEC_POWERPC
 #endif
 #ifdef CONFIG_ARM
-#	define XZ_DEC_ARM
 #	ifdef CONFIG_CPU_64
 #		define XZ_DEC_ARM64
+#	else
+#		define XZ_DEC_ARM
 #	endif
 #endif
 #ifdef CONFIG_IA64
-- 
2.42.0

Re: [PATCH] fixup! decompressors: Update xz to include ARM64 BCJ decoder

[Jules Maselbas]

On Thu, Sep 07, 2023 at 12:51:43PM +0200, Jules Maselbas wrote:
> This saves ~1.2KB from pbl binary size
yeah but that's because there are no BCJ filter at all...
this patch is broken, I have a fix and will send a v2.

> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> ---
>  lib/decompress_unxz.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c
> index 5c906932f8..132ab4a239 100644
> --- a/lib/decompress_unxz.c
> +++ b/lib/decompress_unxz.c
> @@ -131,9 +131,10 @@
>  #	define XZ_DEC_POWERPC
>  #endif
>  #ifdef CONFIG_ARM
> -#	define XZ_DEC_ARM
>  #	ifdef CONFIG_CPU_64
>  #		define XZ_DEC_ARM64
> +#	else
> +#		define XZ_DEC_ARM
>  #	endif
>  #endif
>  #ifdef CONFIG_IA64
> -- 
> 2.42.0
> 
>