* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
@ 2012-10-16 21:50 Stephen Warren
2012-10-16 21:50 ` [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules Stephen Warren
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Stephen Warren @ 2012-10-16 21:50 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
When -ffunction-sections or -fdata-section are used, symbols are placed
into sections such as .data.eserial1_device and .bss.serial_current.
Update the linker script to explicitly include these. Without this
change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
the sections do end up being included, but __bss_end__ gets set to the
same value as __bss_start.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This series fixes an SPL size overflow problem on Tegra. Tom Warren is
out on vacation until Oct 25th, so he certainly won't be able to review
this. Perhaps it could be applied directly to the ARM tree if enough
Tegra people ack the series?
Note that this series is not enough to make Tegra support work; either
you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
serial_initialize() right before serial_init() in preloader_console_init()
or wait for Allen Martin to rework Tegra's SPL support using the common
SPL code.
arch/arm/cpu/u-boot.lds | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index e49ca0c..ae04a6e 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -35,7 +35,7 @@ SECTIONS
{
__image_copy_start = .;
CPUDIR/start.o (.text)
- *(.text)
+ *(.text*)
}
. = ALIGN(4);
@@ -43,14 +43,14 @@ SECTIONS
. = ALIGN(4);
.data : {
- *(.data)
+ *(.data*)
}
. = ALIGN(4);
. = .;
__u_boot_cmd_start = .;
- .u_boot_cmd : { *(.u_boot_cmd) }
+ .u_boot_cmd : { *(.u_boot_cmd*) }
__u_boot_cmd_end = .;
. = ALIGN(4);
@@ -65,7 +65,7 @@ SECTIONS
.dynsym : {
__dynsym_start = .;
- *(.dynsym)
+ *(.dynsym*)
}
_end = .;
@@ -81,7 +81,7 @@ SECTIONS
.bss __rel_dyn_start (OVERLAY) : {
__bss_start = .;
- *(.bss)
+ *(.bss*)
. = ALIGN(4);
__bss_end__ = .;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules
2012-10-16 21:50 [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Stephen Warren
@ 2012-10-16 21:50 ` Stephen Warren
2012-10-18 0:01 ` Simon Glass
2012-10-16 21:50 ` [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile Stephen Warren
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-16 21:50 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
The rules to generate u-boot-{no,}dtb-tegra.bin were almost identical.
Combine them into a single paremeterized rule. This will allow the next
patch to edit a single rule, rather than being cut/paste twice.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Makefile | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index ab34fa7..425adf4 100644
--- a/Makefile
+++ b/Makefile
@@ -514,17 +514,18 @@ $(obj)u-boot.spr: $(obj)u-boot.img $(obj)spl/u-boot-spl.bin
ifeq ($(SOC),tegra20)
ifeq ($(CONFIG_OF_SEPARATE),y)
-$(obj)u-boot-dtb-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(obj)u-boot.dtb
- $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
- cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(obj)u-boot.dtb > $@
- rm $(obj)spl/u-boot-spl-pad.bin
+nodtb=dtb
+dtbfile=$(obj)u-boot.dtb
else
-$(obj)u-boot-nodtb-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
+nodtb=nodtb
+dtbfile=
+endif
+
+$(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
- cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin > $@
+ cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(dtbfile) > $@
rm $(obj)spl/u-boot-spl-pad.bin
endif
-endif
ifeq ($(CONFIG_SANDBOX),y)
GEN_UBOOT = \
--
1.7.0.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-16 21:50 [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Stephen Warren
2012-10-16 21:50 ` [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules Stephen Warren
@ 2012-10-16 21:50 ` Stephen Warren
2012-10-18 0:03 ` Simon Glass
2012-10-18 16:27 ` Tom Rini
2012-10-16 21:50 ` [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE Stephen Warren
2012-10-17 23:58 ` [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Simon Glass
3 siblings, 2 replies; 22+ messages in thread
From: Stephen Warren @ 2012-10-16 21:50 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
corrupt the main U-Boot binary during execution, causing the main U-Boot
binary to fail. Check for this situation during the build to avoid
extremely annoying and hard-to-find bugs. Note that checking the size of
u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
u-boot-spl.bin.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Makefile | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 425adf4..2c2d8b6 100644
--- a/Makefile
+++ b/Makefile
@@ -522,6 +522,11 @@ dtbfile=
endif
$(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
+ ebss=`${CROSS_COMPILE}objdump -t spl/u-boot-spl|awk '/__bss_end__/ {print "0x"$$1}'` ; \
+ if [ $$(($$ebss)) -gt $$(($(CONFIG_SYS_TEXT_BASE))) ]; then \
+ echo ERROR: SPL BSS ends beyond CONFIG_SYS_TEXT_BASE > /dev/stderr; \
+ exit 1; \
+ fi
$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(dtbfile) > $@
rm $(obj)spl/u-boot-spl-pad.bin
--
1.7.0.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-16 21:50 [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Stephen Warren
2012-10-16 21:50 ` [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules Stephen Warren
2012-10-16 21:50 ` [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile Stephen Warren
@ 2012-10-16 21:50 ` Stephen Warren
2012-10-16 22:09 ` Lucas Stach
2012-10-17 23:58 ` [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Simon Glass
3 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-16 21:50 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
overlap the main U-Boot.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
include/configs/tegra20-common.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h
index dc7444d..ced278d 100644
--- a/include/configs/tegra20-common.h
+++ b/include/configs/tegra20-common.h
@@ -168,7 +168,7 @@
#define PHYS_SDRAM_1 NV_PA_SDRC_CS0
#define PHYS_SDRAM_1_SIZE 0x20000000 /* 512M */
-#define CONFIG_SYS_TEXT_BASE 0x0010c000
+#define CONFIG_SYS_TEXT_BASE 0x0010d000
#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1
#define CONFIG_SYS_INIT_RAM_ADDR CONFIG_STACKBASE
--
1.7.0.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-16 21:50 ` [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE Stephen Warren
@ 2012-10-16 22:09 ` Lucas Stach
2012-10-16 22:43 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Lucas Stach @ 2012-10-16 22:09 UTC (permalink / raw)
To: u-boot
Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
> From: Stephen Warren <swarren@nvidia.com>
>
> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
> overlap the main U-Boot.
>
Is there any specific reason why the SPL is now bigger than before? Or
is this just because of the general U-Boot rework (like serial multi
anywhere)? And by how much has it grown? This is really more out of
curiosity rather than any real objection.
Aside from this I think the general idea is reasonable, as we are not
shipping a particularly slim U-Boot on any Tegra platform, nor do we
have to hit a hard size limit, so for the series:
Acked-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> include/configs/tegra20-common.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h
> index dc7444d..ced278d 100644
> --- a/include/configs/tegra20-common.h
> +++ b/include/configs/tegra20-common.h
> @@ -168,7 +168,7 @@
> #define PHYS_SDRAM_1 NV_PA_SDRC_CS0
> #define PHYS_SDRAM_1_SIZE 0x20000000 /* 512M */
>
> -#define CONFIG_SYS_TEXT_BASE 0x0010c000
> +#define CONFIG_SYS_TEXT_BASE 0x0010d000
> #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1
>
> #define CONFIG_SYS_INIT_RAM_ADDR CONFIG_STACKBASE
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-16 22:09 ` Lucas Stach
@ 2012-10-16 22:43 ` Stephen Warren
2012-10-18 0:05 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-16 22:43 UTC (permalink / raw)
To: u-boot
On 10/16/2012 04:09 PM, Lucas Stach wrote:
> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>> overlap the main U-Boot.
>
> Is there any specific reason why the SPL is now bigger than before? Or
> is this just because of the general U-Boot rework (like serial multi
> anywhere)? And by how much has it grown? This is really more out of
> curiosity rather than any real objection.
Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
build flags" which was the last patch in the series that enabled SPL on
Tegra, and it overflows even there:
Configuring for ventana board...
text data bss dec hex filename
226085 4384 274228 504697 7b379 ./u-boot
13857 153 4516 18526 485e ./spl/u-boot-spl
u-boot/master currently has:
Configuring for ventana board...
text data bss dec hex filename
233579 4432 274368 512379 7d17b ./u-boot
14382 201 4520 19103 4a9f ./spl/u-boot-spl
So, there is slight growth, but mainly I think we've just been getting
lucky.
Also, the overflow might possibly only have been exposed by the recent
serial rework; when I found the problem the serial rework caused on
Tegra, Allen mentioned that the missing BSS clearing hadn't been a
problem before since no global variables were used, but the serial
rework caused some to be.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-16 21:50 [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Stephen Warren
` (2 preceding siblings ...)
2012-10-16 21:50 ` [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE Stephen Warren
@ 2012-10-17 23:58 ` Simon Glass
2012-10-18 3:17 ` Stephen Warren
3 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-10-17 23:58 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> When -ffunction-sections or -fdata-section are used, symbols are placed
> into sections such as .data.eserial1_device and .bss.serial_current.
> Update the linker script to explicitly include these. Without this
> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
> the sections do end up being included, but __bss_end__ gets set to the
> same value as __bss_start.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
> out on vacation until Oct 25th, so he certainly won't be able to review
> this. Perhaps it could be applied directly to the ARM tree if enough
> Tegra people ack the series?
>
> Note that this series is not enough to make Tegra support work; either
> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
> serial_initialize() right before serial_init() in preloader_console_init()
> or wait for Allen Martin to rework Tegra's SPL support using the common
> SPL code.
Are you going to submit a patch to enable function-sections, or is
that a separate discussion?
>
> arch/arm/cpu/u-boot.lds | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> index e49ca0c..ae04a6e 100644
> --- a/arch/arm/cpu/u-boot.lds
> +++ b/arch/arm/cpu/u-boot.lds
> @@ -35,7 +35,7 @@ SECTIONS
> {
> __image_copy_start = .;
> CPUDIR/start.o (.text)
> - *(.text)
> + *(.text*)
> }
>
> . = ALIGN(4);
> @@ -43,14 +43,14 @@ SECTIONS
>
> . = ALIGN(4);
> .data : {
> - *(.data)
> + *(.data*)
> }
>
> . = ALIGN(4);
>
> . = .;
> __u_boot_cmd_start = .;
> - .u_boot_cmd : { *(.u_boot_cmd) }
> + .u_boot_cmd : { *(.u_boot_cmd*) }
I don't think this line is needed?
> __u_boot_cmd_end = .;
>
> . = ALIGN(4);
> @@ -65,7 +65,7 @@ SECTIONS
>
> .dynsym : {
> __dynsym_start = .;
> - *(.dynsym)
> + *(.dynsym*)
Nor this one?
> }
>
> _end = .;
> @@ -81,7 +81,7 @@ SECTIONS
>
> .bss __rel_dyn_start (OVERLAY) : {
> __bss_start = .;
> - *(.bss)
> + *(.bss*)
> . = ALIGN(4);
> __bss_end__ = .;
> }
> --
> 1.7.0.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules
2012-10-16 21:50 ` [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules Stephen Warren
@ 2012-10-18 0:01 ` Simon Glass
0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2012-10-18 0:01 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The rules to generate u-boot-{no,}dtb-tegra.bin were almost identical.
> Combine them into a single paremeterized rule. This will allow the next
> patch to edit a single rule, rather than being cut/paste twice.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
> ---
> Makefile | 15 ++++++++-------
> 1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ab34fa7..425adf4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -514,17 +514,18 @@ $(obj)u-boot.spr: $(obj)u-boot.img $(obj)spl/u-boot-spl.bin
>
> ifeq ($(SOC),tegra20)
> ifeq ($(CONFIG_OF_SEPARATE),y)
> -$(obj)u-boot-dtb-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(obj)u-boot.dtb
> - $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> - cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(obj)u-boot.dtb > $@
> - rm $(obj)spl/u-boot-spl-pad.bin
> +nodtb=dtb
> +dtbfile=$(obj)u-boot.dtb
> else
> -$(obj)u-boot-nodtb-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> +nodtb=nodtb
> +dtbfile=
You could omit this line if you like.
> +endif
> +
> +$(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
If you like, you could have a continuation on this line as it is a bit long.
> $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> - cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin > $@
> + cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(dtbfile) > $@
> rm $(obj)spl/u-boot-spl-pad.bin
> endif
> -endif
>
> ifeq ($(CONFIG_SANDBOX),y)
> GEN_UBOOT = \
> --
> 1.7.0.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-16 21:50 ` [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile Stephen Warren
@ 2012-10-18 0:03 ` Simon Glass
2012-10-18 3:18 ` Stephen Warren
2012-10-18 16:27 ` Tom Rini
1 sibling, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-10-18 0:03 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
> corrupt the main U-Boot binary during execution, causing the main U-Boot
> binary to fail. Check for this situation during the build to avoid
> extremely annoying and hard-to-find bugs. Note that checking the size of
> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
> u-boot-spl.bin.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> Makefile | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 425adf4..2c2d8b6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -522,6 +522,11 @@ dtbfile=
> endif
>
> $(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
> + ebss=`${CROSS_COMPILE}objdump -t spl/u-boot-spl|awk '/__bss_end__/ {print "0x"$$1}'` ; \
> + if [ $$(($$ebss)) -gt $$(($(CONFIG_SYS_TEXT_BASE))) ]; then \
> + echo ERROR: SPL BSS ends beyond CONFIG_SYS_TEXT_BASE > /dev/stderr; \
> + exit 1; \
> + fi
Just wanted to check that this works ok for hex values?
> $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(dtbfile) > $@
> rm $(obj)spl/u-boot-spl-pad.bin
> --
> 1.7.0.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-16 22:43 ` Stephen Warren
@ 2012-10-18 0:05 ` Simon Glass
2012-10-18 3:20 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-10-18 0:05 UTC (permalink / raw)
To: u-boot
Hi Stephen.
On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/16/2012 04:09 PM, Lucas Stach wrote:
>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>>> overlap the main U-Boot.
>>
>> Is there any specific reason why the SPL is now bigger than before? Or
>> is this just because of the general U-Boot rework (like serial multi
>> anywhere)? And by how much has it grown? This is really more out of
>> curiosity rather than any real objection.
>
> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
> build flags" which was the last patch in the series that enabled SPL on
> Tegra, and it overflows even there:
>
> Configuring for ventana board...
> text data bss dec hex filename
> 226085 4384 274228 504697 7b379 ./u-boot
> 13857 153 4516 18526 485e ./spl/u-boot-spl
>
> u-boot/master currently has:
>
> Configuring for ventana board...
> text data bss dec hex filename
> 233579 4432 274368 512379 7d17b ./u-boot
> 14382 201 4520 19103 4a9f ./spl/u-boot-spl
>
> So, there is slight growth, but mainly I think we've just been getting
> lucky.
>
> Also, the overflow might possibly only have been exposed by the recent
> serial rework; when I found the problem the serial rework caused on
> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
> problem before since no global variables were used, but the serial
> rework caused some to be.
To ask the opposite question, is it worth increasing by a whole 16KB
so that the base address of U-Boot is a more aligned number?
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-17 23:58 ` [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Simon Glass
@ 2012-10-18 3:17 ` Stephen Warren
2012-10-18 20:36 ` Albert ARIBAUD
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 3:17 UTC (permalink / raw)
To: u-boot
On 10/17/2012 05:58 PM, Simon Glass wrote:
> Hi Stephen,
>
> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> When -ffunction-sections or -fdata-section are used, symbols are placed
>> into sections such as .data.eserial1_device and .bss.serial_current.
>> Update the linker script to explicitly include these. Without this
>> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
>> the sections do end up being included, but __bss_end__ gets set to the
>> same value as __bss_start.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
>> out on vacation until Oct 25th, so he certainly won't be able to review
>> this. Perhaps it could be applied directly to the ARM tree if enough
>> Tegra people ack the series?
>>
>> Note that this series is not enough to make Tegra support work; either
>> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
>> serial_initialize() right before serial_init() in preloader_console_init()
>> or wait for Allen Martin to rework Tegra's SPL support using the common
>> SPL code.
>
> Are you going to submit a patch to enable function-sections, or is
> that a separate discussion?
For the SPL on Tegra, those flags were already on; this patch fixes a
bug rather than prepares for new functionality.
>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
>> - .u_boot_cmd : { *(.u_boot_cmd) }
>> + .u_boot_cmd : { *(.u_boot_cmd*) }
>
> I don't think this line is needed?
>
...
>> - *(.dynsym)
>> + *(.dynsym*)
>
> Nor this one?
Possibly. I changed all the section names to be future-proof. Perhaps a
more targeted patch is warranted.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-18 0:03 ` Simon Glass
@ 2012-10-18 3:18 ` Stephen Warren
0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 3:18 UTC (permalink / raw)
To: u-boot
On 10/17/2012 06:03 PM, Simon Glass wrote:
> Hi Stephen,
>
> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
>> corrupt the main U-Boot binary during execution, causing the main U-Boot
>> binary to fail. Check for this situation during the build to avoid
>> extremely annoying and hard-to-find bugs. Note that checking the size of
>> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
>> u-boot-spl.bin.
>> diff --git a/Makefile b/Makefile
>> $(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
>> + ebss=`${CROSS_COMPILE}objdump -t spl/u-boot-spl|awk '/__bss_end__/ {print "0x"$$1}'` ; \
>> + if [ $$(($$ebss)) -gt $$(($(CONFIG_SYS_TEXT_BASE))) ]; then \
>> + echo ERROR: SPL BSS ends beyond CONFIG_SYS_TEXT_BASE > /dev/stderr; \
>> + exit 1; \
>> + fi
>
> Just wanted to check that this works ok for hex values?
Yes, the reason I used $(( )) around the value is because it was hex, so
I needed to convert it to an integer for -gt.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-18 0:05 ` Simon Glass
@ 2012-10-18 3:20 ` Stephen Warren
2012-10-18 16:31 ` Tom Rini
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 3:20 UTC (permalink / raw)
To: u-boot
On 10/17/2012 06:05 PM, Simon Glass wrote:
> Hi Stephen.
>
> On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/16/2012 04:09 PM, Lucas Stach wrote:
>>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>>>> overlap the main U-Boot.
>>>
>>> Is there any specific reason why the SPL is now bigger than before? Or
>>> is this just because of the general U-Boot rework (like serial multi
>>> anywhere)? And by how much has it grown? This is really more out of
>>> curiosity rather than any real objection.
>>
>> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
>> build flags" which was the last patch in the series that enabled SPL on
>> Tegra, and it overflows even there:
>>
>> Configuring for ventana board...
>> text data bss dec hex filename
>> 226085 4384 274228 504697 7b379 ./u-boot
>> 13857 153 4516 18526 485e ./spl/u-boot-spl
>>
>> u-boot/master currently has:
>>
>> Configuring for ventana board...
>> text data bss dec hex filename
>> 233579 4432 274368 512379 7d17b ./u-boot
>> 14382 201 4520 19103 4a9f ./spl/u-boot-spl
>>
>> So, there is slight growth, but mainly I think we've just been getting
>> lucky.
>>
>> Also, the overflow might possibly only have been exposed by the recent
>> serial rework; when I found the problem the serial rework caused on
>> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
>> problem before since no global variables were used, but the serial
>> rework caused some to be.
>
> To ask the opposite question, is it worth increasing by a whole 16KB
> so that the base address of U-Boot is a more aligned number?
That would bloat the binary by about 12KB more than it needs to be. I
don't believe there's any particular need for the main U-Boot to be
built for any particular address, and we can just continue to bump up
this value as/when the SPL grows.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-16 21:50 ` [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile Stephen Warren
2012-10-18 0:03 ` Simon Glass
@ 2012-10-18 16:27 ` Tom Rini
2012-10-18 20:45 ` Stephen Warren
1 sibling, 1 reply; 22+ messages in thread
From: Tom Rini @ 2012-10-18 16:27 UTC (permalink / raw)
To: u-boot
On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
> corrupt the main U-Boot binary during execution, causing the main U-Boot
> binary to fail. Check for this situation during the build to avoid
> extremely annoying and hard-to-find bugs. Note that checking the size of
> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
> u-boot-spl.bin.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Can't you do this in the linker script like we do for other SPL size
constraints? Or am I just mis-reading how this is unique and that
link-time check can't be used? Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121018/44df933b/attachment.pgp>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-18 3:20 ` Stephen Warren
@ 2012-10-18 16:31 ` Tom Rini
2012-10-18 20:42 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Tom Rini @ 2012-10-18 16:31 UTC (permalink / raw)
To: u-boot
On Wed, Oct 17, 2012 at 09:20:31PM -0600, Stephen Warren wrote:
> On 10/17/2012 06:05 PM, Simon Glass wrote:
> > Hi Stephen.
> >
> > On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> On 10/16/2012 04:09 PM, Lucas Stach wrote:
> >>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
> >>>> From: Stephen Warren <swarren@nvidia.com>
> >>>>
> >>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
> >>>> overlap the main U-Boot.
> >>>
> >>> Is there any specific reason why the SPL is now bigger than before? Or
> >>> is this just because of the general U-Boot rework (like serial multi
> >>> anywhere)? And by how much has it grown? This is really more out of
> >>> curiosity rather than any real objection.
> >>
> >> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
> >> build flags" which was the last patch in the series that enabled SPL on
> >> Tegra, and it overflows even there:
> >>
> >> Configuring for ventana board...
> >> text data bss dec hex filename
> >> 226085 4384 274228 504697 7b379 ./u-boot
> >> 13857 153 4516 18526 485e ./spl/u-boot-spl
> >>
> >> u-boot/master currently has:
> >>
> >> Configuring for ventana board...
> >> text data bss dec hex filename
> >> 233579 4432 274368 512379 7d17b ./u-boot
> >> 14382 201 4520 19103 4a9f ./spl/u-boot-spl
> >>
> >> So, there is slight growth, but mainly I think we've just been getting
> >> lucky.
> >>
> >> Also, the overflow might possibly only have been exposed by the recent
> >> serial rework; when I found the problem the serial rework caused on
> >> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
> >> problem before since no global variables were used, but the serial
> >> rework caused some to be.
> >
> > To ask the opposite question, is it worth increasing by a whole 16KB
> > so that the base address of U-Boot is a more aligned number?
>
> That would bloat the binary by about 12KB more than it needs to be. I
> don't believe there's any particular need for the main U-Boot to be
> built for any particular address, and we can just continue to bump up
> this value as/when the SPL grows.
Well, lets stop and think for a minute more. Are we likely to add new
features to SPL on Tegra (direct OS booting, support in one binary for
both SPL-from-flash and SPL-from-something-else) ? If so, lets increase
this to a reasonable maximum now rather than keep bumping and breaking.
On TI parts for example, we are limited by our SRAM size and set that
(minus a bit for stack) as how big we allow SPL to be so that we don't
have to tweak this value based on toolchain or feature changes (ELDK 4.2
vs current Linaro for example, can be a noticable size difference).
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121018/5af1233a/attachment.pgp>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-18 3:17 ` Stephen Warren
@ 2012-10-18 20:36 ` Albert ARIBAUD
2012-10-18 20:58 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Albert ARIBAUD @ 2012-10-18 20:36 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Wed, 17 Oct 2012 21:17:45 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:
> On 10/17/2012 05:58 PM, Simon Glass wrote:
> > Hi Stephen,
> >
> > On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> When -ffunction-sections or -fdata-section are used, symbols are placed
> >> into sections such as .data.eserial1_device and .bss.serial_current.
> >> Update the linker script to explicitly include these. Without this
> >> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
> >> the sections do end up being included, but __bss_end__ gets set to the
> >> same value as __bss_start.
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> ---
> >> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
> >> out on vacation until Oct 25th, so he certainly won't be able to review
> >> this. Perhaps it could be applied directly to the ARM tree if enough
> >> Tegra people ack the series?
> >>
> >> Note that this series is not enough to make Tegra support work; either
> >> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
> >> serial_initialize() right before serial_init() in preloader_console_init()
> >> or wait for Allen Martin to rework Tegra's SPL support using the common
> >> SPL code.
> >
> > Are you going to submit a patch to enable function-sections, or is
> > that a separate discussion?
>
> For the SPL on Tegra, those flags were already on; this patch fixes a
> bug rather than prepares for new functionality.
>
> >> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
>
> >> - .u_boot_cmd : { *(.u_boot_cmd) }
> >> + .u_boot_cmd : { *(.u_boot_cmd*) }
> >
> > I don't think this line is needed?
> >
> ...
> >> - *(.dynsym)
> >> + *(.dynsym*)
> >
> > Nor this one?
>
> Possibly. I changed all the section names to be future-proof. Perhaps a
> more targeted patch is warranted.
Has this been (at least build-)tested on all boards which have
-ffunction-sections or -fdata-sections?
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE
2012-10-18 16:31 ` Tom Rini
@ 2012-10-18 20:42 ` Stephen Warren
0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 20:42 UTC (permalink / raw)
To: u-boot
On 10/18/2012 10:31 AM, Tom Rini wrote:
> On Wed, Oct 17, 2012 at 09:20:31PM -0600, Stephen Warren wrote:
>> On 10/17/2012 06:05 PM, Simon Glass wrote:
>>> On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren
>>> <swarren@wwwdotorg.org> wrote:
>>>> On 10/16/2012 04:09 PM, Lucas Stach wrote:
...
>>> To ask the opposite question, is it worth increasing by a whole
>>> 16KB so that the base address of U-Boot is a more aligned
>>> number?
>>
>> That would bloat the binary by about 12KB more than it needs to
>> be. I don't believe there's any particular need for the main
>> U-Boot to be built for any particular address, and we can just
>> continue to bump up this value as/when the SPL grows.
>
> Well, lets stop and think for a minute more. Are we likely to add
> new features to SPL on Tegra (direct OS booting, support in one
> binary for both SPL-from-flash and SPL-from-something-else) ?
I don't think so. The only purpose of the SPL on Tegra is to run from
SDRAM on the AVP CPU, set up clocks for the main Cortex-A9 core, and
to cause the A9 to start executing the concatenated main U-Boot image.
The SPL always runs from SDRAM.
(As background, the boot ROM sets up SDRAM on Tegra, and copies the
concatenated SPL+U-Boot binaries into SDRAM from whatever boot device,
so the typical reasons for using SPL don't exist on Tegra).
Allen Martin was thinking about getting the SPL to run from IRAM
rather than SDRAM, and I think only execute on the AVP CPU (e.g. for
use as a slimmed-down flashing tool downloaded via the boot ROM's USB
recovery mechanism). However, I think that would end up being an
entirely separate SPL build (since we'd need both, not cut over), so
the size requirements would not impact in any way the SPL-in-SDRAM
that we have today.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-18 16:27 ` Tom Rini
@ 2012-10-18 20:45 ` Stephen Warren
2012-10-18 20:50 ` Tom Rini
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 20:45 UTC (permalink / raw)
To: u-boot
On 10/18/2012 10:27 AM, Tom Rini wrote:
> On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:
>
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will
>> likely corrupt the main U-Boot binary during execution, causing
>> the main U-Boot binary to fail. Check for this situation during
>> the build to avoid extremely annoying and hard-to-find bugs. Note
>> that checking the size of u-boot-spl.bin is not enough, since BSS
>> size doesn't affect the size of u-boot-spl.bin.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> Can't you do this in the linker script like we do for other SPL
> size constraints? Or am I just mis-reading how this is unique and
> that link-time check can't be used? Thanks!
Ah, there aren't any such checks in the linker script I looked at, so
I wasn't aware of this capability. I found the following in a
different linker script:
ASSERT(__bss_end__ <= 0xfff01000, "NAND bootstrap too big");
I agree using that technique would make sense; I'll try it out.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile
2012-10-18 20:45 ` Stephen Warren
@ 2012-10-18 20:50 ` Tom Rini
0 siblings, 0 replies; 22+ messages in thread
From: Tom Rini @ 2012-10-18 20:50 UTC (permalink / raw)
To: u-boot
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/18/12 13:45, Stephen Warren wrote:
> On 10/18/2012 10:27 AM, Tom Rini wrote:
>> On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:
>>
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will
>>> likely corrupt the main U-Boot binary during execution,
>>> causing the main U-Boot binary to fail. Check for this
>>> situation during the build to avoid extremely annoying and
>>> hard-to-find bugs. Note that checking the size of
>>> u-boot-spl.bin is not enough, since BSS size doesn't affect the
>>> size of u-boot-spl.bin.
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> Can't you do this in the linker script like we do for other SPL
>> size constraints? Or am I just mis-reading how this is unique
>> and that link-time check can't be used? Thanks!
>
> Ah, there aren't any such checks in the linker script I looked at,
> so I wasn't aware of this capability. I found the following in a
> different linker script:
>
> ASSERT(__bss_end__ <= 0xfff01000, "NAND bootstrap too big");
>
> I agree using that technique would make sense; I'll try it out.
There's a few different ones, sadly. grep around on CONFIG_SPL_MAX_SIZE.
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iQIcBAEBAgAGBQJQgGucAAoJENk4IS6UOR1WmZwP/jFzQPiYD4w33IMRtuGe2ip9
QOZUIsvVYUx7ufwhQeUUMjf/JQ/c+qw+GZeVRsj1pdsw0frjuIk1Z1smgx3QAa7w
yXAlQ7bEWhfl2DsXZeVcLjaidTIkeZl6azVOfNEV6pLDlW330z9cOun3AjtV0a1f
kXVGnmyEOsaLURI1DYfkoLD0p+uTNED1VFpTctPWMrCwBoS10M27bwAWkaIjFScu
8Keg+ThAa7Vac8+YKqNPtYuyAO2ZMVXISDe1LLvplcmCtow9g1qm7YnV2QsWqhrM
U8BfYwJdKDf3aQcqiTN5rQD0x7pIHC+3wP2BMACgN3cNu//RY373+D3Ouq3MVEEi
sUw0aMmKyc0Nehj2ONn+tgguZxKrMPNokLYz++/kTOijPZ1AtI1mObsfeT+aV1HW
vVBs5ThKILWOj+pGak7WKhO+yfWiRhmk9JgqAmvf/+uq5vKVr2bF8i2AGc88eAbN
TL8Ct8/u5BDJsNJpY1GMFw1mIR9ZxBBCEbqt3RjRLR3sZLX86qrbRFzVGF1BzPBl
xFYJMRObl4xpgTjf/r9JdROL0c6rizlHvNKwfJCjjea/F8Q0WFaWtlvx15SZQT6Y
s8Xf9iCUPTLLN6hMf0cgN42WCg0THqTbINpa66BsDYPxrVPbBf1hdOb5dhOOMILo
v5VNdn4zdR/d58JLBa0Z
=2haV
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-18 20:36 ` Albert ARIBAUD
@ 2012-10-18 20:58 ` Stephen Warren
2012-10-18 21:17 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 20:58 UTC (permalink / raw)
To: u-boot
On 10/18/2012 02:36 PM, Albert ARIBAUD wrote:
> Hi Stephen,
>
> On Wed, 17 Oct 2012 21:17:45 -0600, Stephen Warren
> <swarren@wwwdotorg.org> wrote:
>
>> On 10/17/2012 05:58 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> When -ffunction-sections or -fdata-section are used, symbols are placed
>>>> into sections such as .data.eserial1_device and .bss.serial_current.
>>>> Update the linker script to explicitly include these. Without this
>>>> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
>>>> the sections do end up being included, but __bss_end__ gets set to the
>>>> same value as __bss_start.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>> ---
>>>> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
>>>> out on vacation until Oct 25th, so he certainly won't be able to review
>>>> this. Perhaps it could be applied directly to the ARM tree if enough
>>>> Tegra people ack the series?
>>>>
>>>> Note that this series is not enough to make Tegra support work; either
>>>> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
>>>> serial_initialize() right before serial_init() in preloader_console_init()
>>>> or wait for Allen Martin to rework Tegra's SPL support using the common
>>>> SPL code.
>>>
>>> Are you going to submit a patch to enable function-sections, or is
>>> that a separate discussion?
>>
>> For the SPL on Tegra, those flags were already on; this patch fixes a
>> bug rather than prepares for new functionality.
>>
>>>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
>>
>>>> - .u_boot_cmd : { *(.u_boot_cmd) }
>>>> + .u_boot_cmd : { *(.u_boot_cmd*) }
>>>
>>> I don't think this line is needed?
>>>
>> ...
>>>> - *(.dynsym)
>>>> + *(.dynsym*)
>>>
>>> Nor this one?
>>
>> Possibly. I changed all the section names to be future-proof. Perhaps a
>> more targeted patch is warranted.
>
> Has this been (at least build-)tested on all boards which have
> -ffunction-sections or -fdata-sections?
Yes; those flags both appear to be turned on when building the SPL for
Tegra (although strangely, not when building the main U-Boot):
xxx-gcc -M -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__
-ffunction-sections -fdata-sections -DCONFIG_SYS_TEXT_BASE=0x0010c000
-DCONFIG_SPL_TEXT_BASE=0x00108000 -DCONFIG_SPL_BUILD
-I/home/swarren/shared/git_wa/u-boot/include -fno-builtin -ffreestanding
-nostdinc -isystem xxx -pipe -DCONFIG_ARM -D__ARM__ -marm
-mno-thumb-interwork -mabi=aapcs-linux -march=armv4 -mtune=arm7tdmi -MQ
xxx/spl/arch/arm/cpu/arm720t/interrupts.o interrupts.c >
xxx/spl/arch/arm/cpu/arm720t/.depend.interrupts
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-18 20:58 ` Stephen Warren
@ 2012-10-18 21:17 ` Stephen Warren
2012-10-20 10:31 ` Albert ARIBAUD
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-10-18 21:17 UTC (permalink / raw)
To: u-boot
On 10/18/2012 02:58 PM, Stephen Warren wrote:
> On 10/18/2012 02:36 PM, Albert ARIBAUD wrote:
>> Hi Stephen,
>>
>> On Wed, 17 Oct 2012 21:17:45 -0600, Stephen Warren
>> <swarren@wwwdotorg.org> wrote:
>>
>>> On 10/17/2012 05:58 PM, Simon Glass wrote:
>>>> Hi Stephen,
>>>>
>>>> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>
>>>>> When -ffunction-sections or -fdata-section are used, symbols are placed
>>>>> into sections such as .data.eserial1_device and .bss.serial_current.
>>>>> Update the linker script to explicitly include these. Without this
>>>>> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
>>>>> the sections do end up being included, but __bss_end__ gets set to the
>>>>> same value as __bss_start.
>>>>>
>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>>> ---
>>>>> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
>>>>> out on vacation until Oct 25th, so he certainly won't be able to review
>>>>> this. Perhaps it could be applied directly to the ARM tree if enough
>>>>> Tegra people ack the series?
>>>>>
>>>>> Note that this series is not enough to make Tegra support work; either
>>>>> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
>>>>> serial_initialize() right before serial_init() in preloader_console_init()
>>>>> or wait for Allen Martin to rework Tegra's SPL support using the common
>>>>> SPL code.
>>>>
>>>> Are you going to submit a patch to enable function-sections, or is
>>>> that a separate discussion?
>>>
>>> For the SPL on Tegra, those flags were already on; this patch fixes a
>>> bug rather than prepares for new functionality.
>>>
>>>>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
>>>
>>>>> - .u_boot_cmd : { *(.u_boot_cmd) }
>>>>> + .u_boot_cmd : { *(.u_boot_cmd*) }
>>>>
>>>> I don't think this line is needed?
>>>>
>>> ...
>>>>> - *(.dynsym)
>>>>> + *(.dynsym*)
>>>>
>>>> Nor this one?
>>>
>>> Possibly. I changed all the section names to be future-proof. Perhaps a
>>> more targeted patch is warranted.
>>
>> Has this been (at least build-)tested on all boards which have
>> -ffunction-sections or -fdata-sections?
>
> Yes; those flags both appear to be turned on when building the SPL for
> Tegra (although strangely, not when building the main U-Boot):
>
> xxx-gcc -M -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__
> -ffunction-sections -fdata-sections -DCONFIG_SYS_TEXT_BASE=0x0010c000
> -DCONFIG_SPL_TEXT_BASE=0x00108000 -DCONFIG_SPL_BUILD
> -I/home/swarren/shared/git_wa/u-boot/include -fno-builtin -ffreestanding
> -nostdinc -isystem xxx -pipe -DCONFIG_ARM -D__ARM__ -marm
> -mno-thumb-interwork -mabi=aapcs-linux -march=armv4 -mtune=arm7tdmi -MQ
> xxx/spl/arch/arm/cpu/arm720t/interrupts.o interrupts.c >
> xxx/spl/arch/arm/cpu/arm720t/.depend.interrupts
I tried to test this more widely. I found that only
arch/arm/cpu/ixp/config.mk enables this on ARM, with the comment:
> # -fdata-sections triggers "section .bss overlaps section .rel.dyn" linker error
> PLATFORM_RELFLAGS += -ffunction-sections
> LDFLAGS_u-boot += --gc-sections
Indeed, if I edit arch/arm/cpu/armv7/config.mk to enable both
-ffunction-sections -fdata-sections, I do see that same section overlap
error building for Tegra.
Then, if I apply the .lds patch we're discussing, Tegra builds (and
runs) OK with those linker flags enabled. So, this patch seems to fix
two issues not just one - bonus:-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections
2012-10-18 21:17 ` Stephen Warren
@ 2012-10-20 10:31 ` Albert ARIBAUD
0 siblings, 0 replies; 22+ messages in thread
From: Albert ARIBAUD @ 2012-10-20 10:31 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Thu, 18 Oct 2012 15:17:52 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:
> On 10/18/2012 02:58 PM, Stephen Warren wrote:
> > On 10/18/2012 02:36 PM, Albert ARIBAUD wrote:
> >> Hi Stephen,
> >>
> >> On Wed, 17 Oct 2012 21:17:45 -0600, Stephen Warren
> >> <swarren@wwwdotorg.org> wrote:
> >>
> >>> On 10/17/2012 05:58 PM, Simon Glass wrote:
> >>>> Hi Stephen,
> >>>>
> >>>> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>>>> From: Stephen Warren <swarren@nvidia.com>
> >>>>>
> >>>>> When -ffunction-sections or -fdata-section are used, symbols are placed
> >>>>> into sections such as .data.eserial1_device and .bss.serial_current.
> >>>>> Update the linker script to explicitly include these. Without this
> >>>>> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
> >>>>> the sections do end up being included, but __bss_end__ gets set to the
> >>>>> same value as __bss_start.
> >>>>>
> >>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >>>>> ---
> >>>>> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
> >>>>> out on vacation until Oct 25th, so he certainly won't be able to review
> >>>>> this. Perhaps it could be applied directly to the ARM tree if enough
> >>>>> Tegra people ack the series?
> >>>>>
> >>>>> Note that this series is not enough to make Tegra support work; either
> >>>>> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
> >>>>> serial_initialize() right before serial_init() in preloader_console_init()
> >>>>> or wait for Allen Martin to rework Tegra's SPL support using the common
> >>>>> SPL code.
> >>>>
> >>>> Are you going to submit a patch to enable function-sections, or is
> >>>> that a separate discussion?
> >>>
> >>> For the SPL on Tegra, those flags were already on; this patch fixes a
> >>> bug rather than prepares for new functionality.
> >>>
> >>>>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> >>>
> >>>>> - .u_boot_cmd : { *(.u_boot_cmd) }
> >>>>> + .u_boot_cmd : { *(.u_boot_cmd*) }
> >>>>
> >>>> I don't think this line is needed?
> >>>>
> >>> ...
> >>>>> - *(.dynsym)
> >>>>> + *(.dynsym*)
> >>>>
> >>>> Nor this one?
> >>>
> >>> Possibly. I changed all the section names to be future-proof. Perhaps a
> >>> more targeted patch is warranted.
> >>
> >> Has this been (at least build-)tested on all boards which have
> >> -ffunction-sections or -fdata-sections?
> >
> > Yes; those flags both appear to be turned on when building the SPL for
> > Tegra (although strangely, not when building the main U-Boot):
> >
> > xxx-gcc -M -g -Os -fno-common -ffixed-r8 -msoft-float -D__KERNEL__
> > -ffunction-sections -fdata-sections -DCONFIG_SYS_TEXT_BASE=0x0010c000
> > -DCONFIG_SPL_TEXT_BASE=0x00108000 -DCONFIG_SPL_BUILD
> > -I/home/swarren/shared/git_wa/u-boot/include -fno-builtin -ffreestanding
> > -nostdinc -isystem xxx -pipe -DCONFIG_ARM -D__ARM__ -marm
> > -mno-thumb-interwork -mabi=aapcs-linux -march=armv4 -mtune=arm7tdmi -MQ
> > xxx/spl/arch/arm/cpu/arm720t/interrupts.o interrupts.c >
> > xxx/spl/arch/arm/cpu/arm720t/.depend.interrupts
>
> I tried to test this more widely. I found that only
> arch/arm/cpu/ixp/config.mk enables this on ARM, with the comment:
>
> > # -fdata-sections triggers "section .bss overlaps section .rel.dyn" linker error
> > PLATFORM_RELFLAGS += -ffunction-sections
> > LDFLAGS_u-boot += --gc-sections
>
> Indeed, if I edit arch/arm/cpu/armv7/config.mk to enable both
> -ffunction-sections -fdata-sections, I do see that same section overlap
> error building for Tegra.
>
> Then, if I apply the .lds patch we're discussing, Tegra builds (and
> runs) OK with those linker flags enabled. So, this patch seems to fix
> two issues not just one - bonus:-)
Unless someone opposes, I will pick this one in u-boot-arm/master later
today, as I will be doing some .lds merging on ARM (and then some
start.S merging). Once this is done, the Tegra tree (Tom already Cc:)
should merge u-boot-arm/master to get this first commit in, then apply
commits 2 to 5.
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-10-20 10:31 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 21:50 [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Stephen Warren
2012-10-16 21:50 ` [U-Boot] [PATCH 2/4] ARM: tegra: combine duplicate Makefile rules Stephen Warren
2012-10-18 0:01 ` Simon Glass
2012-10-16 21:50 ` [U-Boot] [PATCH 3/4] ARM: tegra: check for SPL size overflow in makefile Stephen Warren
2012-10-18 0:03 ` Simon Glass
2012-10-18 3:18 ` Stephen Warren
2012-10-18 16:27 ` Tom Rini
2012-10-18 20:45 ` Stephen Warren
2012-10-18 20:50 ` Tom Rini
2012-10-16 21:50 ` [U-Boot] [PATCH 4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE Stephen Warren
2012-10-16 22:09 ` Lucas Stach
2012-10-16 22:43 ` Stephen Warren
2012-10-18 0:05 ` Simon Glass
2012-10-18 3:20 ` Stephen Warren
2012-10-18 16:31 ` Tom Rini
2012-10-18 20:42 ` Stephen Warren
2012-10-17 23:58 ` [U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections Simon Glass
2012-10-18 3:17 ` Stephen Warren
2012-10-18 20:36 ` Albert ARIBAUD
2012-10-18 20:58 ` Stephen Warren
2012-10-18 21:17 ` Stephen Warren
2012-10-20 10:31 ` Albert ARIBAUD
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox