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@xxxxxxxx> > >>> --- > >>> 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,
