Re: [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig

[SCHNEIDER Johannes <johannes.schneider@leica-geosystems.com>]

Hoi Ahmad,


>
> On 6/13/26 9:56 PM, Johannes Schneider wrote:
> > The PBL verifies fw-external blobs by hashing them in DRAM after the
> > BootROM-supplied load, before handing off to TF-A. SHA-256 is the
> > only choice today, and it runs with the MMU off and DRAM reads
> > uncached -- no D-cache, no hardware crypto, plain software hash
> > walking DRAM through the slowest available access path. The cost is
> > non-trivial.
> >
> > Measured on Cortex-A53, MMU off, uncached DRAM:
> >
> >   SHA-256, ~720 KiB blob:  ~2 s
> >   CRC32,   ~720 KiB blob:  ~10 ms
> >
> > That ~2 s sits in PBL phase 1, before BL31 entry, and is paid every
> > boot. When the SoC's secure boot (HABv4 on i.MX8M, AHAB on i.MX9, or
> > an upstream-signed FIT / bundle) already authenticates the blob
> > before the PBL ever sees it, the in-PBL SHA-256 is redundant:
>
> This is incorrect. The BootROM can only load as many bytes as it can fit
> into on-chip SRAM. The loaded PBL will set up DRAM and then loads the
> full image into DRAM.
>
> Replacing SHA256 here with CRC32 thus breaks verified boot from EL2 onward.
>

ouch, thanks for pushing back and in the right direction here

