public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
@ 2021-05-03 23:10 Simon Glass
  2021-05-03 23:10 ` [PATCH 04/49] btrfs: Use U-Boot API for decompression Simon Glass
  2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
  0 siblings, 2 replies; 10+ messages in thread
From: Simon Glass @ 2021-05-03 23:10 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, Simon Glass, AKASHI Takahiro,
	Adam Ford, Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov,
	Asherah Connor, Bastian Krause, Chan, Donald, Chee Hong Ang,
	Chin-Liang See, Christian Gmeiner, Dinh Nguyen, Etienne Carriere,
	Eugeniy Paltsev, Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sean Anderson,
	Sebastian Reichel, Siew Chin Lim, Stefan Roese, Stefano Babic,
	Suniel Mahesh, T Karthik Reddy, Tero Kristo,
	Thirupathaiah Annapureddy, Trevor Woerner, Wasim Khan, chenshuo,
	linux-btrfs, uboot-snps-arc

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.

Changes in v2:
- Add new abuf_init_set() function

Simon Glass (49):
  Add support for an owned buffer
  compiler: Add a comment to host_build()
  zstd: Create a function for use from U-Boot
  btrfs: Use U-Boot API for decompression
  image: Avoid switch default in image_decomp()
  image: Update zstd to avoid reporting error twice
  gzip: Avoid use of u64
  image: Update image_decomp() to avoid ifdefs
  image: Split board code out into its own file
  image: Fix up checkpatch warnings in image-board.c
  image: Split host code out into its own file
  image: Create a function to do manual relocation
  image: Avoid #ifdefs for manual relocation
  image: Remove ifdefs around image_setup_linux() el at
  image: Add Kconfig options for FIT in the host build
  kconfig: Add host support to CONFIG_IS_ENABLED()
  image: Shorten FIT_ENABLE_SHAxxx_SUPPORT
  image: Rename SPL_SHAxxx_SUPPORT to SPL_FIT_SHAxxx
  hash: Use Kconfig to enable hashing in host tools
  hash: Drop some #ifdefs in hash.c
  image: Drop IMAGE_ENABLE_FIT
  image: Drop IMAGE_ENABLE_OF_LIBFDT
  image: Use Kconfig to enable CONFIG_FIT_VERBOSE on host
  image: Rename CONFIG_FIT_ENABLE_RSASSA_PSS_SUPPORT
  image: Use Kconfig to enable FIT_RSASSA_PSS on host
  Kconfig: Rename SPL_CRC32_SUPPORT to SPL_CRC32
  image: Drop IMAGE_ENABLE_CRC32
  Kconfig: Rename SPL_MD5_SUPPORT to SPL_MD5
  image: Drop IMAGE_ENABLE_MD5
  image: Drop IMAGE_ENABLE_SHA1
  image: Drop IMAGE_ENABLE_SHAxxx
  image: Drop IMAGE_BOOT_GET_CMDLINE
  image: Drop IMAGE_OF_BOARD_SETUP
  image: Drop IMAGE_OF_SYSTEM_SETUP
  image: Drop IMAGE_ENABLE_IGNORE
  image: Drop IMAGE_ENABLE_SIGN/VERIFY defines
  image: Drop IMAGE_ENABLE_BEST_MATCH
  image: Drop IMAGE_ENABLE_EN/DECRYPT defines
  image: Tidy up fit_unsupported_reset()
  image: Drop unnecessary #ifdefs from image.h
  image: Drop #ifdefs for fit_print_contents()
  image: Drop most #ifdefs in image-board.c
  image: Reduce variable scope in boot_get_ramdisk()
  image: Split up boot_get_ramdisk()
  image: Remove #ifdefs from select_ramdisk()
  image: Remove some #ifdefs from image-fit and image-fit-sig
  image: Reduce variable scope in boot_get_fdt()
  image: Split up boot_get_fdt()
  image: Remove #ifdefs from select_fdt()

 arch/arc/lib/bootm.c                      |    2 +-
 arch/arm/lib/bootm.c                      |    4 +-
 arch/arm/mach-imx/hab.c                   |    2 +-
 arch/microblaze/lib/bootm.c               |    2 +-
 arch/nds32/lib/bootm.c                    |    4 +-
 arch/riscv/lib/bootm.c                    |    4 +-
 board/synopsys/hsdk/hsdk.c                |    2 +-
 common/Kconfig.boot                       |   18 +-
 common/Makefile                           |    2 +-
 common/bootm.c                            |   30 +-
 common/bootm_os.c                         |    8 +
 common/hash.c                             |   96 +-
 common/image-board.c                      |  958 +++++++++++++++++
 common/image-cipher.c                     |    6 +-
 common/image-fdt.c                        |  275 ++---
 common/image-fit-sig.c                    |    7 +-
 common/image-fit.c                        |   42 +-
 common/image-host.c                       |   27 +
 common/image-sig.c                        |   57 +-
 common/image.c                            | 1179 ++-------------------
 common/spl/Kconfig                        |   27 +-
 configs/axm_defconfig                     |    2 +-
 configs/bcm963158_ram_defconfig           |    2 +-
 configs/chromebit_mickey_defconfig        |    2 +-
 configs/chromebook_jerry_defconfig        |    2 +-
 configs/chromebook_minnie_defconfig       |    2 +-
 configs/chromebook_speedy_defconfig       |    2 +-
 configs/evb-px30_defconfig                |    2 +-
 configs/firefly-px30_defconfig            |    2 +-
 configs/imxrt1020-evk_defconfig           |    2 +-
 configs/imxrt1050-evk_defconfig           |    2 +-
 configs/mt8516_pumpkin_defconfig          |    2 +-
 configs/odroid-go2_defconfig              |    2 +-
 configs/px30-core-ctouch2-px30_defconfig  |    2 +-
 configs/px30-core-edimm2.2-px30_defconfig |    2 +-
 configs/sandbox_defconfig                 |    3 +-
 configs/socfpga_agilex_atf_defconfig      |    2 +-
 configs/socfpga_agilex_vab_defconfig      |    2 +-
 configs/socfpga_stratix10_atf_defconfig   |    2 +-
 configs/taurus_defconfig                  |    2 +-
 fs/btrfs/compression.c                    |   51 +-
 include/abuf.h                            |  148 +++
 include/compiler.h                        |    8 +
 include/fdt_support.h                     |    2 +-
 include/gzip.h                            |    8 +-
 include/image.h                           |  178 +---
 include/linux/kconfig.h                   |   13 +-
 include/linux/zstd.h                      |   11 +
 include/relocate.h                        |   30 +-
 include/u-boot/aes.h                      |    8 +-
 include/u-boot/ecdsa.h                    |    2 +-
 include/u-boot/hash-checksum.h            |    5 +-
 include/u-boot/rsa.h                      |   12 +-
 lib/Kconfig                               |    5 +
 lib/Makefile                              |    5 +-
 lib/abuf.c                                |  103 ++
 lib/gunzip.c                              |   28 +-
 lib/hash-checksum.c                       |    2 +-
 lib/lmb.c                                 |    2 +-
 lib/rsa/rsa-sign.c                        |    4 +-
 lib/rsa/rsa-verify.c                      |    4 +-
 lib/zstd/Makefile                         |    2 +-
 lib/zstd/zstd.c                           |   64 ++
 test/lib/Makefile                         |    1 +
 test/lib/abuf.c                           |  303 ++++++
 tools/Kconfig                             |  111 ++
 tools/Makefile                            |   19 +-
 tools/image-host.c                        |    6 +-
 68 files changed, 2276 insertions(+), 1650 deletions(-)
 create mode 100644 common/image-board.c
 create mode 100644 common/image-host.c
 create mode 100644 include/abuf.h
 create mode 100644 lib/abuf.c
 create mode 100644 lib/zstd/zstd.c
 create mode 100644 test/lib/abuf.c

-- 
2.31.1.527.g47e6f16901-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 04/49] btrfs: Use U-Boot API for decompression
  2021-05-03 23:10 [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Simon Glass
@ 2021-05-03 23:10 ` Simon Glass
  2021-05-03 23:25   ` Qu Wenruo
  2021-05-03 23:45   ` Marek Behun
  2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
  1 sibling, 2 replies; 10+ messages in thread
