[PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[Ahmad Fatoum]

For platforms such as the at91, the boot ROM imposes an upper limit
on barebox file size.  Prior to 5a1a5ed253 ("ARM: images: use piggydata"),
BAREBOX_MAX_PBLX_SIZE seems to have been the way to go for limiting
the size of the final barebox binary when using the PBL.
With pblx removed, this variable is of no use, so have the existing
BAREBOX_MAX_IMAGE_SIZE replace its functionality.

Currently BAREBOX_MAX_IMAGE_SIZE is only checked against in the non-PBL
case, so add a check in the PBL case as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/configs/am335x_mlo_defconfig |  2 +-
 common/Kconfig                        | 10 ----------
 images/Makefile                       |  1 +
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/arch/arm/configs/am335x_mlo_defconfig b/arch/arm/configs/am335x_mlo_defconfig
index b58b71a85937..d6909154c443 100644
--- a/arch/arm/configs/am335x_mlo_defconfig
+++ b/arch/arm/configs/am335x_mlo_defconfig
@@ -8,7 +8,7 @@ CONFIG_MACH_PHYTEC_SOM_AM335X=y
 CONFIG_THUMB2_BAREBOX=y
 # CONFIG_MEMINFO is not set
 CONFIG_MMU=y
-CONFIG_BAREBOX_MAX_PBLX_SIZE=0x1b400
+CONFIG_BAREBOX_MAX_IMAGE_SIZE=0x1b400
 CONFIG_MALLOC_SIZE=0x0
 CONFIG_MALLOC_TLSF=y
 CONFIG_RELOCATABLE=y
diff --git a/common/Kconfig b/common/Kconfig
index 0ac83a22ca0c..2dd5842e530f 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -245,16 +245,6 @@ config BAREBOX_MAX_BARE_INIT_SIZE
 	  this will allow your bare_init to fit in SRAM as example
 	  ARCH can overwrite it via ARCH_BAREBOX_MAX_BARE_INIT_SIZE
 
-config BAREBOX_MAX_PBLX_SIZE
-	depends on PBL_MULTI_IMAGES
-	depends on IMAGE_COMPRESSION
-	prompt "Maximum PBLX size"
-	hex
-	default 0xffffffff
-	help
-	  Define the maximum size of the PBLX image.
-	  The pblx is a self extracting barebox binary.
-
 config HAVE_CONFIGURABLE_MEMORY_LAYOUT
 	bool
 
diff --git a/images/Makefile b/images/Makefile
index 4e82dc92ee76..59b81f9b6d8f 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -66,6 +66,7 @@ $(obj)/%.pbl: $(pbl-lds) $(barebox-pbl-common) $(obj)/piggy.o FORCE
 
 $(obj)/%.pblb: $(obj)/%.pbl FORCE
 	$(call if_changed,objcopy_bin,$(*F))
+	$(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
 
 $(obj)/%.s: $(obj)/% FORCE
 	$(call if_changed,disasm)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] images: allow overriding BAREBOX_MAX_IMAGE_SIZE from arch Kconfig

[Ahmad Fatoum]

The size restriction imposed by the ROM code of the target SoC is the
upper limit on how big a barebox binary should get. Thus allow the
architecture Kconfig to define an optional (possibly subarch-specific
ARCH_BAREBOX_MAX_IMAGE_SIZE), as a ceiling for the resulting barebox
binary size.

As we define a min function to calculate this, define a max function
as well for symmetry.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Makefile               |  4 +++-
 arch/arm/pbl/Makefile  |  4 +++-
 arch/mips/pbl/Makefile |  4 +++-
 common/Kconfig         |  3 ++-
 images/Makefile        |  4 +++-
 scripts/Makefile.lib   | 13 +++++++++++++
 6 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index c4030ecbf12e..decba5ee572e 100644
--- a/Makefile
+++ b/Makefile
@@ -719,7 +719,9 @@ OBJCOPYFLAGS_barebox.bin = -O binary
 barebox.bin: barebox FORCE
 	$(call if_changed,objcopy)
 ifndef CONFIG_PBL_IMAGE
-	$(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
+	$(call cmd,check_file_size,$@, \
+	  $(call min,$(CONFIG_ARCH_BAREBOX_MAX_IMAGE_SIZE) \
+	             $(CONFIG_BAREBOX_MAX_IMAGE_SIZE)))
 endif
 
 # By default the uImage load address is 2MB below CONFIG_TEXT_BASE,
diff --git a/arch/arm/pbl/Makefile b/arch/arm/pbl/Makefile
index 5d7e85b373a6..95b6772fc21b 100644
--- a/arch/arm/pbl/Makefile
+++ b/arch/arm/pbl/Makefile
@@ -22,7 +22,9 @@ endif
 
 $(obj)/zbarebox.bin:	$(obj)/zbarebox FORCE
 	$(call if_changed,objcopy)
-	$(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
+	$(call cmd,check_file_size,$@, \
+	  $(call min,$(CONFIG_ARCH_BAREBOX_MAX_IMAGE_SIZE) \
+	             $(CONFIG_BAREBOX_MAX_IMAGE_SIZE)))
 	$(Q)$(kecho) '  Barebox: fix size'
 	$(Q)$(objtree)/scripts/fix_size -i -f $(objtree)/$@ $(FIX_SIZE)
 	$(Q)$(kecho) '  Barebox: $@ is ready'
diff --git a/arch/mips/pbl/Makefile b/arch/mips/pbl/Makefile
index 44ce3d1c922e..8904d269088a 100644
--- a/arch/mips/pbl/Makefile
+++ b/arch/mips/pbl/Makefile
@@ -16,7 +16,9 @@ extra-y       += piggy.gzip piggy.lz4 piggy.lzo piggy.lzma piggy.xzkern piggy.sh
 
 $(obj)/zbarebox.bin:	$(obj)/zbarebox FORCE
 	$(call if_changed,objcopy)
-	$(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
+	$(call cmd,check_file_size,$@, \
+	  $(call min,$(CONFIG_ARCH_BAREBOX_MAX_IMAGE_SIZE) \
+	             $(CONFIG_BAREBOX_MAX_IMAGE_SIZE)))
 	$(Q)$(kecho) '  Barebox: $@ is ready'
 
 $(obj)/zbarebox.S: $(obj)/zbarebox FORCE
diff --git a/common/Kconfig b/common/Kconfig
index 2dd5842e530f..b109328b7887 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -225,7 +225,8 @@ config BAREBOX_MAX_IMAGE_SIZE
 	hex
 	default 0xffffffff
 	help
-	  Define the maximum size of barebox
+	  Define the maximum size of barebox unless ARCH_BAREBOX_MAX_IMAGE_SIZE
+	  is set to a more restrictive value.
 
 config BAREBOX_MAX_PBL_SIZE
 	depends on PBL_IMAGE
diff --git a/images/Makefile b/images/Makefile
index 59b81f9b6d8f..b6ec16467308 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -66,7 +66,9 @@ $(obj)/%.pbl: $(pbl-lds) $(barebox-pbl-common) $(obj)/piggy.o FORCE
 
 $(obj)/%.pblb: $(obj)/%.pbl FORCE
 	$(call if_changed,objcopy_bin,$(*F))
-	$(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
+	$(call cmd,check_file_size,$@, \
+	  $(call min,$(CONFIG_ARCH_BAREBOX_MAX_IMAGE_SIZE) \
+	             $(CONFIG_BAREBOX_MAX_IMAGE_SIZE)))
 
 $(obj)/%.s: $(obj)/% FORCE
 	$(call if_changed,disasm)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 7b8643bf57a7..1c720515ba54 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -498,3 +498,16 @@ quiet_cmd_b64dec = B64DEC  $@
 
 %: %.base64
 	$(call cmd,b64dec)
+
+# Misc helpers
+# ---------------------------------------------------------------------------
+
+define min
+$(shell printf '%u ' $1 | awk 'BEGIN{ RS=" " } NR==1 { min=$$0 } \
+    NR>1 { if ($$0<min) min=$$0 } END{print min}')
+endef
+
+define max
+$(shell printf '%u ' $1 | awk 'BEGIN{ RS=" " } NR==1 { max=$$0 } \
+    NR>1 { if ($$0>max) max=$$0 } END{print max}')
+endef
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[Ahmad Fatoum]

On 19/2/19 13:10, Ahmad Fatoum wrote:
> 5a1a5ed253 ("ARM: images: use piggydata")

Should've added a Fixes: 5a1a5ed253 ("ARM: images: use piggydata") as well.

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[Ahmad Fatoum]

Hello,

On 19/2/19 13:10, Ahmad Fatoum wrote:
> For platforms such as the at91, the boot ROM imposes an upper limit
> on barebox file size.  Prior to 5a1a5ed253 ("ARM: images: use piggydata"),
> BAREBOX_MAX_PBLX_SIZE seems to have been the way to go for limiting
> the size of the final barebox binary when using the PBL.
> With pblx removed, this variable is of no use, so have the existing
> BAREBOX_MAX_IMAGE_SIZE replace its functionality.
> 
> Currently BAREBOX_MAX_IMAGE_SIZE is only checked against in the non-PBL
> case, so add a check in the PBL case as well.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/configs/am335x_mlo_defconfig |  2 +-
>  common/Kconfig                        | 10 ----------
>  images/Makefile                       |  1 +
>  3 files changed, 2 insertions(+), 11 deletions(-)
> 

any news on these two patches?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[Sascha Hauer]

On Mon, Mar 04, 2019 at 03:16:51PM +0100, Ahmad Fatoum wrote:
> Hello,
> 
> On 19/2/19 13:10, Ahmad Fatoum wrote:
> > For platforms such as the at91, the boot ROM imposes an upper limit
> > on barebox file size.  Prior to 5a1a5ed253 ("ARM: images: use piggydata"),
> > BAREBOX_MAX_PBLX_SIZE seems to have been the way to go for limiting
> > the size of the final barebox binary when using the PBL.
> > With pblx removed, this variable is of no use, so have the existing
> > BAREBOX_MAX_IMAGE_SIZE replace its functionality.
> > 
> > Currently BAREBOX_MAX_IMAGE_SIZE is only checked against in the non-PBL
> > case, so add a check in the PBL case as well.
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  arch/arm/configs/am335x_mlo_defconfig |  2 +-
> >  common/Kconfig                        | 10 ----------
> >  images/Makefile                       |  1 +
> >  3 files changed, 2 insertions(+), 11 deletions(-)
> > 
> 
> any news on these two patches?

I applied the first one as I think it does nothing wrong. I am always
hesitating to apply such patches because I think we need to do this
better. For example when doing multi image builds different ROMs with
different size limitations might be involved, so putting the limitation
in Kconfig is wrong.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] images: pbl: verify CONFIG_BAREBOX_MAX_IMAGE_SIZE is not exceeded

[Ahmad Fatoum]

On 3/7/19 8:32 AM, Sascha Hauer wrote:
> On Mon, Mar 04, 2019 at 03:16:51PM +0100, Ahmad Fatoum wrote:
>> Hello,
>>
>> On 19/2/19 13:10, Ahmad Fatoum wrote:
>>> For platforms such as the at91, the boot ROM imposes an upper limit
>>> on barebox file size.  Prior to 5a1a5ed253 ("ARM: images: use piggydata"),
>>> BAREBOX_MAX_PBLX_SIZE seems to have been the way to go for limiting
>>> the size of the final barebox binary when using the PBL.
>>> With pblx removed, this variable is of no use, so have the existing
>>> BAREBOX_MAX_IMAGE_SIZE replace its functionality.
>>>
>>> Currently BAREBOX_MAX_IMAGE_SIZE is only checked against in the non-PBL
>>> case, so add a check in the PBL case as well.
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>  arch/arm/configs/am335x_mlo_defconfig |  2 +-
>>>  common/Kconfig                        | 10 ----------
>>>  images/Makefile                       |  1 +
>>>  3 files changed, 2 insertions(+), 11 deletions(-)
>>>
>>
>> any news on these two patches?
> 
> I applied the first one as I think it does nothing wrong. I am always
> hesitating to apply such patches because I think we need to do this
> better. For example when doing multi image builds different ROMs with
> different size limitations might be involved, so putting the limitation
> in Kconfig is wrong.

Ah, I didn't think about multi-image when I wrote this patch. I am
slowly getting it. Thanks for the feedback!

Specifying it entry-point-wise in images/Makefile.* seems to be the better
place then.

Cheers
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox