Re: [PATCH v2 0/8] bootm: size the noload buffer from the compressor header
Aristo Chen <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <CAA=k7==gD0A2Ri=-=4ogTf=yJH5s6mOUMF7k5LGYGRR-Sommfg@mail.gmail.com> |
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