From: Simon Glass @ 2021-05-03 23:10 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, Simon Glass, Marek Behun,
	Qu Wenruo, linux-btrfs

Use the common function to avoid code duplication.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 fs/btrfs/compression.c | 51 +++++-------------------------------------
 1 file changed, 5 insertions(+), 46 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 23efefa1997..7adfbb04a7c 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -6,6 +6,7 @@
  */
 
 #include "btrfs.h"
+#include <abuf.h>
 #include <log.h>
 #include <malloc.h>
 #include <linux/lzo.h>
@@ -136,54 +137,12 @@ static u32 decompress_zlib(const u8 *_cbuf, u32 clen, u8 *dbuf, u32 dlen)
 
 static u32 decompress_zstd(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
 {
-	ZSTD_DStream *dstream;
-	ZSTD_inBuffer in_buf;
-	ZSTD_outBuffer out_buf;
-	void *workspace;
-	size_t wsize;
-	u32 res = -1;
-
-	wsize = ZSTD_DStreamWorkspaceBound(ZSTD_BTRFS_MAX_INPUT);
-	workspace = malloc(wsize);
-	if (!workspace) {
-		debug("%s: cannot allocate workspace of size %zu\n", __func__,
-		      wsize);
-		return -1;
-	}
-
-	dstream = ZSTD_initDStream(ZSTD_BTRFS_MAX_INPUT, workspace, wsize);
-	if (!dstream) {
-		printf("%s: ZSTD_initDStream failed\n", __func__);
-		goto err_free;
-	}
+	struct abuf in, out;
 
-	in_buf.src = cbuf;
-	in_buf.pos = 0;
-	in_buf.size = clen;
+	abuf_init_set(&in, (u8 *)cbuf, clen);
+	abuf_init_set(&out, dbuf, dlen);
 
-	out_buf.dst = dbuf;
-	out_buf.pos = 0;
-	out_buf.size = dlen;
-
-	while (1) {
-		size_t ret;
-
-		ret = ZSTD_decompressStream(dstream, &out_buf, &in_buf);
-		if (ZSTD_isError(ret)) {
-			printf("%s: ZSTD_decompressStream error %d\n", __func__,
-			       ZSTD_getErrorCode(ret));
-			goto err_free;
-		}
-
-		if (in_buf.pos >= clen || !ret)
-			break;
-	}
-
-	res = out_buf.pos;
-
-err_free:
-	free(workspace);
-	return res;
+	return zstd_decompress(&in, &out);
 }
 
 u32 btrfs_decompress(u8 type, const char *c, u32 clen, char *d, u32 dlen)
-- 
2.31.1.527.g47e6f16901-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 04/49] btrfs: Use U-Boot API for decompression
  2021-05-03 23:10 ` [PATCH 04/49] btrfs: Use U-Boot API for decompression Simon Glass
@ 2021-05-03 23:25   ` Qu Wenruo
  2021-05-03 23:45   ` Marek Behun
  1 sibling, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2021-05-03 23:25 UTC (permalink / raw)
  To: Simon Glass, U-Boot Mailing List
  Cc: Tom Rini, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, Marek Behun, Qu Wenruo,
	linux-btrfs



On 2021/5/4 上午7:10, Simon Glass wrote:
> Use the common function to avoid code duplication.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>
> (no changes since v1)
>
>   fs/btrfs/compression.c | 51 +++++-------------------------------------
>   1 file changed, 5 insertions(+), 46 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 23efefa1997..7adfbb04a7c 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -6,6 +6,7 @@
>    */
>
>   #include "btrfs.h"
> +#include <abuf.h>
>   #include <log.h>
>   #include <malloc.h>
>   #include <linux/lzo.h>
> @@ -136,54 +137,12 @@ static u32 decompress_zlib(const u8 *_cbuf, u32 clen, u8 *dbuf, u32 dlen)
>
>   static u32 decompress_zstd(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
>   {
> -	ZSTD_DStream *dstream;
> -	ZSTD_inBuffer in_buf;
> -	ZSTD_outBuffer out_buf;
> -	void *workspace;
> -	size_t wsize;
> -	u32 res = -1;
> -
> -	wsize = ZSTD_DStreamWorkspaceBound(ZSTD_BTRFS_MAX_INPUT);
> -	workspace = malloc(wsize);
> -	if (!workspace) {
> -		debug("%s: cannot allocate workspace of size %zu\n", __func__,
> -		      wsize);
> -		return -1;
> -	}
> -
> -	dstream = ZSTD_initDStream(ZSTD_BTRFS_MAX_INPUT, workspace, wsize);
> -	if (!dstream) {
> -		printf("%s: ZSTD_initDStream failed\n", __func__);
> -		goto err_free;
> -	}
> +	struct abuf in, out;
>
> -	in_buf.src = cbuf;
> -	in_buf.pos = 0;
> -	in_buf.size = clen;
> +	abuf_init_set(&in, (u8 *)cbuf, clen);
> +	abuf_init_set(&out, dbuf, dlen);
>
> -	out_buf.dst = dbuf;
> -	out_buf.pos = 0;
> -	out_buf.size = dlen;
> -
> -	while (1) {
> -		size_t ret;
> -
> -		ret = ZSTD_decompressStream(dstream, &out_buf, &in_buf);
> -		if (ZSTD_isError(ret)) {
> -			printf("%s: ZSTD_decompressStream error %d\n", __func__,
> -			       ZSTD_getErrorCode(ret));
> -			goto err_free;
> -		}
> -
> -		if (in_buf.pos >= clen || !ret)
> -			break;
> -	}
> -
> -	res = out_buf.pos;
> -
> -err_free:
> -	free(workspace);
> -	return res;
> +	return zstd_decompress(&in, &out);
>   }
>
>   u32 btrfs_decompress(u8 type, const char *c, u32 clen, char *d, u32 dlen)
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 04/49] btrfs: Use U-Boot API for decompression
  2021-05-03 23:10 ` [PATCH 04/49] btrfs: Use U-Boot API for decompression Simon Glass
  2021-05-03 23:25   ` Qu Wenruo
@ 2021-05-03 23:45   ` Marek Behun
  2021-05-04 16:58     ` Simon Glass
  1 sibling, 1 reply; 10+ messages in thread
From: Marek Behun @ 2021-05-03 23:45 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt, Bin Meng,
	Robert Marko, Andre Przywara, Masahiro Yamada, Qu Wenruo,
	linux-btrfs

On Mon,  3 May 2021 17:10:51 -0600
Simon Glass <sjg@chromium.org> wrote:

> Use the common function to avoid code duplication.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Is this tested? Why only zstd?

marek

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 04/49] btrfs: Use U-Boot API for decompression
  2021-05-03 23:45   ` Marek Behun
@ 2021-05-04 16:58     ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2021-05-04 16:58 UTC (permalink / raw)
  To: Marek Behun
  Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt, Bin Meng,
	Robert Marko, Andre Przywara, Masahiro Yamada, Qu Wenruo,
	linux-btrfs

Hi Marek,

On Mon, 3 May 2021 at 17:45, Marek Behun <marek.behun@nic.cz> wrote:
>
> On Mon,  3 May 2021 17:10:51 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > Use the common function to avoid code duplication.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Is this tested? Why only zstd?

No, there are no tests for zstd, as I mentioned in the other patch.
Are you able to add some to the compression.c file? It should really
be done if we expect it to keep working in U-Boot.

The other compression algos already have a suitable function, so don't
need to be reworked.

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
  2021-05-03 23:10 [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Simon Glass
  2021-05-03 23:10 ` [PATCH 04/49] btrfs: Use U-Boot API for decompression Simon Glass
@ 2021-05-04 21:40 ` Tom Rini
  2021-05-04 21:49   ` Simon Glass
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Tom Rini @ 2021-05-04 21:40 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, AKASHI Takahiro, Adam Ford,
	Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov, Asherah Connor,
	Bastian Krause, Chan, Donald, Chee Hong Ang, Chin-Liang See,
	Christian Gmeiner, Dinh Nguyen, Etienne Carriere, Eugeniy Paltsev,
	Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sean Anderson,
	Sebastian Reichel, Siew Chin Lim, Stefan Roese, Stefano Babic,
	Suniel Mahesh, T Karthik Reddy, Tero Kristo,
	Thirupathaiah Annapureddy, Trevor Woerner, Wasim Khan, chenshuo,
	linux-btrfs, uboot-snps-arc

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
  2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
@ 2021-05-04 21:49   ` Simon Glass
  2021-05-04 23:24   ` Sean Anderson
  2021-05-05 23:38   ` Simon Glass
  2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2021-05-04 21:49 UTC (permalink / raw)
  To: Tom Rini
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, AKASHI Takahiro, Adam Ford,
	Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov, Asherah Connor,
	Bastian Krause, Chan, Donald, Chee Hong Ang, Chin-Liang See,
	Christian Gmeiner, Dinh Nguyen, Etienne Carriere, Eugeniy Paltsev,
	Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sean Anderson,
	Sebastian Reichel, Siew Chin Lim, Stefan Roese, Stefano Babic,
	Suniel Mahesh, T Karthik Reddy, Tero Kristo,
	Thirupathaiah Annapureddy, Trevor Woerner, Wasim Khan, chenshuo,
	linux-btrfs, uboot-snps-arc

Hi Tom,

On Tue, 4 May 2021 at 15:40, Tom Rini <trini@konsulko.com> wrote:
>
> 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?

OK thanks for that. I will take a look at the cases you mention. I
found a fair few problems but clearly not all.

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
  2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
  2021-05-04 21:49   ` Simon Glass
@ 2021-05-04 23:24   ` Sean Anderson
  2021-05-05  1:11     ` Tom Rini
  2021-05-05 23:38   ` Simon Glass
  2 siblings, 1 reply; 10+ messages in thread
From: Sean Anderson @ 2021-05-04 23:24 UTC (permalink / raw)
  To: Tom Rini, Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, AKASHI Takahiro, Adam Ford,
	Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov, Asherah Connor,
	Bastian Krause, Chan, Donald, Chee Hong Ang, Chin-Liang See,
	Christian Gmeiner, Dinh Nguyen, Etienne Carriere, Eugeniy Paltsev,
	Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sebastian Reichel,
	Siew Chin Lim, Stefan Roese, Stefano Babic, Suniel Mahesh,
	T Karthik Reddy, Tero Kristo, Thirupathaiah Annapureddy,
	Trevor Woerner, Wasim Khan, chenshuo, linux-btrfs, uboot-snps-arc

Hi Tom,

On 5/4/21 5:40 PM, Tom Rini wrote:
> 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):
What tool do you use to generate this output? Thanks.

--Sean

>              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?
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
  2021-05-04 23:24   ` Sean Anderson
@ 2021-05-05  1:11     ` Tom Rini
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2021-05-05  1:11 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Simon Glass, U-Boot Mailing List, Heinrich Schuchardt, Bin Meng,
	Robert Marko, Andre Przywara, Masahiro Yamada, AKASHI Takahiro,
	Adam Ford, Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov,
	Asherah Connor, Bastian Krause, Chan, Donald, Chee Hong Ang,
	Chin-Liang See, Christian Gmeiner, Dinh Nguyen, Etienne Carriere,
	Eugeniy Paltsev, Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sebastian Reichel,
	Siew Chin Lim, Stefan Roese, Stefano Babic, Suniel Mahesh,
	T Karthik Reddy, Tero Kristo, Thirupathaiah Annapureddy,
	Trevor Woerner, Wasim Khan, chenshuo, linux-btrfs, uboot-snps-arc

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

On Tue, May 04, 2021 at 07:24:25PM -0400, Sean Anderson wrote:
> Hi Tom,
> 
> On 5/4/21 5:40 PM, Tom Rini wrote:
> > 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):
> What tool do you use to generate this output? Thanks.

buildman will give that for you.  This was from my world build
before/after wrapper, but for a single machine I have:
#!/bin/bash

