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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |