[U-Boot] [PATCH 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da840_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250,snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 10 ++++++++-- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- include/configs/da850evm.h | 4 +++- include/configs/exynos5250-dt.h | 3 ++- 9 files changed, 34 insertions(+), 14 deletions(-)
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..df3682b 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL max size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 12288 +#define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
#ifndef CONFIG_SPL_BUILD #define CONFIG_SYS_TEXT_BASE 0x81080000
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- include/configs/da850evm.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds index bc34fb5..d7edeb2 100644 --- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds +++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index 99b4de7..e15c86e 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -399,7 +399,9 @@ #define CONFIG_SPL_LDSCRIPT "board/$(BOARDDIR)/u-boot-spl-da850evm.lds" #define CONFIG_SPL_STACK 0x8001ff00 #define CONFIG_SPL_TEXT_BASE 0x80000000 -#define CONFIG_SPL_MAX_SIZE 32768 +/* SPL max size is 24K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 24576 +#define CONFIG_SPL_BSS_MAX_SIZE (8*1024) #endif
/* Load U-Boot Image From MMC */
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/exynos5250-dt.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds index 4c8baaa..208e626 100644 --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds @@ -26,7 +26,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 496a194..372b0b4 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -141,7 +141,8 @@ /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400 -#define CONFIG_SPL_MAX_SIZE (14 * 1024) +#define CONFIG_SPL_MAX_SIZE (10 * 1024) +#define CONFIG_SPL_BSS_MAX_SIZE (4 * 1024)
#define CONFIG_BOOTCOMMAND "mmc read 40007000 451 2000; bootm 40007000"
The ASSERT() in arch/arm/cpu/u-boot.lds is unneeded as this linker file is not used for SPL builds. Remove it.
The ASSERT() in arch/arm/cpu/u-boot-spl.lds is wrong as it compares image+BSS size to max image size only. Split it in two distinct ASSERT()s, one for image size, one for BSS size.
Finally, update README regarding the (now homogeneous) semantics of the CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE macros.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 10 ++++++++-- arch/arm/cpu/u-boot.lds | 4 ---- 3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/README b/README index a35ef31..c902421 100644 --- a/README +++ b/README @@ -2784,7 +2784,14 @@ FIT uImage format: LDSCRIPT for linking the SPL binary.
CONFIG_SPL_MAX_SIZE - Maximum binary size (text, data and rodata) of the SPL binary. + Maximum size for the image (sum of the text, data, rodata + and linker lists sections) of the SPL executable. + When defined, linker checks that the actual image size does + not exceed it. + The total amount of memory used by the SPL when running is + equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_TEXT_BASE TEXT_BASE for linking the SPL binary. @@ -2797,7 +2804,13 @@ FIT uImage format: Link address for the BSS within the SPL binary.
CONFIG_SPL_BSS_MAX_SIZE - Maximum binary size of the BSS section of the SPL binary. + Maximum size of the BSS section of the SPL executable. + When defined, linker checks that the actual BSS size does + not exceed it. + The total amount of memory used by the SPL when running is + equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_STACK Adress of the start of the stack SPL will use diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \ + "SPL image too big"); +#endif + +#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \ + "SPL image BSS too big"); #endif diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 3a1083d..7bbc4f5 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -101,7 +101,3 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } } - -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); -#endif
Hi Albert,
On Monday, April 8, 2013 9:58:29 PM, Albert ARIBAUD wrote:
The ASSERT() in arch/arm/cpu/u-boot.lds is unneeded as this linker file is not used for SPL builds. Remove it.
The ASSERT() in arch/arm/cpu/u-boot-spl.lds is wrong as it compares image+BSS size to max image size only. Split it in two distinct ASSERT()s, one for image size, one for BSS size.
Finally, update README regarding the (now homogeneous) semantics of the CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE macros.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 10 ++++++++-- arch/arm/cpu/u-boot.lds | 4 ---- 3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/README b/README index a35ef31..c902421 100644 --- a/README +++ b/README @@ -2784,7 +2784,14 @@ FIT uImage format: LDSCRIPT for linking the SPL binary.
CONFIG_SPL_MAX_SIZE
Maximum binary size (text, data and rodata) of the SPL binary.
Maximum size for the image (sum of the text, data, rodataand linker lists sections) of the SPL executable.When defined, linker checks that the actual image size doesnot exceed it.The total amount of memory used by the SPL when running isequal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE ifit exists.Note: image and BSS are disjoint for some targets.CONFIG_SPL_TEXT_BASE TEXT_BASE for linking the SPL binary.
@@ -2797,7 +2804,13 @@ FIT uImage format: Link address for the BSS within the SPL binary.
CONFIG_SPL_BSS_MAX_SIZE
Maximum binary size of the BSS section of the SPL binary.
Maximum size of the BSS section of the SPL executable.When defined, linker checks that the actual BSS size doesnot exceed it.The total amount of memory used by the SPL when running isequal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE ifit exists.Note: image and BSS are disjoint for some targets.CONFIG_SPL_STACK Adress of the start of the stack SPL will use
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
The possible relocation and MMU data is also part of the binary image file, so that would be __bss_start rather than __image_copy_end above, and README should be updated to reflect this.
- "SPL image too big");
+#endif
+#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \
- "SPL image BSS too big");
#endif diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 3a1083d..7bbc4f5 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -101,7 +101,3 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
-#endif
1.7.10.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Benoît
Hi Benoît,
On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
The possible relocation and MMU data is also part of the binary image file, so that would be __bss_start rather than __image_copy_end above, and README should be updated to reflect this.
Actually, mmutable is not used in any SPL; it is used only in targets h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL. I have just confirmed this with a MAKEALL -a arm and a grep on all map files.
This presence of mmutable in u-boot-spl.lds is in fact an overlook that I missed when I created this file from u-boot.lds. I have just finished verifying that removing the mmutable section altogether does not change a single bit to any of the 309 ARM platforms currently built under MAKEALL -a arm.
I'll remove mmutable entries from u-boot-spl.lds in V2.
Amicalement,
Hi Albert,
On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
The possible relocation and MMU data is also part of the binary image file, so that would be __bss_start rather than __image_copy_end above, and README should be updated to reflect this.
Actually, mmutable is not used in any SPL; it is used only in targets h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL. I have just confirmed this with a MAKEALL -a arm and a grep on all map files.
This presence of mmutable in u-boot-spl.lds is in fact an overlook that I missed when I created this file from u-boot.lds. I have just finished verifying that removing the mmutable section altogether does not change a single bit to any of the 309 ARM platforms currently built under MAKEALL -a arm.
I'll remove mmutable entries from u-boot-spl.lds in V2.
OK, that's perfect for MMU data, but what about relocation data?
Best regards, Benoît
Hi Benoît,
On Tue, 9 Apr 2013 16:24:36 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
The possible relocation and MMU data is also part of the binary image file, so that would be __bss_start rather than __image_copy_end above, and README should be updated to reflect this.
Actually, mmutable is not used in any SPL; it is used only in targets h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL. I have just confirmed this with a MAKEALL -a arm and a grep on all map files.
This presence of mmutable in u-boot-spl.lds is in fact an overlook that I missed when I created this file from u-boot.lds. I have just finished verifying that removing the mmutable section altogether does not change a single bit to any of the 309 ARM platforms currently built under MAKEALL -a arm.
I'll remove mmutable entries from u-boot-spl.lds in V2.
OK, that's perfect for MMU data, but what about relocation data?
Relocation data should not exist for SPLs, which do not relocate.
Unfortunately, most tegra and some exynos have start.S code going through the relocation loop even for their SPL; that's cardhu, colibri_t20_iris, dalmore, harmony, medcom-wide, origen, paz00, plutux, seaboard, smdkv310, tec, trimslice, ventana, and whistler.
Tegras do it for their arm720t; Exynos' probably do something similar. I am not going to try and change some start.S so close in time to release. :)
Fortunately, for all the SPLs that fail building if I remove .rel.dyn and .dynsym, these sections are actually empty, i.e. their __end is equal to their __image_copy_end. I have manually verified both reloc section emptiness and equality of _end and __image_copy_end.
Therefore I'll leave the ASSERT() as written in V2, and will provide a separate patch for fixing the Tegra / Exynos unneeded relocation data issue.
Best regards, Benoît
Amicalement,
Hi Albert,
On Tuesday, April 9, 2013 7:43:06 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Tue, 9 Apr 2013 16:24:36 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..89ef9ce 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -88,6 +88,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
The possible relocation and MMU data is also part of the binary image file, so that would be __bss_start rather than __image_copy_end above, and README should be updated to reflect this.
Actually, mmutable is not used in any SPL; it is used only in targets h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL. I have just confirmed this with a MAKEALL -a arm and a grep on all map files.
This presence of mmutable in u-boot-spl.lds is in fact an overlook that I missed when I created this file from u-boot.lds. I have just finished verifying that removing the mmutable section altogether does not change a single bit to any of the 309 ARM platforms currently built under MAKEALL -a arm.
I'll remove mmutable entries from u-boot-spl.lds in V2.
OK, that's perfect for MMU data, but what about relocation data?
Relocation data should not exist for SPLs, which do not relocate.
Unfortunately, most tegra and some exynos have start.S code going through the relocation loop even for their SPL; that's cardhu, colibri_t20_iris, dalmore, harmony, medcom-wide, origen, paz00, plutux, seaboard, smdkv310, tec, trimslice, ventana, and whistler.
Tegras do it for their arm720t; Exynos' probably do something similar. I am not going to try and change some start.S so close in time to release. :)
Fortunately, for all the SPLs that fail building if I remove .rel.dyn and .dynsym, these sections are actually empty, i.e. their __end is equal to their __image_copy_end. I have manually verified both reloc section emptiness and equality of _end and __image_copy_end.
Therefore I'll leave the ASSERT() as written in V2, and will provide a separate patch for fixing the Tegra / Exynos unneeded relocation data issue.
That's perfect.
Best regards, Benoît
On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
LENGTH = CONFIG_SPL_MAX_SIZE }
LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..df3682b 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL max size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 12288 +#define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
This is wrong, you've just increased the overall limit to 16K. I know there's a reason that current limit is so exact, Heiko? And also, this shows the conceptual problem I have (and 2/2 has the same, along with tegra). The important limit is the combined size. It doesn't matter if it's 11K text/data/rodata and 1K BSS, or 8+4. When using custom linker scripts, we avoid this and can just comment overall (which would need adding here) that we only care about the combined size. But then tegra would be wrong since it uses the generic arm spl linker script?
Hello Tom,
Am 08.04.2013 22:43, schrieb Tom Rini:
On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
LENGTH = CONFIG_SPL_MAX_SIZE }
LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..df3682b 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL max size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 12288 +#define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
This is wrong, you've just increased the overall limit to 16K. I know there's a reason that current limit is so exact, Heiko? And also, this
The cam_enc_4xx use only 12k for the SPL code. This is defined in the UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:
http://www.ti.com/lit/gpn/tms320dm368
I have no access anymore to this HW to do some tests :-( so I looked into the hexdump of the current u-boot code with your patch applied, and the code on the interesting borders (0x0, 0x800 and 0x3800) looks good to me ...
shows the conceptual problem I have (and 2/2 has the same, along with tegra). The important limit is the combined size. It doesn't matter if it's 11K text/data/rodata and 1K BSS, or 8+4. When using custom linker scripts, we avoid this and can just comment overall (which would need adding here) that we only care about the combined size. But then tegra would be wrong since it uses the generic arm spl linker script?
bye, Heiko
Hi Heiko,
On Tue, 09 Apr 2013 08:50:26 +0200, Heiko Schocher hs@denx.de wrote:
Hello Tom,
Am 08.04.2013 22:43, schrieb Tom Rini:
On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
LENGTH = CONFIG_SPL_MAX_SIZE }
LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..df3682b 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL max size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 12288 +#define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
This is wrong, you've just increased the overall limit to 16K. I know there's a reason that current limit is so exact, Heiko? And also, this
The cam_enc_4xx use only 12k for the SPL code. This is defined in the UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:
http://www.ti.com/lit/gpn/tms320dm368
I have no access anymore to this HW to do some tests :-( so I looked into the hexdump of the current u-boot code with your patch applied, and the code on the interesting borders (0x0, 0x800 and 0x3800) looks good to me ...
shows the conceptual problem I have (and 2/2 has the same, along with tegra). The important limit is the combined size. It doesn't matter if it's 11K text/data/rodata and 1K BSS, or 8+4. When using custom linker scripts, we avoid this and can just comment overall (which would need adding here) that we only care about the combined size. But then tegra would be wrong since it uses the generic arm spl linker script?
Thanks Heiko.
I'd read about the SoC IRAM, and had chosen 16K indeed arbitrarily but taking care not to use most of it -- half felt like safe enough. However, I'd missed the UBL thing, thanks for pointing this out. So either I keep 12K, split for instance 10K and 2K (5 pages and 1 page), or I reaise the number of pages in board/ait/cam_enc_4xx/ublimage.cfg, correct?
Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
text data bss dec hex filename 439526 13148 311092 763766 ba776 ./u-boot 9073 840 500 10413 28ad ./spl/u-boot-spl
And the map file gives __start = 0x20, __bss_start = 0x26e0, and __bss_end = __image_copy_end = _end = 0x28d4, which makes the size of the non-BSS part of the image equal to 9952 bytes (thus below 10K) and the BSS part size is 500 bytes, below 2K.
So, it seems I could just replace
#define CONFIG_SPL_MAX_SIZE 12288 #define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
with
#define CONFIG_SPL_MAX_SIZE 10240 #define CONFIG_SPL_BSS_MAX_SIZE (2*1024)
and keep the UBL cfg file untouched -- any future size issue with image or BSS size would imply changing these values and uptating the UBL cfg file.
Would that be ok?
bye, Heiko
Amicalement,
Hello Albert,
On 09.04.2013 11:08, Albert ARIBAUD wrote:
Hi Heiko,
On Tue, 09 Apr 2013 08:50:26 +0200, Heiko Schocher hs@denx.de wrote:
Hello Tom,
Am 08.04.2013 22:43, schrieb Tom Rini:
On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split max size between image and BSS based on sizes reported for current build.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
LENGTH = CONFIG_SPL_MAX_SIZE }
LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..df3682b 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL max size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 12288 +#define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
This is wrong, you've just increased the overall limit to 16K. I know there's a reason that current limit is so exact, Heiko? And also, this
The cam_enc_4xx use only 12k for the SPL code. This is defined in the UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:
http://www.ti.com/lit/gpn/tms320dm368
I have no access anymore to this HW to do some tests :-( so I looked into the hexdump of the current u-boot code with your patch applied, and the code on the interesting borders (0x0, 0x800 and 0x3800) looks good to me ...
shows the conceptual problem I have (and 2/2 has the same, along with tegra). The important limit is the combined size. It doesn't matter if it's 11K text/data/rodata and 1K BSS, or 8+4. When using custom linker scripts, we avoid this and can just comment overall (which would need adding here) that we only care about the combined size. But then tegra would be wrong since it uses the generic arm spl linker script?
Thanks Heiko.
I'd read about the SoC IRAM, and had chosen 16K indeed arbitrarily but taking care not to use most of it -- half felt like safe enough. However, I'd missed the UBL thing, thanks for pointing this out. So either I keep 12K, split for instance 10K and 2K (5 pages and 1 page), or I reaise the number of pages in board/ait/cam_enc_4xx/ublimage.cfg, correct?
Yes, but I would prefer not to change the number of pages.
Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
text data bss dec hex filename 439526 13148 311092 763766 ba776 ./u-boot 9073 840 500 10413 28ad ./spl/u-boot-spl
And the map file gives __start = 0x20, __bss_start = 0x26e0, and __bss_end = __image_copy_end = _end = 0x28d4, which makes the size of the non-BSS part of the image equal to 9952 bytes (thus below 10K) and the BSS part size is 500 bytes, below 2K.
So, it seems I could just replace
#define CONFIG_SPL_MAX_SIZE 12288 #define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
with
#define CONFIG_SPL_MAX_SIZE 10240 #define CONFIG_SPL_BSS_MAX_SIZE (2*1024)
and keep the UBL cfg file untouched -- any future size issue with image or BSS size would imply changing these values and uptating the UBL cfg file.
Would that be ok?
Yes, that seems good to me, but I could not test it ...
bye, Heiko
Hi Heiko,
On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher hs@denx.de wrote:
Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
text data bss dec hex filename 439526 13148 311092 763766 ba776 ./u-boot 9073 840 500 10413 28ad ./spl/u-boot-spl
And the map file gives __start = 0x20, __bss_start = 0x26e0, and __bss_end = __image_copy_end = _end = 0x28d4, which makes the size of the non-BSS part of the image equal to 9952 bytes (thus below 10K) and the BSS part size is 500 bytes, below 2K.
So, it seems I could just replace
#define CONFIG_SPL_MAX_SIZE 12288 #define CONFIG_SPL_BSS_MAX_SIZE (4*1024)
with
#define CONFIG_SPL_MAX_SIZE 10240 #define CONFIG_SPL_BSS_MAX_SIZE (2*1024)
and keep the UBL cfg file untouched -- any future size issue with image or BSS size would imply changing these values and uptating the UBL cfg file.
Would that be ok?
Yes, that seems good to me, but I could not test it ...
I can do a diff of the binaries with and without the patch -- normally, they should be the same except for the U-Boot version identification. Would that do?
bye, Heiko
Amicalement,
Hello Albert,
on 09.04.2013 14:42, wrote Albert ARIBAUD:
Hi Heiko,
On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher hs@denx.de wrote:
Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
text data bss dec hex filename 439526 13148 311092 763766 ba776 ./u-boot 9073 840 500 10413 28ad ./spl/u-boot-spl
[...]
Would that be ok?
Yes, that seems good to me, but I could not test it ...
I can do a diff of the binaries with and without the patch -- normally, they should be the same except for the U-Boot version identification. Would that do?
Yes. I did this too, and see only a diff for the version string. (But not forget to commit your change first, else the "-dirty" in the version string will insert an offset ;-)
bye, Heiko
Hi Heiko,
On Tue, 09 Apr 2013 15:17:02 +0200, Heiko Schocher hs@denx.de wrote:
Hello Albert,
on 09.04.2013 14:42, wrote Albert ARIBAUD:
Hi Heiko,
On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher hs@denx.de wrote:
Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
text data bss dec hex filename 439526 13148 311092 763766 ba776 ./u-boot 9073 840 500 10413 28ad ./spl/u-boot-spl
[...]
Would that be ok?
Yes, that seems good to me, but I could not test it ...
I can do a diff of the binaries with and without the patch -- normally, they should be the same except for the U-Boot version identification. Would that do?
Yes. I did this too, and see only a diff for the version string.
Thanks -- I've staged the 10K+2K change for V2.
(But not forget to commit your change first, else the "-dirty" in the version string will insert an offset ;-)
I actually keep a 'fake_build' branch with a single nifty commit on it that makes almost all version information sources constant (only mkimage resists as it collects the current date and time programmatically, but that's ok, I don't aim for 100% faking.
So, whenever I need to do bulk comparisons, I add this commit above the two branches to be compared, and two batch builds later, I can compare the .bins, even if different commit ID and/or local changes are involved. :)
bye, Heiko
Amicalement,
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v2: - brought back total size to 12K - fixed commit summary typoes - fixed spacing in commit summary - removed mmutable in SPL linker file
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 19 ++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- include/configs/da850evm.h | 4 +++- include/configs/exynos5250-dt.h | 3 ++- 9 files changed, 34 insertions(+), 23 deletions(-)
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 12K max size between 10K image (text,rodata,data) and 2K BSS based on sizes reported for current build:
text data bss 9073 840 500
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v2: - brought back total size to 12K
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index dd9d52d..25625dc 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..b73b72c 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL total size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 10240 +#define CONFIG_SPL_BSS_MAX_SIZE (2*1024)
#ifndef CONFIG_SPL_BUILD #define CONFIG_SYS_TEXT_BASE 0x81080000
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 32K max size between 24K image (text,rodata,data) and 8K BSS based on sizes reported for current build:
text data bss 15676 1316 108
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v2: - fixed commit summary typoes
board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- include/configs/da850evm.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds index bc34fb5..d7edeb2 100644 --- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds +++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index 99b4de7..e15c86e 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -399,7 +399,9 @@ #define CONFIG_SPL_LDSCRIPT "board/$(BOARDDIR)/u-boot-spl-da850evm.lds" #define CONFIG_SPL_STACK 0x8001ff00 #define CONFIG_SPL_TEXT_BASE 0x80000000 -#define CONFIG_SPL_MAX_SIZE 32768 +/* SPL max size is 24K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 24576 +#define CONFIG_SPL_BSS_MAX_SIZE (8*1024) #endif
/* Load U-Boot Image From MMC */
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 14K max size between 10K image (text,rodata,data) and 4K BSS based on sizes reported for current build:
text data bss 4136 904 0
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v2: - fixed spacing in commit summary
board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/exynos5250-dt.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds index 4c8baaa..208e626 100644 --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds @@ -26,7 +26,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 496a194..372b0b4 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -141,7 +141,8 @@ /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400 -#define CONFIG_SPL_MAX_SIZE (14 * 1024) +#define CONFIG_SPL_MAX_SIZE (10 * 1024) +#define CONFIG_SPL_BSS_MAX_SIZE (4 * 1024)
#define CONFIG_BOOTCOMMAND "mmc read 40007000 451 2000; bootm 40007000"
Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds as this file is never used for SPL builds.
Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds to separately test image (text,data,rodata...) size and BSS size each against its own max.
Also, output section mmutable is not used in SPL builds. Remove it.
Finally, update README regarding the (now homogeneous) semantics of the CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE macros.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- Changes in v2: - removed mmutable in SPL linker file
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 19 ++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- 3 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/README b/README index 5c5cd18..f31ded0 100644 --- a/README +++ b/README @@ -2776,7 +2776,14 @@ FIT uImage format: LDSCRIPT for linking the SPL binary.
CONFIG_SPL_MAX_SIZE - Maximum binary size (text, data and rodata) of the SPL binary. + Maximum size for the image (sum of the text, data, rodata + and linker lists sections) of the SPL executable. + When defined, linker checks that the actual image size does + not exceed it. + The total amount of memory used by the SPL when running is + equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_TEXT_BASE TEXT_BASE for linking the SPL binary. @@ -2789,7 +2796,13 @@ FIT uImage format: Link address for the BSS within the SPL binary.
CONFIG_SPL_BSS_MAX_SIZE - Maximum binary size of the BSS section of the SPL binary. + Maximum size of the BSS section of the SPL executable. + When defined, linker checks that the actual BSS size does + not exceed it. + The total amount of memory used by the SPL when running is + equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_STACK Adress of the start of the stack SPL will use diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..2e86de4 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -65,15 +65,6 @@ SECTIONS
_end = .;
- /* - * Deprecated: this MMU section is used by pxa at present but - * should not be used by new boards/CPUs. - */ - . = ALIGN(4096); - .mmutable : { - *(.mmutable) - } - .bss __rel_dyn_start (OVERLAY) : { __bss_start = .; *(.bss*) @@ -88,6 +79,12 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \ + "SPL image too big"); +#endif + +#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \ + "SPL image BSS too big"); #endif diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 3a1083d..7bbc4f5 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -101,7 +101,3 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } } - -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); -#endif
On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds as this file is never used for SPL builds.
Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds to separately test image (text,data,rodata...) size and BSS size each against its own max.
Also, output section mmutable is not used in SPL builds. Remove it.
Finally, update README regarding the (now homogeneous) semantics of the CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE macros.
This still seems to have separate defines for SPL text/data/rodata size and BSS size. If I want instead to limit the total text/data/rodata/bss size, but place no specific limit on the bss size individually, can I not do that?
Hi Stephen,
On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds as this file is never used for SPL builds.
Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds to separately test image (text,data,rodata...) size and BSS size each against its own max.
Also, output section mmutable is not used in SPL builds. Remove it.
Finally, update README regarding the (now homogeneous) semantics of the CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE macros.
This still seems to have separate defines for SPL text/data/rodata size and BSS size. If I want instead to limit the total text/data/rodata/bss size, but place no specific limit on the bss size individually, can I not do that?
This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant meaning -- one of the issues which prompted this patch series is that in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime excluded, and this inconsistency had to be resolved. As in the rest of U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the semantics that was decided.
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
- if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
- if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
I guess this will be considered useless complication -- after all, once you have artificially partitioned your SPL space into image+BSS -- and you know from the build command how much should be allotted to each of them -- the worst that can happen is that a later build fails with an explicit error message forcing you to look at current image and BSS size and adjust one or both of the max values accordingly.
Amicalement,
On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
...
This still seems to have separate defines for SPL text/data/rodata size and BSS size. If I want instead to limit the total text/data/rodata/bss size, but place no specific limit on the bss size individually, can I not do that?
This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant meaning -- one of the issues which prompted this patch series is that in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime excluded, and this inconsistency had to be resolved. As in the rest of U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the semantics that was decided.
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
Why not either:
a) define CONFIG_SPL_MAX_SIZE to include the BSS size if CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.
or:
b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined) always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined) always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if defined) always limits bss size.
Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme. Other boards would presumably define other combinations of those.
Hi Stephen,
On Wed, 10 Apr 2013 17:09:45 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
...
This still seems to have separate defines for SPL text/data/rodata size and BSS size. If I want instead to limit the total text/data/rodata/bss size, but place no specific limit on the bss size individually, can I not do that?
This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant meaning -- one of the issues which prompted this patch series is that in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime excluded, and this inconsistency had to be resolved. As in the rest of U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the semantics that was decided.
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
Why not either:
a) define CONFIG_SPL_MAX_SIZE to include the BSS size if CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.
That was in my original proposals, and it was spoken against: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073 http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094
or:
b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined) always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined) always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if defined) always limits bss size.
Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme. Other boards would presumably define other combinations of those.
That is a possibility, with the minor nitpick that we could possibly end up having conflicting values for the three symbols, whereas with the CONFIG_SPL_BSS_START_ADDR solution, there can be no value conflict.
Amicalement,
Hi Stephen,
On Wed, 10 Apr 2013 17:09:45 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
...
This still seems to have separate defines for SPL text/data/rodata size and BSS size. If I want instead to limit the total text/data/rodata/bss size, but place no specific limit on the bss size individually, can I not do that?
This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant meaning -- one of the issues which prompted this patch series is that in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime excluded, and this inconsistency had to be resolved. As in the rest of U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the semantics that was decided.
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
Why not either:
a) define CONFIG_SPL_MAX_SIZE to include the BSS size if CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.
or:
b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined) always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined) always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if defined) always limits bss size.
Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme. Other boards would presumably define other combinations of those.
Tom R on IRC showed preference for CONFIG_SPL_MAX_FOOTPRINT -- I'll send out a V3 with this solution.
Amicalement,
On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
I guess this will be considered useless complication -- after all, once you have artificially partitioned your SPL space into image+BSS -- and you know from the build command how much should be allotted to each of them -- the worst that can happen is that a later build fails with an explicit error message forcing you to look at current image and BSS size and adjust one or both of the max values accordingly.
P.S. In any case, the proposal above will go in, if at all, as a separate patch; the current patch series is going in right now as it is.
Amicalement,
On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
I guess this will be considered useless complication -- after all, once you have artificially partitioned your SPL space into image+BSS -- and you know from the build command how much should be allotted to each of them -- the worst that can happen is that a later build fails with an explicit error message forcing you to look at current image and BSS size and adjust one or both of the max values accordingly.
P.S. In any case, the proposal above will go in, if at all, as a separate patch; the current patch series is going in right now as it is.
I wonder what the point of code-review is if you're just going to ignore it.
What's really odd here is that by my reading of the relevant threads, TomR already pointed out this exact issue earlier on, and you had agreed that you'd resolve it in a way that didn't have this issue, yet the patch has this issue???
Hi Stephen,
On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
I guess this will be considered useless complication -- after all, once you have artificially partitioned your SPL space into image+BSS -- and you know from the build command how much should be allotted to each of them -- the worst that can happen is that a later build fails with an explicit error message forcing you to look at current image and BSS size and adjust one or both of the max values accordingly.
P.S. In any case, the proposal above will go in, if at all, as a separate patch; the current patch series is going in right now as it is.
I wonder what the point of code-review is if you're just going to ignore it.
Can we please avoid this kind of talk? It has no argumentative value and can only lead to conflicts. Your account below is sufficient to convey your argumentation without any of the potential ill-effects of the above.
What's really odd here is that by my reading of the relevant threads, TomR already pointed out this exact issue earlier on, and you had agreed that you'd resolve it in a way that didn't have this issue, yet the patch has this issue???
Then our reading of the thread do not agree. Here is mine:
- in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158046, Tom clearly asks for separate text+data+rodata size on one hand and BSS size on the other hand (his #2 case, which he wants applied uniformally and a solution found for Tegra.
- in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073 I replied to Tom with a proposal in two parts, the first implementing his #2 case strictly, the second implementing case #1 at the cost of some minor added complexity and of muddying the symbol's semantics; I suggested that if Tom really did not want the second part of my proposal, it could be dropped and only the first part implemented.
- in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094, Tom asked that I keep part 1 and drop part 2 -- which I did.
Additionally, I did ask Tom on IRC if V2 was ok with him, and had his agreement.
Amicalement,
On Thu, Apr 11, 2013 at 04:32:53PM +0200, Albert ARIBAUD wrote:
Hi Stephen,
On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What we could do, though, is subdivide testing based on the existence or non-existence of CONFIG_SPL_BSS_START_ADDR:
if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and BSS are disjoint and we test each one against its max size, as this patch series does;
if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image and BSS are contiguous and we test the whole of SPL against the sum of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
I guess this will be considered useless complication -- after all, once you have artificially partitioned your SPL space into image+BSS -- and you know from the build command how much should be allotted to each of them -- the worst that can happen is that a later build fails with an explicit error message forcing you to look at current image and BSS size and adjust one or both of the max values accordingly.
P.S. In any case, the proposal above will go in, if at all, as a separate patch; the current patch series is going in right now as it is.
I wonder what the point of code-review is if you're just going to ignore it.
Can we please avoid this kind of talk? It has no argumentative value and can only lead to conflicts. Your account below is sufficient to convey your argumentation without any of the potential ill-effects of the above.
What's really odd here is that by my reading of the relevant threads, TomR already pointed out this exact issue earlier on, and you had agreed that you'd resolve it in a way that didn't have this issue, yet the patch has this issue???
Then our reading of the thread do not agree. Here is mine:
in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158046, Tom clearly asks for separate text+data+rodata size on one hand and BSS size on the other hand (his #2 case, which he wants applied uniformally and a solution found for Tegra.
in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073 I replied to Tom with a proposal in two parts, the first implementing his #2 case strictly, the second implementing case #1 at the cost of some minor added complexity and of muddying the symbol's semantics; I suggested that if Tom really did not want the second part of my proposal, it could be dropped and only the first part implemented.
in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094, Tom asked that I keep part 1 and drop part 2 -- which I did.
Additionally, I did ask Tom on IRC if V2 was ok with him, and had his agreement.
To be clear, I think the fake partitioning scheme we use when the case is "we care about text/data/rodata/bss fitting in $X" is not optimal, but it covers all of the cases without adding more complexity / another way to achieve the same ends. If it turns out that platforms end up needing to re-tweak this value we can come back and say it's time to add a different mechanism for this hard-limit case.
On 04/11/2013 10:08 AM, Tom Rini wrote: ...
To be clear, I think the fake partitioning scheme we use when the case is "we care about text/data/rodata/bss fitting in $X" is not optimal, but it covers all of the cases without adding more complexity / another way to achieve the same ends.
But it does add more complexity; somebody has to pick, and potentially keep adjusting, the split between text/rodata/data and bss, even though they don't care about that split, but it's an implementation wart. It's also trivial to fix it properly as I described.
But, if it absolutely has to be that way, then OK. However, I would ask that at least the Tegra board configuration files be set up so that the previous overall restriction (bss doesn't overlap the main U-Boot) be left in tact, so we don't have to debug that problem again.
On 10/04/13 08:14, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 14K max size between 10K image (text,rodata,data) and 4K BSS based on sizes reported for current build:
text data bss 4136 904 0
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
Changes in v2:
- fixed spacing in commit summary
board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/exynos5250-dt.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds index 4c8baaa..208e626 100644 --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds @@ -26,7 +26,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
LENGTH = CONFIG_SPL_MAX_SIZE }
LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 496a194..372b0b4 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -141,7 +141,8 @@ /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400 -#define CONFIG_SPL_MAX_SIZE (14 * 1024) +#define CONFIG_SPL_MAX_SIZE (10 * 1024) +#define CONFIG_SPL_BSS_MAX_SIZE (4 * 1024)
#define CONFIG_BOOTCOMMAND "mmc read 40007000 451 2000; bootm 40007000"
Acked-by: Minkyu Kang mk7.kang@samsung.com
Thanks, Minkyu Kang.
Hello Albert,
On 10.04.2013 01:14, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 12K max size between 10K image (text,rodata,data) and 2K BSS based on sizes reported for current build:
text data bss 9073 840 500
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
Thanks!
Acked-by: Heiko Schocher hs@denx.de
bye, Heiko
On Wed, 10 Apr 2013 01:14:51 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v2:
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
- removed mmutable in SPL linker file
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 19 ++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- include/configs/da850evm.h | 4 +++- include/configs/exynos5250-dt.h | 3 ++- 9 files changed, 34 insertions(+), 23 deletions(-)
Applied to u-boot-arm/master.
Amicalement,
On Thu, 11 Apr 2013 01:10:17 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
On Wed, 10 Apr 2013 01:14:51 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v2:
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
- removed mmutable in SPL linker file
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
README | 17 +++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 19 ++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- include/configs/da850evm.h | 4 +++- include/configs/exynos5250-dt.h | 3 ++- 9 files changed, 34 insertions(+), 23 deletions(-)
Applied to u-boot-arm/master.
... and then rolled back.
Amicalement,
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v3: - introduced CONFIG_SPL_MAX_FOOTPRINT - fixed typo in BSS size test
Changes in v2: - brought back total size to 12K - fixed commit summary typoes - fixed spacing in commit summary - removed mmutable in SPL linker file
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
README | 26 ++++++++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 ++++++++++++---------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- include/configs/da850evm.h | 4 +++- include/configs/exynos5250-dt.h | 3 ++- include/configs/tegra-common.h | 2 +- 10 files changed, 49 insertions(+), 24 deletions(-)
On Fri, Apr 12, 2013 at 01:55:39PM +0200, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test
Changes in v2:
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
- removed mmutable in SPL linker file
Albert ARIBAUD (4): cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics ARM: fix CONFIG_SPL_MAX_SIZE semantics
The problem is that cam_enc_4xx, da850* and quite likely smdk5250 all want the new CONFIG_SPL_MAX_FOOTPRINT and not an arbitrary split between BSS and everything else.
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v4: - reordered patches so that FOOTPRINT is introduced first, then individual boards are switched to using it - rewrote README entries - converted to CONFIG_SPL_MAX_FOOTPRINT - limited SPL size to exactly 6 2K pages - converted to CONFIG_SPL_MAX_FOOTPRINT - converted to CONFIG_SPL_MAX_FOOTPRINT
Changes in v3: - introduced CONFIG_SPL_MAX_FOOTPRINT - fixed typo in BSS size test
Changes in v2: - removed mmutable in SPL linker file - brought back total size to 12K - fixed commit summary typoes - fixed spacing in commit summary
Albert ARIBAUD (4): ARM: fix CONFIG_SPL_MAX_SIZE semantics cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT
README | 18 ++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 +++++++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 2 +- include/configs/da850evm.h | 2 +- include/configs/exynos5250-dt.h | 2 +- include/configs/tegra-common.h | 2 +- 10 files changed, 36 insertions(+), 24 deletions(-)
Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds as this file is never used for SPL builds.
Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds to separately test image (text,data,rodata...) size, BSS size, and full footprint each against its own max, and make Tegra boards check full footprint.
Also, output section mmutable is not used in SPL builds. Remove it.
Finally, update README regarding the (now homogeneous) semantics of CONFIG_SPL_[BSS_]MAX_SIZE and add the new CONFIG_SPL_MAX_FOOTPRINT macro.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- Changes in v4: - rewrote README entries
Changes in v3: - introduced CONFIG_SPL_MAX_FOOTPRINT - fixed typo in BSS size test
Changes in v2: - removed mmutable in SPL linker file
README | 18 ++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 +++++++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- include/configs/tegra-common.h | 2 +- 4 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/README b/README index 6272853..f544c88 100644 --- a/README +++ b/README @@ -2775,8 +2775,18 @@ FIT uImage format: CONFIG_SPL_LDSCRIPT LDSCRIPT for linking the SPL binary.
+ CONFIG_SPL_MAX_FOOTPRINT + Maximum size in memory allocated to the SPL, BSS included. + When defined, the linker checks that the actual memory + used by SPL from _start to __bss_end does not exceed it. + CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not + be both defined at the same time. + CONFIG_SPL_MAX_SIZE - Maximum binary size (text, data and rodata) of the SPL binary. + Maximum size of the SPL image (text, data, rodata, and + linker lists sections), BSS excluded. + When defined, the linker checks that the actual size does + not exceed it.
CONFIG_SPL_TEXT_BASE TEXT_BASE for linking the SPL binary. @@ -2789,7 +2799,11 @@ FIT uImage format: Link address for the BSS within the SPL binary.
CONFIG_SPL_BSS_MAX_SIZE - Maximum binary size of the BSS section of the SPL binary. + Maximum size in memory allocated to the SPL BSS. + When defined, the linker checks that the actual memory used + by SPL from __bss_start to __bss_end does not exceed it. + CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not + be both defined at the same time.
CONFIG_SPL_STACK Adress of the start of the stack SPL will use diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..1408f03 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -65,15 +65,6 @@ SECTIONS
_end = .;
- /* - * Deprecated: this MMU section is used by pxa at present but - * should not be used by new boards/CPUs. - */ - . = ALIGN(4096); - .mmutable : { - *(.mmutable) - } - .bss __rel_dyn_start (OVERLAY) : { __bss_start = .; *(.bss*) @@ -88,6 +79,17 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \ + "SPL image too big"); +#endif + +#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \ + "SPL image BSS too big"); +#endif + +#if defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \ + "SPL image plus BSS too big"); #endif diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 3a1083d..7bbc4f5 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -101,7 +101,3 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } } - -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); -#endif diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index 036ded0..e9241b8 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -158,7 +158,7 @@ #define CONFIG_SPL_RAM_DEVICE #define CONFIG_SPL_BOARD_INIT #define CONFIG_SPL_NAND_SIMPLE -#define CONFIG_SPL_MAX_SIZE (CONFIG_SYS_TEXT_BASE - \ +#define CONFIG_SPL_MAX_FOOTPRINT (CONFIG_SYS_TEXT_BASE - \ CONFIG_SPL_TEXT_BASE) #define CONFIG_SYS_SPL_MALLOC_SIZE 0x00010000
This target wants to check full SPL size, BSS included. Remove CONFIG_SPL_MAX_SIZE definition and instead define CONFIG_SPL_MAX_FOOTPRINT.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v4: - converted to CONFIG_SPL_MAX_FOOTPRINT - limited SPL size to exactly 6 2K pages
Changes in v3: None Changes in v2: - brought back total size to 12K
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index be1027d..1daa1b3 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..b27551d 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,7 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +#define CONFIG_SPL_MAX_FOOTPRINT 12288
#ifndef CONFIG_SPL_BUILD #define CONFIG_SYS_TEXT_BASE 0x81080000
This target wants to check full SPL size, BSS included. Remove CONFIG_SPL_MAX_SIZE definition and instead define CONFIG_SPL_MAX_FOOTPRINT.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v4: - converted to CONFIG_SPL_MAX_FOOTPRINT
Changes in v3: None Changes in v2: - fixed commit summary typoes
board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- include/configs/da850evm.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds index 2ae5a2c..b1b8701 100644 --- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds +++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index 99b4de7..583568d 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -399,7 +399,7 @@ #define CONFIG_SPL_LDSCRIPT "board/$(BOARDDIR)/u-boot-spl-da850evm.lds" #define CONFIG_SPL_STACK 0x8001ff00 #define CONFIG_SPL_TEXT_BASE 0x80000000 -#define CONFIG_SPL_MAX_SIZE 32768 +#define CONFIG_SPL_MAX_FOOTPRINT 32768 #endif
/* Load U-Boot Image From MMC */
This target wants to check full SPL size, BSS included. Remove CONFIG_SPL_MAX_SIZE definition and instead define CONFIG_SPL_MAX_FOOTPRINT.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v4: - converted to CONFIG_SPL_MAX_FOOTPRINT
Changes in v3: None Changes in v2: - fixed spacing in commit summary
board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/exynos5250-dt.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds index 7df0a1d..c0a7602 100644 --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds @@ -26,7 +26,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 496a194..3aed696 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -141,7 +141,7 @@ /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400 -#define CONFIG_SPL_MAX_SIZE (14 * 1024) +#define CONFIG_SPL_MAX_FOOTPRINT (14 * 1024)
#define CONFIG_BOOTCOMMAND "mmc read 40007000 451 2000; bootm 40007000"
On Fri, Apr 12, 2013 at 05:14:30PM +0200, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_FOOTPRINTMaximum size in memory allocated to the SPL, BSS included.When defined, the linker checks that the actual memoryused by SPL from _start to __bss_end does not exceed it.CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not
must not, not should not. And CONFIG_SPL_... for both. You can just fix that up when commiting, assuming no other comments require change :)
Hi Tom,
On Fri, 12 Apr 2013 11:30:32 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 12, 2013 at 05:14:30PM +0200, Albert ARIBAUD wrote:
CONFIG_SPL_MAX_FOOTPRINTMaximum size in memory allocated to the SPL, BSS included.When defined, the linker checks that the actual memoryused by SPL from _start to __bss_end does not exceed it.CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should notmust not, not should not. And CONFIG_SPL_... for both. You can just fix that up when commiting, assuming no other comments require change :)
Will do; I'll just wait for 24 hours, to make sure no other change is requested.
Amicalement,
On Fri, 12 Apr 2013 17:14:29 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v4:
- reordered patches so that FOOTPRINT is introduced first, then individual boards are switched to using it
- rewrote README entries
- converted to CONFIG_SPL_MAX_FOOTPRINT
- limited SPL size to exactly 6 2K pages
- converted to CONFIG_SPL_MAX_FOOTPRINT
- converted to CONFIG_SPL_MAX_FOOTPRINT
Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test
Changes in v2:
- removed mmutable in SPL linker file
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
Albert ARIBAUD (4): ARM: fix CONFIG_SPL_MAX_SIZE semantics cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT
README | 18 ++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 +++++++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 2 +- include/configs/da850evm.h | 2 +- include/configs/exynos5250-dt.h | 2 +- include/configs/tegra-common.h | 2 +- 10 files changed, 36 insertions(+), 24 deletions(-)
Applied to u-boot-arm/master, with README corrected on the fly as suggested by Tom.
Amicalement,
Hi Albert,
On Sunday, April 14, 2013 4:10:37 PM, Albert ARIBAUD wrote:
On Fri, 12 Apr 2013 17:14:29 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant semantics across all of U-boot. This patch series aims at fixing this by splitting the maximum size into separate image (code + data + rodata + linker list) size on the one hand, and BSS size on the other hand.
Changes in v4:
- reordered patches so that FOOTPRINT is introduced first, then individual boards are switched to using it
- rewrote README entries
- converted to CONFIG_SPL_MAX_FOOTPRINT
- limited SPL size to exactly 6 2K pages
- converted to CONFIG_SPL_MAX_FOOTPRINT
- converted to CONFIG_SPL_MAX_FOOTPRINT
Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test
Changes in v2:
- removed mmutable in SPL linker file
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
Albert ARIBAUD (4): ARM: fix CONFIG_SPL_MAX_SIZE semantics cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT
README | 18 ++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 +++++++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 2 +- include/configs/da850evm.h | 2 +- include/configs/exynos5250-dt.h | 2 +- include/configs/tegra-common.h | 2 +- 10 files changed, 36 insertions(+), 24 deletions(-)
Applied to u-boot-arm/master, with README corrected on the fly as suggested by Tom.
This on-the-fly correction is incomplete: CONFIG_SPL_MAX_BSS_SIZE -> CONFIG_SPL_BSS_MAX_SIZE
Best regards, Benoît
Hi Benoît,
Applied to u-boot-arm/master, with README corrected on the fly as suggested by Tom.
This on-the-fly correction is incomplete: CONFIG_SPL_MAX_BSS_SIZE -> CONFIG_SPL_BSS_MAX_SIZE
Best regards, Benoît
Argh! Where were you when I was typing? :)
Well, since it is in the README, i.e. non-functional, there's no point in rolling back; I'll make it a cosmetic patch.
Thanks for noticing it!
Amicalement,
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 12K max size between 10K image (text,rodata,data) and 2K BSS based on sizes reported for current build:
text data bss 9073 840 500
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v3: None Changes in v2: - brought back total size to 12K
board/ait/cam_enc_4xx/u-boot-spl.lds | 2 +- include/configs/cam_enc_4xx.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds index be1027d..18a152d 100644 --- a/board/ait/cam_enc_4xx/u-boot-spl.lds +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h index 56528dd..b73b72c 100644 --- a/include/configs/cam_enc_4xx.h +++ b/include/configs/cam_enc_4xx.h @@ -230,7 +230,9 @@ #define CONFIG_SPL_STACK (0x00010000 + 0x7f00)
#define CONFIG_SPL_TEXT_BASE 0x00000020 /*CONFIG_SYS_SRAM_START*/ -#define CONFIG_SPL_MAX_SIZE 12320 +/* SPL total size is 12K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 10240 +#define CONFIG_SPL_BSS_MAX_SIZE (2*1024)
#ifndef CONFIG_SPL_BUILD #define CONFIG_SYS_TEXT_BASE 0x81080000
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 32K max size between 24K image (text,rodata,data) and 8K BSS based on sizes reported for current build:
text data bss 15676 1316 108
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v3: None Changes in v2: - fixed commit summary typoes
board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 2 +- include/configs/da850evm.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds index 2ae5a2c..25d6728 100644 --- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds +++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds @@ -25,7 +25,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index 99b4de7..e15c86e 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -399,7 +399,9 @@ #define CONFIG_SPL_LDSCRIPT "board/$(BOARDDIR)/u-boot-spl-da850evm.lds" #define CONFIG_SPL_STACK 0x8001ff00 #define CONFIG_SPL_TEXT_BASE 0x80000000 -#define CONFIG_SPL_MAX_SIZE 32768 +/* SPL max size is 24K -- but --pad-to requires a single decimal number */ +#define CONFIG_SPL_MAX_SIZE 24576 +#define CONFIG_SPL_BSS_MAX_SIZE (8*1024) #endif
/* Load U-Boot Image From MMC */
CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split 14K max size between 10K image (text,rodata,data) and 4K BSS based on sizes reported for current build:
text data bss 4136 904 0
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- Changes in v3: None Changes in v2: - fixed spacing in commit summary
board/samsung/smdk5250/smdk5250-uboot-spl.lds | 2 +- include/configs/exynos5250-dt.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds index 7df0a1d..c45cc37 100644 --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds @@ -26,7 +26,7 @@ */
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \ - LENGTH = CONFIG_SPL_MAX_SIZE } + LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 496a194..372b0b4 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -141,7 +141,8 @@ /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400 -#define CONFIG_SPL_MAX_SIZE (14 * 1024) +#define CONFIG_SPL_MAX_SIZE (10 * 1024) +#define CONFIG_SPL_BSS_MAX_SIZE (4 * 1024)
#define CONFIG_BOOTCOMMAND "mmc read 40007000 451 2000; bootm 40007000"
Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds as this file is never used for SPL builds.
Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds to separately test image (text,data,rodata...) size, BSS size, and full footprint each against its own max, and make Tegra boards check full footprint.
Also, output section mmutable is not used in SPL builds. Remove it.
Finally, update README regarding the (now homogeneous) semantics of CONFIG_SPL_[BSS_]MAX_SIZE and add the new CONFIG_SPL_MAX_FOOTPRINT macro.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- Changes in v3: - introduced CONFIG_SPL_MAX_FOOTPRINT - fixed typo in BSS size test
Changes in v2: - removed mmutable in SPL linker file
README | 26 ++++++++++++++++++++++++-- arch/arm/cpu/u-boot-spl.lds | 24 +++++++++++++----------- arch/arm/cpu/u-boot.lds | 4 ---- include/configs/tegra-common.h | 2 +- 4 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/README b/README index 6272853..c470489 100644 --- a/README +++ b/README @@ -2775,8 +2775,24 @@ FIT uImage format: CONFIG_SPL_LDSCRIPT LDSCRIPT for linking the SPL binary.
+ CONFIG_SPL_MAX_FOOTPRINT + Maximum size in memory allocated to the SPL. + When defined, linker checks that the actual memory used + by SPL size does not exceed it. + CONFIG_MAX_FOOTPRINT should always be equal to or greater + than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets. + CONFIG_SPL_MAX_SIZE - Maximum binary size (text, data and rodata) of the SPL binary. + Maximum size for the image (sum of the text, data, rodata + and linker lists sections) of the SPL executable. + When defined, linker checks that the actual image size does + not exceed it. + CONFIG_MAX_FOOTPRINT should always be equal to or greater + than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_TEXT_BASE TEXT_BASE for linking the SPL binary. @@ -2789,7 +2805,13 @@ FIT uImage format: Link address for the BSS within the SPL binary.
CONFIG_SPL_BSS_MAX_SIZE - Maximum binary size of the BSS section of the SPL binary. + Maximum size for the BSS section of the SPL executable. + When defined, linker checks that the actual BSS size does + not exceed it. + CONFIG_MAX_FOOTPRINT should always be equal to or greater + than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if + it exists. + Note: image and BSS are disjoint for some targets.
CONFIG_SPL_STACK Adress of the start of the stack SPL will use diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 3c0d99c..1408f03 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -65,15 +65,6 @@ SECTIONS
_end = .;
- /* - * Deprecated: this MMU section is used by pxa at present but - * should not be used by new boards/CPUs. - */ - . = ALIGN(4096); - .mmutable : { - *(.mmutable) - } - .bss __rel_dyn_start (OVERLAY) : { __bss_start = .; *(.bss*) @@ -88,6 +79,17 @@ SECTIONS /DISCARD/ : { *(.gnu*) } }
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); +#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \ + "SPL image too big"); +#endif + +#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \ + "SPL image BSS too big"); +#endif + +#if defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \ + "SPL image plus BSS too big"); #endif diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 3a1083d..7bbc4f5 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -101,7 +101,3 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } } - -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE) -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big"); -#endif diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index 036ded0..e9241b8 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -158,7 +158,7 @@ #define CONFIG_SPL_RAM_DEVICE #define CONFIG_SPL_BOARD_INIT #define CONFIG_SPL_NAND_SIMPLE -#define CONFIG_SPL_MAX_SIZE (CONFIG_SYS_TEXT_BASE - \ +#define CONFIG_SPL_MAX_FOOTPRINT (CONFIG_SYS_TEXT_BASE - \ CONFIG_SPL_TEXT_BASE) #define CONFIG_SYS_SPL_MALLOC_SIZE 0x00010000
participants (6)
-
Albert ARIBAUD -
Benoît Thébaudeau -
Heiko Schocher -
Minkyu Kang -
Stephen Warren -
Tom Rini