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

2019-06-07 Thread Tom Rini
On Fri, May 24, 2019 at 10:07:04PM +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 

Applied to u-boot/master, 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 v3] spl: add overall SPL size check

2019-05-28 Thread Simon Goldschmidt

Am 28.05.2019 um 17:18 schrieb Jean-Jacques Hiblot:


On 27/05/2019 17:15, Simon Goldschmidt wrote:



Tom Rini mailto:tr...@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
mailto:simon.k.r.goldschm...@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.


If it works, then it's ok for me (maybe with a little added 
documentation to make it cleare). I'm not at all obsessed by my version :-)


Regards,
Simon



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 00..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_limi

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

2019-05-28 Thread Jean-Jacques Hiblot


On 27/05/2019 17:15, Simon Goldschmidt wrote:



Tom Rini mailto:tr...@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
mailto:simon.k.r.goldschm...@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 00..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)
--
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-05-27 Thread Simon Goldschmidt

Am 27.05.2019 um 18:48 schrieb Tom Rini:

On Tue, May 28, 2019 at 01:35:52AM +0900, Masahiro Yamada wrote:

On Sat, May 25, 2019 at 5:07 AM 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 
---


Some architectures have SPL size checks.
CONFIG_SPL_MAX_FOOTPRINT
CONFIG_SPL_MAX_SIZE
CONFIG_SPL_BSS_MAX_SIZE

Will they be replaced by this new mechanism?


Yes, as the existing checks are only sufficient for final link time
overflows and not final image overflows.


This patch builds on Heinrich Schuchardt's series that add 
CONFIG_SPL_SIZE_LIMIT to check maximum image size (after potentially 
adding image headers):


https://patchwork.ozlabs.org/patch/1074741/

I just now realize I have forgotton to add that dependency to the patch 
mail.


However, we do have numerous max size defines. This patch tries to bring 
a better calculation for the maximum SPL image size by taking SRAM size 
and subtracting what's required in addition to the binary size.


In general, the different max size defines could need a cleanup as well, 
I guess...


Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-05-27 Thread Tom Rini
On Tue, May 28, 2019 at 01:35:52AM +0900, Masahiro Yamada wrote:
> On Sat, May 25, 2019 at 5:07 AM 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 
> > ---
> 
> Some architectures have SPL size checks.
> CONFIG_SPL_MAX_FOOTPRINT
> CONFIG_SPL_MAX_SIZE
> CONFIG_SPL_BSS_MAX_SIZE
> 
> Will they be replaced by this new mechanism?

Yes, as the existing checks are only sufficient for final link time
overflows and not final image overflows.

-- 
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 v3] spl: add overall SPL size check

2019-05-27 Thread Masahiro Yamada
On Sat, May 25, 2019 at 5:07 AM 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 
> ---

Some architectures have SPL size checks.
CONFIG_SPL_MAX_FOOTPRINT
CONFIG_SPL_MAX_SIZE
CONFIG_SPL_BSS_MAX_SIZE

Will they be replaced by this new mechanism?


-- 
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-05-27 Thread Simon Goldschmidt
Tom Rini  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 
> > >>---
> > >>
> > >>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...

Regrds,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-05-27 Thread Tom Rini
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 
> >>---
> >>
> >>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.
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 v3] spl: add overall SPL size check

2019-05-27 Thread Jean-Jacques Hiblot

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

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


If this flies, it could be done by a python script without the need to 
compile a program


JJ



Regard,
Simon



Changes in v2:
- added missing tools/spl_size_limit.c

  Kconfig    |  8 
  Makefile   |  3 ++-
  common/spl/Kconfig | 36 
  tools/Makefile |  4 
  tools/spl_size_limit.c | 30 ++
  5 files changed, 72 insertions(+), 9 deletions(-)
  create mode 100644 tools/spl_size_limit.c

diff --git a/Kconfig b/Kconfig
index 1221d1af69..5f5c5ccfd6 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 7d910b3682..ed7e12120f 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
@@ -1779,6 +1779,7 @@ checkarmreloc: u-boot
  envtools: scripts_basic $(version_h) $(timestamp_h)
  $(Q)$(MAKE) $(build)=tools/env
  +tools-only: export TOOLS_ONLY=y
  tools-only: scripts_basic $(version_h) $(timestamp_h)
  $(Q)$(MAKE) $(build)=tools
  diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c7cd34449a..6a98536f20 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 e2f572cae1..33e90a8025 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -199,6 +199,10 @@ hostprogs-$(CONFIG_RISCV) += prelink-riscv
  hostprogs-

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

2019-05-26 Thread Tom Rini
On Fri, May 24, 2019 at 10:10:48PM +0200, 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 
> >---
> >
> >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.

OK.  Hopefully at least we can use a Makefile rule to transform the
source code rather than have a copy/paste version of the code.  Thanks
again!

-- 
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 v3] spl: add overall SPL size check

2019-05-24 Thread Simon Goldschmidt

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

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.


Regard,
Simon



Changes in v2:
- added missing tools/spl_size_limit.c

  Kconfig|  8 
  Makefile   |  3 ++-
  common/spl/Kconfig | 36 
  tools/Makefile |  4 
  tools/spl_size_limit.c | 30 ++
  5 files changed, 72 insertions(+), 9 deletions(-)
  create mode 100644 tools/spl_size_limit.c

diff --git a/Kconfig b/Kconfig
index 1221d1af69..5f5c5ccfd6 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 7d910b3682..ed7e12120f 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
@@ -1779,6 +1779,7 @@ checkarmreloc: u-boot
  envtools: scripts_basic $(version_h) $(timestamp_h)
$(Q)$(MAKE) $(build)=tools/env
  
+tools-only: export TOOLS_ONLY=y

  tools-only: scripts_basic $(version_h) $(timestamp_h)
$(Q)$(MAKE) $(build)=tools
  
diff --git a/common/spl/Kconfig b/common/spl/Kconfig

index c7cd34449a..6a98536f20 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 e2f572cae1..33e90a8025 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -199,6 +199,10 @@ hostprogs-$(CONFIG_RISCV) += prelink-riscv
  hostprogs-y += fdtgrep
  fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o
  
+ifneq ($(TOOLS_ONLY),y)

+hostprogs-y += spl_size_limit
+endif
+
  hostprogs-$(CONFIG_MIPS) += mips-relocs
  
  # We build some files with extra pedantic

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

2019-05-24 Thread 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 
---

Changes in v3:
- don't build this new tools for 'make tools-only'

Changes in v2:
- added missing tools/spl_size_limit.c

 Kconfig|  8 
 Makefile   |  3 ++-
 common/spl/Kconfig | 36 
 tools/Makefile |  4 
 tools/spl_size_limit.c | 30 ++
 5 files changed, 72 insertions(+), 9 deletions(-)
 create mode 100644 tools/spl_size_limit.c

diff --git a/Kconfig b/Kconfig
index 1221d1af69..5f5c5ccfd6 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 7d910b3682..ed7e12120f 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
@@ -1779,6 +1779,7 @@ checkarmreloc: u-boot
 envtools: scripts_basic $(version_h) $(timestamp_h)
$(Q)$(MAKE) $(build)=tools/env
 
+tools-only: export TOOLS_ONLY=y
 tools-only: scripts_basic $(version_h) $(timestamp_h)
$(Q)$(MAKE) $(build)=tools
 
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c7cd34449a..6a98536f20 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 e2f572cae1..33e90a8025 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -199,6 +199,10 @@ hostprogs-$(CONFIG_RISCV) += prelink-riscv
 hostprogs-y += fdtgrep
 fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o
 
+ifneq ($(TOOLS_ONLY),y)
+hostprogs-y += spl_size_limit
+endif
+
 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[])
+{
+   in