Hi Tom,

On Wed, Aug 19, 2026 at 6:10 AM Tom Rini <[email protected]> wrote:
>
> On Tue, Aug 18, 2026 at 01:23:14PM +0000, Aristo Chen wrote:
>
> > This is v2 of "bootm: size the noload decompression buffer from the
> > compressor header". Tom pushed back on v1
> > (https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/)
> > on two grounds:
> >
> >   1. No concrete problem report driving the change.
> >   2. ~1297 platforms grew by ~170-400 bytes; the change is not
> >      opt-in, so the size cost falls on everyone.
>
> So, the first example that pops up in my builds is imx8mn_beacon_2g.
> And for v1 of the series:
>    aarch64: (for 1/1 boards) all +16384.0 data +14336.0 text +2048.0
>             imx8mn_beacon_2g: all +16384 data +14336 text +2048
>                u-boot: add: 0/0, grow: 1/0 bytes: 328/0 (328)
>                  function                                   old     new   
> delta
>                  bootm_run_states                          3316    3644    
> +328
>
> And now for v2:
>    aarch64: (for 1/1 boards) all +16384.0 data +14336.0 text +2048.0
>             imx8mn_beacon_2g: all +16384 data +14336 text +2048
>                u-boot: add: 0/0, grow: 1/0 bytes: 368/0 (368)
>                  function                                   old     new   
> delta
>                  bootm_run_states                          3316    3684    
> +368
>
> Next, picking turris_mox as it enables ZSTD:
> v1:
>    aarch64: (for 1/1 boards) all +472.0 text +472.0
>             turris_mox     : all +472 text +472
>                u-boot: add: 2/0, grow: 1/0 bytes: 472/0 (472)
>                  function                                   old     new   
> delta
>                  image_decomp_get_uncompressed_size           -     416    
> +416
>                  bootm_run_states                          2360    2412     
> +52
>                  zstd_get_frame_header                        -       4      
> +4
> v2:
>    aarch64: (for 1/1 boards) all +428.0 text +428.0
>             turris_mox     : all +428 text +428
>                u-boot: add: 1/0, grow: 1/0 bytes: 428/0 (428)
>                  function                                   old     new   
> delta
>                  bootm_run_states                          2360    2784    
> +424
>                  zstd_get_frame_header                        -       4      
> +4
>
> So, that is better. Looking at smartweb, both iterations are the same:
>        arm: (for 1/1 boards) all +96.0 text +96.0
>             smartweb       : all +96 text +96
>                u-boot: add: 0/0, grow: 1/0 bytes: 76/0 (76)
>                  function                                   old     new   
> delta
>                  bootm_run_states                          3592    3668     
> +76
>
> What's honestly concerning is chromebook_coral where v2 *shrinks*:
>                u-boot: add: 0/0, grow: 0/-1 bytes: 0/-2 (-2)
>                  function                                   old     new   
> delta
>                  bootm_load_os                              520     518      
> -2
> but v1 grows:
>                u-boot: add: 1/0, grow: 1/0 bytes: 110/0 (110)
>                  function                                   old     new   
> delta
>                  bootm_load_os                              520     587     
> +67
>                  image_decomp_get_uncompressed_size           -      43     
> +43
>

That one is working as intended: chromebook_coral enables none of
GZIP/LZMA/LZ4/ZSTD, so it is one of the "bootm with no decompressor
at all" boards from the cover letter. In v2 every helper and its
switch case sit behind CONFIG_IS_ENABLED(<format>), so on that board
they all compile away and only the unchanged 8x fallback remains.
The -2 bytes is codegen noise from the restructure; I diffed the
disassembly and the function is otherwise unchanged. v1 grew there
because its helper in image.c was built unconditionally. Behaviour is
unchanged either way: with no decompressor enabled, a compressed
kernel_noload image already fails in image_decomp().

The imx8mn_beacon_2g result has a similar shape to what I measured
on am62x_evm_a53: both enable LTO, and on these LTO configurations
compiler inlining and layout make v2 a few tens of bytes larger
than v1 (+368 vs +328 here, with the growth landing inside
bootm_run_states either way). That is the trade of the per-format
split, which is what makes the no-decompressor boards free, trims
the gzip-only majority, and keeps each format individually
droppable.

turris_mox is the non-LTO counterpart and shows the intended
direction for the multi-algorithm case: v2 comes in 44 bytes below
v1 there (+428 vs +472 in your run).

smartweb is the expected gzip-only LTO case: both versions cost
essentially the same (+96 in your run).

Thanks for the u-boot-size-test.sh pointer. I re-ran your four
boards with it against this series' base and reproduce your numbers
to within a few bytes of toolchain difference, including coral's -2
(here: v1 +108 with bootm_load_os +65 plus the unconditional helper
+43, v2 -2). I will use the script for the size numbers from now on;
if you would like the cover letter regenerated with those numbers, I
am happy to respin as v3 with the code unchanged.

> > On the first point, Nora Schiffer replied with a concrete use case
> > (EFI-in-FIT plus padded loaders such as shim, systemd-boot, and
> > OpenWrt's lzma-loader can produce compression ratios that outrun the
> > 8x heuristic), and mentioned this is on the road map for TQ-Systems
> > standard BSPs.
> >
> > On the second point, v2 reworks the implementation to cut the size
> > cost, measures it across the format and architecture buckets, and
> > splits the work per format so each decompressor's support can be
> > taken or dropped on its own.
> >
> > Background: for a compressed kernel_noload image, bootm_load_os()
> > sizes the decompression buffer as ALIGN(image_len * 8, SZ_1M). The
> > 8x heuristic works for typical kernels, but any well-compressed
> > payload can exceed it, and no fixed multiplier is safe against
> > arbitrarily compressible input.
> >
> > Each implementation patch adds a small static header-parse helper in
> > bootm.c (no new public API) and wires it into a size-hint switch;
> > helper and switch case are only compiled when the matching
> > decompressor is enabled, so boards that do not build a format pay no
> > code for it. gzip's ISIZE is a fixed trailer read, lzma's size a
> > fixed header read, lz4 mirrors ulz4fn()'s frame-header validation,
> > and zstd asks zstd_get_frame_header(), whose frame-parsing code
> > already ships with the zstd decompressor. The header-recorded value
> > is attacker-controlled, so it is capped at CONFIG_SYS_BOOTM_LEN, and
> > it is only an allocation hint: the decoder stays authoritative
> > during the actual decompression.
> >
> > Text size deltas of the u-boot ELF (size(1), distro gcc 13.3 cross
> > toolchains); data/bss are unchanged everywhere. To make the columns
> > directly comparable, the v1 column is v1's implementation commit
> > cherry-picked onto this series' base, so both columns share one
> > baseline:
>
> Please use binman to look at the size changes, as it gives much more
> useful information. I've noted
> https://git.u-boot-project.org/u-boot/u-boot-extras/-/blob/master/contrib/trini/u-boot-size-test.sh?ref_type=heads
> for others before as a wrapper around the options to get the most useful
> information out.
>
> --
> Tom

Regards,
Aristo

Reply via email to