Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 5723 bytes --]

On Mon, May 03, 2021 at 05:10:47PM -0600, Simon Glass wrote:

> Much of the image-handling code predates the introduction of Kconfig and
> has quite a few #ifdefs in it. It also uses its own IMAGE_... defines to
> help reduce the #ifdefs, which is unnecessary now that we can use
> IS_ENABLED() et al.
> 
> The image code is also where quite a bit of code is shared with the host
> tools. At present this uses a lot of checks of USE_HOSTCC.
> 
> This series introduces 'host' Kconfig options and a way to use
> CONFIG_IS_ENABLED() to check them. This works in a similar way to SPL, so
> 
>    CONFIG_IS_ENABLED(FIT)
> 
> will evaluate to true on the host build (USE_HOSTCC) if CONFIG_HOST_FIT is
> enabled. This allows quite a bit of clean-up of the image.h header file
> and many of the image C files.
> 
> The 'host' Kconfig options should help to solve a more general problem in
> that we mostly want the host tools to build with all features enabled, no
> matter which features the 'target' build actually uses. This is a pain to
> arrange at present, but with 'host' Kconfigs, we can just define them all
> to y.
> 
> There are cases where the host tools do not have features which are
> present on the target, for example environment and physical addressing.
> To help with this, some of the core image code is split out into
> image-board.c and image-host.c files.
> 
> Even with these changes, some #ifdefs remain (101 down to 42 in
> common/image*). But the code is somewhat easier to follow and there are
> fewer build paths.
> 
> In service of the above, this series includes a patch to add an API function
> for zstd, so the code can be dropped from bootm.c
> 
> It also introduces a function to handle manual relocation.

I like this idea overall.  The good news is this reduces the size in a
few places.  The bad news, but I can live with if we can't restructure
the changes more, is a few functions grow a bit.  This shows the good
and the bad (something like sama5d2_ptc_ek_mmc shows only growth, to be
clear):
            px30-core-edimm2.2-px30: all -36 rodata -24 text -12
               u-boot: add: 0/0, grow: 3/-4 bytes: 36/-48 (-12)
                 function                                   old     new   delta
                 boot_get_fdt                               896     924     +28
                 image_decomp                               372     376      +4
                 boot_get_ramdisk                           868     872      +4
                 do_bootm_vxworks                           552     540     -12
                 do_bootm_rtems                             124     112     -12
                 do_bootm_plan9                             228     216     -12
                 do_bootm_netbsd                            324     312     -12
            odroid-c2      : all -105 bss +128 rodata -65 text -168
               u-boot: add: 0/0, grow: 2/-3 bytes: 108/-172 (-64)
                 function                                   old     new   delta
                 images                                     504     608    +104
                 image_decomp                               372     376      +4
                 image_setup_linux                          108      96     -12
                 boot_get_ramdisk                           620     580     -40
                 boot_get_fdt                               660     540    -120
            origen         : all +47 bss +96 rodata -57 text +8
               u-boot: add: 0/0, grow: 15/-2 bytes: 180/-104 (76)
                 function                                   old     new   delta
                 images                                     288     340     +52
                 do_bootm_states                           1304    1348     +44
                 do_bootz                                   164     176     +12
                 do_bootm_vxworks                           332     344     +12
                 image_setup_libfdt                         168     176      +8
                 image_decomp                               156     164      +8
                 bootm_find_images                          212     220      +8
                 boot_prep_linux                            276     284      +8
                 image_setup_linux                           54      58      +4
                 do_bootm_standalone                         60      64      +4
                 do_bootm_plan9                             104     108      +4
                 do_bootm_netbsd                            168     172      +4
                 boot_prep_vxworks                           48      52      +4
                 boot_jump_vxworks                            6      10      +4
                 boot_jump_linux                            148     152      +4
                 boot_get_ramdisk                           420     392     -28
                 boot_get_fdt                               420     344     -76

And looking at ls1088ardb_sdcard_qspi_SECURE_BOOT I think there might be
something wrong as that looks to drop all crypto algos from SPL.  Other
layerscape SECURE_BOOT configs show this as well.  It does however seem
to clear up some other issues around unused code, so a deeper dive on
which patch is dropping stuff is needed.  I see a huge drop on
am65x_evm_a53 / j721e_evm_a72 SPL as well but I can test those and at
least the basic case is fine.  socfpga_agilex_atf is one I don't know
about being right or wrong.  socfpga_agilex_vab dropping hashing code
does look worrying however, but maybe it's a configuration issue in the
end?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]