Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
Am 27.05.2019 um 18:25 schrieb Masahiro Yamada: On Fri, May 24, 2019 at 5:17 AM Tom Rini wrote: On Thu, May 23, 2019 at 10:08:54PM +0200, Simon Goldschmidt wrote: Am 13.05.2019 um 14:45 schrieb Simon Goldschmidt: On Sat, May 11, 2019 at 3:55 AM Tom Rini wrote: On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: This adds a size check for SPL that can dynamically check generated SPL binaries (including devicetree) for a size limit that ensures this image plus global data, heap and stack fit in initial SRAM. Since some of these sizes are not available to make, a new host tool 'spl_size_limit' is added that dumps the resulting maximum size for an SPL binary to stdout. This tool is used in toplevel Makefile to implement the size check on SPL binaries. Signed-off-by: Simon Goldschmidt OK, this has a race / dependency problem: https://travis-ci.org/trini/u-boot/jobs/530803338 Hmm, let me check that. OK, so as I see it, this is not a race but a dependency problem: the "tools-only" target just does not generate the "generic-asm-offsets.h" file. However, just adding a dependency to include/generated/generic-asm-offsets.h leads to an error (no rule to build that file) since that file is generated by Kbuild magic that I don't really get... Any idea how to make this depend on /Kbuild being run? For context, https://patchwork.ozlabs.org/patch/105/ Yamada-san, do you have any ideas? Thanks! No, I have no idea. 'tools' depends on 'prepare', which depends on 'prepare0'. So, the dependency should be correct, but I do not see CHK include/generated/generic-asm-offsets.h in the log at all. I do not know why. Thanks for the hint! The target in question was 'tools-only', not 'tools'. And in contrast to 'tools', 'tools-only' is missing the dependency to 'prepare'. We could just add that unless we go JJ's python way. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Fri, May 24, 2019 at 5:17 AM Tom Rini wrote: > > On Thu, May 23, 2019 at 10:08:54PM +0200, Simon Goldschmidt wrote: > > Am 13.05.2019 um 14:45 schrieb Simon Goldschmidt: > > >On Sat, May 11, 2019 at 3:55 AM Tom Rini wrote: > > >> > > >>On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > > >> > > >>>This adds a size check for SPL that can dynamically check generated > > >>>SPL binaries (including devicetree) for a size limit that ensures > > >>>this image plus global data, heap and stack fit in initial SRAM. > > >>> > > >>>Since some of these sizes are not available to make, a new host tool > > >>>'spl_size_limit' is added that dumps the resulting maximum size for > > >>>an SPL binary to stdout. This tool is used in toplevel Makefile to > > >>>implement the size check on SPL binaries. > > >>> > > >>>Signed-off-by: Simon Goldschmidt > > >> > > >>OK, this has a race / dependency problem: > > >>https://travis-ci.org/trini/u-boot/jobs/530803338 > > > > > >Hmm, let me check that. > > > > OK, so as I see it, this is not a race but a dependency problem: the > > "tools-only" target just does not generate the "generic-asm-offsets.h" file. > > > > However, just adding a dependency to include/generated/generic-asm-offsets.h > > leads to an error (no rule to build that file) since that file is generated > > by Kbuild magic that I don't really get... > > > > Any idea how to make this depend on /Kbuild being run? > > For context, https://patchwork.ozlabs.org/patch/105/ > > Yamada-san, do you have any ideas? Thanks! No, I have no idea. 'tools' depends on 'prepare', which depends on 'prepare0'. So, the dependency should be correct, but I do not see CHK include/generated/generic-asm-offsets.h in the log at all. I do not know why. -- Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Thu, May 23, 2019 at 10:08:54PM +0200, Simon Goldschmidt wrote: > Am 13.05.2019 um 14:45 schrieb Simon Goldschmidt: > >On Sat, May 11, 2019 at 3:55 AM Tom Rini wrote: > >> > >>On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > >> > >>>This adds a size check for SPL that can dynamically check generated > >>>SPL binaries (including devicetree) for a size limit that ensures > >>>this image plus global data, heap and stack fit in initial SRAM. > >>> > >>>Since some of these sizes are not available to make, a new host tool > >>>'spl_size_limit' is added that dumps the resulting maximum size for > >>>an SPL binary to stdout. This tool is used in toplevel Makefile to > >>>implement the size check on SPL binaries. > >>> > >>>Signed-off-by: Simon Goldschmidt > >> > >>OK, this has a race / dependency problem: > >>https://travis-ci.org/trini/u-boot/jobs/530803338 > > > >Hmm, let me check that. > > OK, so as I see it, this is not a race but a dependency problem: the > "tools-only" target just does not generate the "generic-asm-offsets.h" file. > > However, just adding a dependency to include/generated/generic-asm-offsets.h > leads to an error (no rule to build that file) since that file is generated > by Kbuild magic that I don't really get... > > Any idea how to make this depend on /Kbuild being run? For context, https://patchwork.ozlabs.org/patch/105/ Yamada-san, do you have any ideas? Thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
Am 13.05.2019 um 14:45 schrieb Simon Goldschmidt: On Sat, May 11, 2019 at 3:55 AM Tom Rini wrote: On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: This adds a size check for SPL that can dynamically check generated SPL binaries (including devicetree) for a size limit that ensures this image plus global data, heap and stack fit in initial SRAM. Since some of these sizes are not available to make, a new host tool 'spl_size_limit' is added that dumps the resulting maximum size for an SPL binary to stdout. This tool is used in toplevel Makefile to implement the size check on SPL binaries. Signed-off-by: Simon Goldschmidt OK, this has a race / dependency problem: https://travis-ci.org/trini/u-boot/jobs/530803338 Hmm, let me check that. OK, so as I see it, this is not a race but a dependency problem: the "tools-only" target just does not generate the "generic-asm-offsets.h" file. However, just adding a dependency to include/generated/generic-asm-offsets.h leads to an error (no rule to build that file) since that file is generated by Kbuild magic that I don't really get... Any idea how to make this depend on /Kbuild being run? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Sat, May 11, 2019 at 3:55 AM Tom Rini wrote: > > On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > > > This adds a size check for SPL that can dynamically check generated > > SPL binaries (including devicetree) for a size limit that ensures > > this image plus global data, heap and stack fit in initial SRAM. > > > > Since some of these sizes are not available to make, a new host tool > > 'spl_size_limit' is added that dumps the resulting maximum size for > > an SPL binary to stdout. This tool is used in toplevel Makefile to > > implement the size check on SPL binaries. > > > > Signed-off-by: Simon Goldschmidt > > OK, this has a race / dependency problem: > https://travis-ci.org/trini/u-boot/jobs/530803338 Hmm, let me check that. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > This adds a size check for SPL that can dynamically check generated > SPL binaries (including devicetree) for a size limit that ensures > this image plus global data, heap and stack fit in initial SRAM. > > Since some of these sizes are not available to make, a new host tool > 'spl_size_limit' is added that dumps the resulting maximum size for > an SPL binary to stdout. This tool is used in toplevel Makefile to > implement the size check on SPL binaries. > > Signed-off-by: Simon Goldschmidt OK, this has a race / dependency problem: https://travis-ci.org/trini/u-boot/jobs/530803338 -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Mon, Apr 22, 2019 at 11:10 PM Tom Rini wrote: > > On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > > This adds a size check for SPL that can dynamically check generated > > SPL binaries (including devicetree) for a size limit that ensures > > this image plus global data, heap and stack fit in initial SRAM. > > > > Since some of these sizes are not available to make, a new host tool > > 'spl_size_limit' is added that dumps the resulting maximum size for > > an SPL binary to stdout. This tool is used in toplevel Makefile to > > implement the size check on SPL binaries. > > > > Signed-off-by: Simon Goldschmidt > > --- > > > > Changes in v2: > > - added missing tools/spl_size_limit.c > > > > Kconfig| 8 > > Makefile | 2 +- > > common/spl/Kconfig | 36 > > tools/Makefile | 2 ++ > > tools/spl_size_limit.c | 30 ++ > > Ah, now I get it, and why you said you depend on Heinrich's series now, > OK. This isn't quite what I envisioned, but, maybe that's OK. I was > thinking we could drop the whole of the shell function, stat() the file > and return 0/error. But I guess on the whole we've got the shell > function portable and not too fragile looking, so maybe it's not worth > the extra work there. I also thought in your direction first. But then I figured with the current way of doing the size check, showing the current vs. maximum would be easier. If the Makefile didn't know the size, the new tool would have to print it to show the difference... > I do have one problem I'd like to discuss, which is that to replicate > this for a TPL size limit (which we totally have and need to deal with), > we cp and sed spl_size_limit.c to tpl_size_limit.c. > > I don't however see a clever way to avoid that. CONFIG_IS_ENABLED() > would let us have the same #if test, but doing > size_limit -= CONFIG_SPL_FOO CONFIG_TPL_FOO; > is probably getting too clever. Hmm, let me think about that. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spl: add overall SPL size check
On Mon, Apr 22, 2019 at 10:27:21PM +0200, Simon Goldschmidt wrote: > This adds a size check for SPL that can dynamically check generated > SPL binaries (including devicetree) for a size limit that ensures > this image plus global data, heap and stack fit in initial SRAM. > > Since some of these sizes are not available to make, a new host tool > 'spl_size_limit' is added that dumps the resulting maximum size for > an SPL binary to stdout. This tool is used in toplevel Makefile to > implement the size check on SPL binaries. > > Signed-off-by: Simon Goldschmidt > --- > > Changes in v2: > - added missing tools/spl_size_limit.c > > Kconfig| 8 > Makefile | 2 +- > common/spl/Kconfig | 36 > tools/Makefile | 2 ++ > tools/spl_size_limit.c | 30 ++ Ah, now I get it, and why you said you depend on Heinrich's series now, OK. This isn't quite what I envisioned, but, maybe that's OK. I was thinking we could drop the whole of the shell function, stat() the file and return 0/error. But I guess on the whole we've got the shell function portable and not too fragile looking, so maybe it's not worth the extra work there. I do have one problem I'd like to discuss, which is that to replicate this for a TPL size limit (which we totally have and need to deal with), we cp and sed spl_size_limit.c to tpl_size_limit.c. I don't however see a clever way to avoid that. CONFIG_IS_ENABLED() would let us have the same #if test, but doing size_limit -= CONFIG_SPL_FOO CONFIG_TPL_FOO; is probably getting too clever. Thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] spl: add overall SPL size check
This adds a size check for SPL that can dynamically check generated
SPL binaries (including devicetree) for a size limit that ensures
this image plus global data, heap and stack fit in initial SRAM.
Since some of these sizes are not available to make, a new host tool
'spl_size_limit' is added that dumps the resulting maximum size for
an SPL binary to stdout. This tool is used in toplevel Makefile to
implement the size check on SPL binaries.
Signed-off-by: Simon Goldschmidt
---
Changes in v2:
- added missing tools/spl_size_limit.c
Kconfig| 8
Makefile | 2 +-
common/spl/Kconfig | 36
tools/Makefile | 2 ++
tools/spl_size_limit.c | 30 ++
5 files changed, 69 insertions(+), 9 deletions(-)
create mode 100644 tools/spl_size_limit.c
diff --git a/Kconfig b/Kconfig
index ca31bcebde..7a5491bd67 100644
--- a/Kconfig
+++ b/Kconfig
@@ -172,14 +172,6 @@ config TPL_SYS_MALLOC_F_LEN
particular needs this to operate, so that it can allocate the
initial serial device and any others that are needed.
-config SPL_SIZE_LIMIT
- int "Maximum size of SPL image"
- depends on SPL
- default 0
- help
- Specifies the maximum length of the U-Boot SPL image.
- If this value is zero, it is ignored.
-
menuconfig EXPERT
bool "Configure standard U-Boot features (expert users)"
default y
diff --git a/Makefile b/Makefile
index ac375f396c..df674b0e96 100644
--- a/Makefile
+++ b/Makefile
@@ -797,7 +797,7 @@ BOARD_SIZE_CHECK =
endif
ifneq ($(CONFIG_SPL_SIZE_LIMIT),0)
-SPL_SIZE_CHECK = @$(call size_check,$@,$(CONFIG_SPL_SIZE_LIMIT))
+SPL_SIZE_CHECK = @$(call size_check,$@,$$(tools/spl_size_limit))
else
SPL_SIZE_CHECK =
endif
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 206c24076d..24ddab3a48 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -25,6 +25,42 @@ config SPL_FRAMEWORK
supports MMC, NAND and YMODEM and other methods loading of U-Boot
and the Linux Kernel. If unsure, say Y.
+config SPL_SIZE_LIMIT
+ hex "Maximum size of SPL image"
+ depends on SPL
+ default 0
+ help
+ Specifies the maximum length of the U-Boot SPL image.
+ If this value is zero, it is ignored.
+
+config SPL_SIZE_LIMIT_SUBTRACT_GD
+ bool "SPL image size check: provide space for global data"
+ depends on SPL_SIZE_LIMIT > 0
+ help
+ If enabled, aligned size of global data is reserved in
+ SPL_SIZE_LIMIT check to ensure such an image does not overflow SRAM
+ if SPL_SIZE_LIMIT describes the size of SRAM available for SPL when
+ pre-reloc global data is put into this SRAM, too.
+
+config SPL_SIZE_LIMIT_SUBTRACT_MALLOC
+ bool "SPL image size check: provide space for malloc() pool before
relocation"
+ depends on SPL_SIZE_LIMIT > 0
+ help
+ If enabled, SPL_SYS_MALLOC_F_LEN is reserved in SPL_SIZE_LIMIT check
+ to ensure such an image does not overflow SRAM if SPL_SIZE_LIMIT
+ describes the size of SRAM available for SPL when pre-reloc malloc
+ pool is put into this SRAM, too.
+
+config SPL_SIZE_LIMIT_PROVIDE_STACK
+ hex "SPL image size check: provide stack space before relocation"
+ depends on SPL_SIZE_LIMIT > 0
+ default 0
+ help
+ If set, this size is reserved in SPL_SIZE_LIMIT check to ensure such
+ an image does not overflow SRAM if SPL_SIZE_LIMIT describes the size
+ of SRAM available for SPL when the stack required before reolcation
+ uses this SRAM, too.
+
config HANDOFF
bool "Pass hand-off information from SPL to U-Boot proper"
depends on BLOBLIST
diff --git a/tools/Makefile b/tools/Makefile
index d377d85f74..d3b950553f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -199,6 +199,8 @@ hostprogs-$(CONFIG_RISCV) += prelink-riscv
hostprogs-y += fdtgrep
fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o
+hostprogs-y += spl_size_limit
+
hostprogs-$(CONFIG_MIPS) += mips-relocs
# We build some files with extra pedantic flags to try to minimize things
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
new file mode 100644
index 00..98ff491867
--- /dev/null
+++ b/tools/spl_size_limit.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019, Simon Goldschmidt
+ *
+ * This tool helps to return the size available for SPL image during build
+ */
+
+#include
+#include
+
+int main(int argc, char *argv[])
+{
+ int spl_size_limit = 0;
+
+#ifdef CONFIG_SPL_SIZE_LIMIT
+ spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
+#ifdef CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD
+ spl_size_limit -= GENERATED_GBL_DATA_SIZE;
+#endif
+#ifdef CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC
+ spl_size_limit -= CONFIG_SPL_SYS_MALLOC_F_LEN;
+#endif
+#ifdef CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK
+
