[U-Boot] [PATCH v3] spl: add overall SPL size check

[Jean-Jacques Hiblot <jjhiblot@ti.com>]

On 27/05/2019 17:15, Simon Goldschmidt wrote:
>
>
> Tom Rini <trini at konsulko.com <mailto:trini@konsulko.com>> schrieb am 
> Mo., 27. Mai 2019, 16:54:
>
>     On Mon, May 27, 2019 at 03:47:13PM +0200, Jean-Jacques Hiblot wrote:
>     > Simon,
>     >
>     >
>     > On 24/05/2019 22:10, Simon Goldschmidt wrote:
>     > >Am 24.05.2019 um 22:07 schrieb Simon Goldschmidt:
>     > >>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
>     <simon.k.r.goldschmidt@gmail.com
>     <mailto:simon.k.r.goldschmidt@gmail.com>>
>     > >>---
>     > >>
>     > >>Changes in v3:
>     > >>- don't build this new tools for 'make tools-only'
>     > >
>     > >So this is how far I got.
>     > >
>     > >Tom, your idea with making this multi-config aware (U-Boot, SPL
>     and TPL)
>     > >does not seem to work as 'tools' are only built once, not once per
>     > >U-Boot/SPL/TPL. So if we wanted to use this for TPL, too, that
>     would
>     > >either mean create yet another tool or pass an option to this
>     new tool to
>     > >differ between SPL and TPL.
>     >
>     > If the trouble comes from GENERATED_GBL_DATA_SIZE, you could get
>     its value
>     > by parsing lib/asm-offsets.s. all the other values could be
>     extracted from
>     > {.,spl,tpl}/u-boot.cfg
>
>     Getting that file to exist has the same problem over for "tools-only".
>
>     > If this flies, it could be done by a python script without the
>     need to
>     > compile a program
>
>     I'm not sure that provides better clarity over what we have here tho.
>
>
> I also think a python script would be less clear than a C tool. But it 
> could have the problem hidden by not being used for "tools-only" - it 
> would only be executed after linking SPL...

Yes that is  what I was trying to express.

Below is an draft of what I had in mind. Admittedly it less clear than 
the C file.

JJ

>
> Regrds,
> Simon


  Makefile                |  2 +-

  tools/spl_size_limit.py | 51 +++++++++++++++++++++++++++++++++++++++++
  2 files changed, 52 insertions(+), 1 deletion(-)
  create mode 100755 tools/spl_size_limit.py

diff --git a/Makefile b/Makefile
index 8de3d4120a..440ea1da2d 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,$@,$$(tools/spl_size_limit))
+SPL_SIZE_CHECK = $(call size_check,$@,$$($(src)/tools/spl_size_limit.py 
SPL))
  else
  SPL_SIZE_CHECK =
  endif
diff --git a/tools/spl_size_limit.py b/tools/spl_size_limit.py
new file mode 100755
index 0000000000..2c40f5701e
--- /dev/null
+++ b/tools/spl_size_limit.py
@@ -0,0 +1,51 @@
+#! /usr/bin/env python
+import os
+import sys
+
+binary_types = {"U-BOOT":("","./"), "SPL":("SPL_","spl/"), 
"TPL":("TPL_","tpl/")}
+
+def get_from_file(f, pattern):
+    for l in f.readlines():
+        if l.startswith(pattern):
+            return l[len(pattern):].strip()
+    return None
+
+def get_from_cfg_file(bintype, name):
+    pattern = '#define CONFIG_{}{} '.format(binary_types[bintype][0], name)
+    with open("{}u-boot.cfg".format(binary_types[bintype][1])) as f:
+        return get_from_file(f, pattern)
+
+def get_from_header(fname, name):
+    pattern = '#define {} '.format(name)
+    with open("{}".format(fname)) as f:
+        return get_from_file(f, pattern).split()[0]
+
+def usage():
+    print("usage: {} [SPL|TPL]")
+    sys.exit(1)
+
+if len(sys.argv) != 2 and len(sys.argv) != 1:
+    usage()
+if len(sys.argv) == 2 and sys.argv[1] not in binary_types.keys():
+    usage()
+
+bintype = sys.argv[1]
+
+gbl_data_size = 
int(get_from_header("include/generated/generic-asm-offsets.h",
+                    "GENERATED_GBL_DATA_SIZE"), 0)
+size_limit_subtract_gd = get_from_cfg_file(bintype, 
"SIZE_LIMIT_SUBTRACT_GD")
+size_limit_subtract_malloc = get_from_cfg_file(bintype,
+                           "SIZE_LIMIT_SUBTRACT_MALLOC")
+sys_malloc_f_len = int(get_from_cfg_file(bintype, "SYS_MALLOC_F_LEN"), 0)
+size_limit_provide_stack = get_from_cfg_file(bintype,
+                         "SIZE_LIMIT_PROVIDE_STACK")
+
+size_limit = int(get_from_cfg_file(bintype, "SIZE_LIMIT"), 0)
+if size_limit_subtract_gd:
+    size_limit = size_limit - gbl_data_size
+if size_limit_subtract_malloc:
+    size_limit = size_limit - sys_malloc_f_len
+if size_limit_provide_stack:
+    size_limit = size_limit - int(size_limit_provide_stack, 0)
+
+print(size_limit)
--