# Initial and constant buildman args
ARGS="-devl"
ALL=0
KEEP=0

# Find our arguments
while test $# -ne 0; do
	if [ "$1" == "--all" ]; then
		ALL=1
		shift 1
	elif [ "$1" == "--branch" ]; then
		BRANCH=$2
		shift 2
	elif [ "$1" == "--keep" ]; then
		KEEP=1
		ARGS="$ARGS -k"
		shift 1
	else
		MACHINE=$1
		shift
	fi
done

if [ -z $MACHINE ]; then
	echo Usage: $0 MACHINE [--all] [--keep] [--branch BRANCH]
	exit 1
fi

# If not all, then only first/last
if [ $ALL -ne 1 ]; then
	ARGS="$ARGS --step 0"
fi

if [ ! -z $BRANCH ]; then
	ARGS="$ARGS -b $BRANCH"
else
	ARGS="$ARGS -b `git rev-parse --abbrev-ref HEAD`"
fi

mkdir -p /tmp/$MACHINE

export SOURCE_DATE_EPOCH=`date +%s`
./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SBC $MACHINE
./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SsB $MACHINE

[ $KEEP -eq 0 ] && rm -rf /tmp/$MACHINE

In a script called "uboot-size-test.sh" to dig in to individual cases
more.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code
  2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
  2021-05-04 21:49   ` Simon Glass
  2021-05-04 23:24   ` Sean Anderson
