Hi Alexey,
On 2026-09-18T10:52:27, Alexey Charkov <[email protected]> wrote:
> test: spl: Cover FIT image decompression
>
> Nothing exercises the decompression in load_simple_fit(): create_fit()
> only ever writes compression = "none", and the canned LZMA blob is only
> used on the legacy uImage path.
>
> Let create_fit() take a compression, and load a compressed FIT both ways:
> with the data inside the FIT and beside it, since only the latter stages
> it through CONFIG_SYS_LOAD_ADDR. Point that address at the same offset
> sandbox_noinst uses, since otherwise the staging corrupts what's currently
> at 0x0.
Please can you say what gets corrupted? I believe it is the bloblist,
since CONFIG_BLOBLIST_ADDR is 0x100 and the LZMA blob is about 2.8K.
With the address left at 0x0, running spl_test_nor_FIT_EXTERNAL_LZMA
on its own passes, but then I see:
SPL: SPL hand-off write failed (err=-2)
Since this also changes loadaddr for U-Boot proper on sandbox_spl,
please can you put the defconfig change in its own patch, before this
one?
BTW sandbox_vpl still has CONFIG_SYS_LOAD_ADDR=0x0 and enables
SPL_UNIT_TEST. It skips these tests only because it lacks SPL_LZMA, so
it is probably worth changing that one too.
>
> Signed-off-by: Alexey Charkov <[email protected]>
>
> configs/sandbox_spl_defconfig | 2 +-
> include/test/spl.h | 8 +++++++
> test/image/spl_load.c | 52
> ++++++++++++++++++++++++++++++-------------
> test/image/spl_load_nor.c | 2 ++
> 4 files changed, 48 insertions(+), 16 deletions(-)
> diff --git a/test/image/spl_load.c b/test/image/spl_load.c
> @@ -40,7 +40,25 @@ void *board_spl_fit_buffer_addr(ulong fit_size, int
> sectors, int bl_len)
> +static const char *image_compression(enum spl_test_image type)
> +{
> + switch (type) {
> + case LEGACY_LZMA:
> + case FIT_INTERNAL_LZMA:
> + case FIT_EXTERNAL_LZMA:
> + return "lzma";
LZMA in a FIT already worked before patch 5, so as far as I can tell
these tests would pass without it. See my comment on patch 9 about
adding zstd cases.
This helper is only half general. It returns a name, but
create_legacy() still turns SPL_COMPRESSED into IH_COMP_LZMA and
do_spl_test_load() always copies in lzma_compressed with the 'lzma'
seed. I suspect it would be cleaner to return an IH_COMP_... value,
pass that to image_set_comp() for legacy and use
genimg_get_comp_short_name() in create_fit(). Then IH_COMP_NONE gives
you 'none' without the special case in create_image(). What do you
think?
> diff --git a/test/image/spl_load_nor.c b/test/image/spl_load_nor.c
> @@ -35,6 +35,8 @@ SPL_IMG_TEST(spl_test_nor, LEGACY, 0);
> +SPL_IMG_TEST(spl_test_nor, FIT_INTERNAL_LZMA, 0);
> #if !IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)
> SPL_IMG_TEST(spl_test_nor, FIT_EXTERNAL, 0);
> +SPL_IMG_TEST(spl_test_nor, FIT_EXTERNAL_LZMA, 0);
Why only NOR? It reads with a bl_len of 1, so the external case never
has a non-zero get_aligned_image_overhead() when staging through
CONFIG_SYS_LOAD_ADDR. The SPI, NAND and net tests already go through
do_spl_test_load() so they should just need the extra lines.
The MMC test would cover the block-aligned read, but spl_load_fs.c has
its own copy of the type == LEGACY_LZMA checks. Please can you export
image_compression() in test/spl.h and use it there too?
Regards,
Simon