>
> > it
> > only re-checks the BootROM->DRAM copy against a value baked into the
> > PBL itself, not against an external trust anchor. A 32-bit CRC over
> > the same window catches the realistic failure mode (a corrupted copy
> > into DRAM) at ~200x lower wall-clock cost.
>
> Give recent commit f2ae1a4a85d0 ("ARM: rockchip: atf: enable MMU in
> PBL") and its follow-up a look.
>

thanks for the hint, i'm "toying" with the code now, and am wondering: instead
of enabling the MMU to speed up the software sha256, would you have any concerns
if i would enable and use the hardware sha256 that lives inside the imx8?

from my first couple of measurements i get:
| Variant                  |   bl32-verify | Delta vs prev | Cumulative drop |
| Pre-MMU-on (generic C, uncached)    |    2190 ms |        -- |        -- |
| Post-MMU-on (generic C, cached)     |     451 ms |   -1739 ms |   -1739 ms |
| Crypto-ext, per 64bytes block       |     192 ms |    -259 ms |   -1998 ms |
| Crypto-ext, batched                 |   **5 ms** |    -187 ms |   -2185 ms |

i could send either the "enable mmu" or the "enable hardware hashing" a better follow up to this patch - opinions/preferences?

> I'd suggest you do the same: Keep the hash as-is, but set up MMU as soon
> as you have the memory size.
>
> You can also play around with mimicking compression of firmware blobs
> like we have on Rockchip. Depending on CPU vs I/O speed, this can also
> reduce your startup time.

compressing the optee tee.bin (~720KiB) and having PBL uncompress it again would
(probably) work, i have some code that does this - but for some reason the way
we build and sign the whole barebox binary now doesn't agree with the imx8m*
bootrom, and it drops me back into serial-dl mode = it refuses the image
completly.

Any hints for what i need to look out for when adding such feature to the PBL?


gruß
Johannes


>
> Cheers,
> Ahmad
>
> >
> > Make the verify algorithm a Kconfig choice:
> >
> >   PBL_FIRMWARE_EXT_VERIFY_NONE    no verify
> >   PBL_FIRMWARE_EXT_VERIFY_CRC32   4-byte CRC, cheap corruption check
> >   PBL_FIRMWARE_EXT_VERIFY_SHA256  current behaviour, default
> >
> > firmware/Makefile selects an algorithm and passes it to gen-fw-s,
> > which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes
> > for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro
> > in include/firmware.h declares the verify symbols only when actually
> > used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op
> > and in-tree firmware never grew a section dependency.
> > firmware_ext_verify() dispatches via IS_ENABLED() at compile time.
> >
> > The verify-section symbol names (_fw_*_sha_*) are kept as-is rather
> > than renamed, since firmware_next_image_verify() also references
> > them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name
> > no longer describes the contents, but renaming would expand the
> > patch scope unnecessarily.
> >
> > Default stays at SHA-256 so existing defconfigs keep their current
> > semantics.
> >
> > Assisted-by: Claude:claude-opus-4-7
> > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> > ---
> >  firmware/Kconfig   | 33 +++++++++++++++++++++++++++
> >  firmware/Makefile  | 15 ++++++++++--
> >  include/firmware.h | 41 +++++++++++++++++++++++----------
> >  include/pbl.h      |  2 ++
> >  pbl/decomp.c       | 35 ++++++++++++++++++++++++++++
> >  scripts/gen-fw-s   | 57 +++++++++++++++++++++++++++++++++-------------
> >  6 files changed, 153 insertions(+), 30 deletions(-)
> >
> > diff --git a/firmware/Kconfig b/firmware/Kconfig
> > index b9b4556dbd..f745819a67 100644
> > --- a/firmware/Kconfig
> > +++ b/firmware/Kconfig
> > @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE
> >         verified images. The function to check the next stage image hash is
> >         firmware_next_image_verify(), make sure your SoC code uses it.
> >
> > +choice
> > +     prompt "PBL fw-external blob verification"
> > +     default PBL_FIRMWARE_EXT_VERIFY_SHA256
> > +     help
> > +       Algorithm used by the PBL to verify an fw-external blob against a
> > +       value embedded at build time, before handoff to TF-A. Runs with
> > +       the MMU off and DRAM reads uncached, so cost scales sharply with
> > +       blob size and algorithm. This checks integrity vs the build-time
> > +       value; it is not authentication against an external trust anchor.
> > +
> > +config PBL_FIRMWARE_EXT_VERIFY_NONE
> > +     bool "none"
> > +     help
> > +       Skip the verify. Appropriate when the boot chain authenticates the
> > +       blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the
> > +       in-PBL check is then redundant.
> > +
> > +config PBL_FIRMWARE_EXT_VERIFY_CRC32
> > +     bool "CRC32"
> > +     select CRC32
> > +     help
> > +       4-byte CRC32 (zlib polynomial). Catches accidental corruption
> > +       of the BootROM->DRAM copy; not cryptographic.
> > +
> > +config PBL_FIRMWARE_EXT_VERIFY_SHA256
> > +     bool "SHA-256"
> > +     help
> > +       Cryptographic hash, software-computed with MMU off. Strongest
> > +       option but by far the slowest -- prefer NONE or CRC32 when the
> > +       blob is already authenticated upstream of the PBL.
> > +
> > +endchoice
> > +
> >  endmenu
> > diff --git a/firmware/Makefile b/firmware/Makefile
> > index 7e433a1824..1590bed9f1 100644
> > --- a/firmware/Makefile
> > +++ b/firmware/Makefile
> > @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y))
> >
> >  FWNAME    = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
> >
> > -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir)
> > -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir)
> > +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the
> > +# verify metadata section emitted into .extgen.S and matched by
> > +# firmware_ext_verify() in include/firmware.h.
> > +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE
> > +fw_verify_algo := none
> > +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32
> > +fw_verify_algo := crc32
> > +else
> > +fw_verify_algo := sha256
> > +endif
> > +
> > +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256
> > +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo)
> >
> >  $(obj)/%.gen.S: FORCE
> >       $(call filechk,fwbin)
> > diff --git a/include/firmware.h b/include/firmware.h
> > index 6511d56b2e..72a47f24e3 100644
> > --- a/include/firmware.h
> > +++ b/include/firmware.h
> > @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw)
> >
> >  void firmwaremgr_list_handlers(void);
> >
> > -static inline void firmware_ext_verify(const void *data_start, size_t data_size,
> > -                                    const void *hash_start, size_t hash_size)
> > +static inline void firmware_ext_verify(const void *data, size_t data_size,
> > +                                    const void *verify, size_t verify_size)
> >  {
> > -     if (pbl_barebox_verify(data_start, data_size,
> > -                            hash_start, hash_size) != 0) {
> > +     int ret;
> > +
> > +     if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32))
> > +             ret = pbl_barebox_verify_crc32(data, data_size,
> > +                                            verify, verify_size);
> > +     else
> > +             ret = pbl_barebox_verify(data, data_size,
> > +                                      verify, verify_size);
> > +
> > +     if (ret != 0) {
> >               putc_ll('!');
> > -             panic("hash mismatch, refusing to decompress");
> > +             panic("firmware verify mismatch, refusing to decompress");
> >       }
> >  }
> >
> > @@ -96,22 +104,31 @@ struct fwobj {
> >       void *data;
> >  };
> >
> > +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE)
> > +# define __fw_ext_verify(name, fwobj)        do { } while (0)
> > +#else
> > +# define __fw_ext_verify(name, fwobj)                                        \
> > +     do {                                                            \
> > +             extern char _fw_##name##_sha_start[];           \
> > +             extern char _fw_##name##_sha_end[];                     \
> > +             firmware_ext_verify(                                    \
> > +                     (fwobj)->data, (fwobj)->size,                   \
> > +                     _fw_##name##_sha_start,                 \
> > +                     _fw_##name##_sha_end -                  \
> > +                     _fw_##name##_sha_start);                        \
> > +     } while (0)
> > +#endif
> > +
> >  #define __get_builtin_firmware(name, offset, fwobj)                  \
> >       do {                                                            \
> >               extern char _fw_##name##_start[];                       \
> >               extern char _fw_##name##_end[];                         \
> > -             extern char _fw_##name##_sha_start[];                   \
> > -             extern char _fw_##name##_sha_end[];                     \
> >               (fwobj)->data = _fw_##name##_start;                     \
> >               (fwobj)->size = _fw_##name##_end - _fw_##name##_start;  \
> >               if (!(offset))                                          \
> >                       break;                                          \
> >               (fwobj)->data += (offset);                              \
> > -             firmware_ext_verify(                                    \
> > -                     (fwobj)->data, (fwobj)->size,                   \
> > -                     _fw_##name##_sha_start,                         \
> > -                     _fw_##name##_sha_end - _fw_##name##_sha_start   \
> > -             );                                                      \
> > +             __fw_ext_verify(name, fwobj);                           \
> >       } while (0)
> >
> >
> > diff --git a/include/pbl.h b/include/pbl.h
> > index fe4367825c..57ca8b4eb3 100644
> > --- a/include/pbl.h
> > +++ b/include/pbl.h
> > @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath,
> >
> >  int pbl_barebox_verify(const void *compressed_start, unsigned int len,
> >                      const void *hash, unsigned int hash_len);
> > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len,
> > +                          const void *crc, unsigned int crc_len);
> >  int pbl_load_fdt(const void *fdt, void *dest, int destsize);
> >
> >  #define PBL_MALLOC_SIZE SZ_128K
> > diff --git a/pbl/decomp.c b/pbl/decomp.c
> > index 1539a6b67e..ace121eac6 100644
> > --- a/pbl/decomp.c
> > +++ b/pbl/decomp.c
> > @@ -13,6 +13,10 @@
> >  #include <asm/sections.h>
> >  #include <pbl.h>
> >  #include <debug_ll.h>
> > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32
> > +#include <crc.h>
> > +#include <asm/unaligned.h>
> > +#endif
> >
> >  #define STATIC static
> >
> > @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len,
> >       return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE);
> >  }
> >
> > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32
> > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len,
> > +                          const void *crc, unsigned int crc_len)
> > +{
> > +     uint32_t expected, computed;
> > +
> > +     if (crc_len != sizeof(uint32_t))
> > +             return -1;
> > +
> > +     /* Stored little-endian by .long in the .rodata.*.sha section. */
> > +     expected = get_unaligned_le32(crc);
> > +     computed = crc32(0, data_start, len);
> > +
> > +     if (IS_ENABLED(CONFIG_DEBUG_LL)) {
> > +             puts_ll("CRC ");
> > +             puthexc_ll((computed >> 24) & 0xff);
> > +             puthexc_ll((computed >> 16) & 0xff);
> > +             puthexc_ll((computed >> 8)  & 0xff);
> > +             puthexc_ll(computed & 0xff);
> > +             puts_ll(" vs ");
> > +             puthexc_ll((expected >> 24) & 0xff);
> > +             puthexc_ll((expected >> 16) & 0xff);
> > +             puthexc_ll((expected >> 8)  & 0xff);
> > +             puthexc_ll(expected & 0xff);
> > +             putc_ll('\n');
> > +     }
> > +
> > +     return computed == expected ? 0 : -1;
> > +}
> > +#endif
> > +
> >  void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len)
> >  {
> >       uint32_t pbl_hash_len;
> > diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s
> > index 78c3193479..d7d7d15824 100755
> > --- a/scripts/gen-fw-s
> > +++ b/scripts/gen-fw-s
> > @@ -3,13 +3,20 @@
> >  #
> >  # Generate assembly source to embed firmware binary
> >  #
> > -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir]
> > +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo]
> > +#
> > +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha
> > +# section, matched by firmware_ext_verify() in include/firmware.h:
> > +#   sha256 (default) -- 32-byte SHA-256
> > +#   crc32            -- 4-byte CRC32 (zlib polynomial)
> > +#   none             -- no section, no symbols
> >
> >  fwname=$1
> >  fwdir=$2
> >  secprefix=$3
> >  secflags=$4
> >  fwobjdir=$5
> > +verify_algo=${6:-sha256}
> >
> >  fwstr=$(echo "$fwname" | tr '/.-' '___')
> >  fwpath="$fwdir/$fwname"
> > @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then
> >       fw_uncompressed=$(stat -c %s "$fwpath")
> >  fi
> >
> > -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//')
> > -
> >  echo "/* Generated by scripts/gen-fw-s */"
> >  echo "#include <asm-generic/pointer.h>"
> >  echo ".section .note.GNU-stack,\"\",%progbits"
> > @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end"
> >  echo "_fw_z_${fwstr}_end:"
> >  echo "#endif"
> >
> > -# include sha256, needed for external firmware
> > -echo "    .section .rodata.${fwstr}.sha"
> > -echo "    .p2align ASM_LGPTR"
> > -echo ".global _fw_${fwstr}_sha_start"
> > -echo "_fw_${fwstr}_sha_start:"
> > -echo "    .byte ${sha}"
> > -echo ".global _fw_${fwstr}_sha_end"
> > -echo "_fw_${fwstr}_sha_end:"
> > -if [ -f "$fwpath" ]; then
> > -     echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end"
> > -     echo ".error \"sha256sum invalid\""
> > -     echo ".endif"
> > -fi
> > +# verify metadata for fw-external blobs, consumed by firmware_ext_verify()
> > +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig
> > +# choice (passed in from firmware/Makefile).
> > +case "$verify_algo" in
> > +none)
> > +     ;;
> > +crc32)
> > +     crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \
> > +             "$fwpath" 2>/dev/null || echo "0x00000000")
> > +     echo "    .section .rodata.${fwstr}.sha"
> > +     echo "    .p2align 2"
> > +     echo ".global _fw_${fwstr}_sha_start"
> > +     echo "_fw_${fwstr}_sha_start:"
> > +     echo "    .long ${crc}"
> > +     echo ".global _fw_${fwstr}_sha_end"
> > +     echo "_fw_${fwstr}_sha_end:"
> > +     ;;
> > +sha256|*)
> > +     sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//')
> > +     echo "    .section .rodata.${fwstr}.sha"
> > +     echo "    .p2align ASM_LGPTR"
> > +     echo ".global _fw_${fwstr}_sha_start"
> > +     echo "_fw_${fwstr}_sha_start:"
> > +     echo "    .byte ${sha}"
> > +     echo ".global _fw_${fwstr}_sha_end"
> > +     echo "_fw_${fwstr}_sha_end:"
> > +     if [ -f "$fwpath" ]; then
> > +             echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end"
> > +             echo ".error \"sha256sum invalid\""
> > +             echo ".endif"
> > +     fi
> > +     ;;
> > +esac
> >
> >  # include a string containing the firmware name. When a non existing
> >  # firmware is referenced in the PBL then _fwname_${fwstr} is referenced
> >
> > base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b
>
> --
> 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 |
>