@ 2021-05-05 23:38   ` Simon Glass
  2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2021-05-05 23:38 UTC (permalink / raw)
  To: Tom Rini
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Bin Meng, Robert Marko,
	Andre Przywara, Masahiro Yamada, AKASHI Takahiro, Adam Ford,
	Alexandru Gagniuc, Alexey Brodkin, Andrii Anisov, Asherah Connor,
	Bastian Krause, Chan, Donald, Chee Hong Ang, Chin-Liang See,
	Christian Gmeiner, Dinh Nguyen, Etienne Carriere, Eugeniy Paltsev,
	Fabien Parent, Fabio Estevam, Frieder Schrempf,
	Frédéric Danis, George McCollister, Giulio Benetti,
	Harald Seiler, Heiko Schocher, Heiko Stuebner, Hongwei Zhang,
	Jagan Teki, Jan Kiszka, Jan Luebbe, Jernej Skrabec,
	Joe Hershberger, Joel Peshkin, Joel Stanley, Jonathan Gray,
	Jorge Ramirez-Ortiz, Kever Yang, Klaus Heinrich Kiwi,
	Ley Foon Tan, Lukasz Majewski, Marcin Juszkiewicz, Marek Behun,
	Marek Szyprowski, Marek Vasut, Masahiro Yamada, Matthieu CASTET,
	Michal Simek, Michal Simek, NXP i.MX U-Boot Team, Naoki Hayama,
	Oleksandr Andrushchenko, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Patrick Oppenlander, Peng Fan, Philippe Reynes,
	Pragnesh Patel, Qu Wenruo, Rasmus Villemoes, Reuben Dowle,
	Rick Chen, Samuel Holland, Sean Anderson, Sean Anderson,
	Sebastian Reichel, Siew Chin Lim, Stefan Roese, Stefano Babic,
	Suniel Mahesh, T Karthik Reddy, Tero Kristo,
	Thirupathaiah Annapureddy, Trevor Woerner, Wasim Khan, chenshuo,
	linux-btrfs, uboot-snps-arc

Hi Tom,

On Tue, 4 May 2021 at 14:40, Tom Rini <trini@konsulko.com> wrote:
>
> 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?

Yes it seems to be config. Thank you for all the pointers.

The small increase is unavoidable with this approach - basically we
add 16 bytes of rodata as part of making the switch cases into if()
instead of #ifdef. I found that SPL hashing was dropped, which
explained another problem that I had seen but not yet diagnosed.

I will send v2.

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-05-05 23:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-03 23:10 [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Simon Glass
2021-05-03 23:10 ` [PATCH 04/49] btrfs: Use U-Boot API for decompression Simon Glass
2021-05-03 23:25   ` Qu Wenruo
2021-05-03 23:45   ` Marek Behun
2021-05-04 16:58     ` Simon Glass
2021-05-04 21:40 ` [PATCH 00/49] image: Reduce #ifdefs and ad-hoc defines in image code Tom Rini
2021-05-04 21:49   ` Simon Glass
2021-05-04 23:24   ` Sean Anderson
2021-05-05  1:11     ` Tom Rini
2021-05-05 23:38   ` Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox