[PATCH] env: another attempt at fixing SPL build failures
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
tried to fix, namely that the reference to env_flags_validate from env_htab cannot be satisfied when flags.o is not built in. However, that commit got reverted by
d90fc9c3de Revert "env: solve compilation error in SPL"
Necessary, but not sufficient conditions to see this are
CONFIG_SPL=y (obviously) CONFIG_SPL_ENV_SUPPORT=n (so flags.o does not get compiled) CONFIG_SPL_LIBCOMMON_SUPPORT=y (so env/built-in.o is part of the SPL link)
Now, these are satisfied for e.g. imx6q_logic_defconfig. But that builds just fine, and spl/u-boot-spl.map lists .data.env_htab among the discarded (garbage collected) sections. Yet, on our mpc8309-derived board, we do see the build failure, so perhaps the linker works a bit differently on ppc than on ARM, or there's yet some other configuration option needed to observe the break.
This is another attempt at solving it, which also cleans up env/Makefile a bit: Introduce a def_bool y symbol CONFIG_ENV_SUPPORT which complements CONFIG_(SPL/TPL)_SUPPORT. Then use CONFIG_$(SPL_TPL_)ENV_SUPPORT to decide whether to include the five basic env/*.o files. For attr.o, flags.o and callback.o, this shouldn't change anything. Also, common.o and env.o still get unconditionally built for U-boot proper. But for TPL/SPL, those two are only included if CONFIG_(SPL/TPL)_SUPPORT is set.
Having that symbol should also allow simplifying conditionals such as
#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
found in drivers/reset/reset-socfpga.c to just CONFIG_IS_ENABLED(ENV_SUPPORT).
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- env/Kconfig | 3 +++ env/Makefile | 13 +++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig index ed12609f6a..4661082f0e 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -1,5 +1,8 @@ menu "Environment"
+config ENV_SUPPORT + def_bool y + config ENV_IS_NOWHERE bool "Environment is not stored" default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \ diff --git a/env/Makefile b/env/Makefile index 90144d6caf..e2a165b8f1 100644 --- a/env/Makefile +++ b/env/Makefile @@ -3,12 +3,13 @@ # (C) Copyright 2004-2006 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
-obj-y += common.o env.o +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += common.o +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += env.o +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o
ifndef CONFIG_SPL_BUILD -obj-y += attr.o -obj-y += callback.o -obj-y += flags.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o extra-$(CONFIG_ENV_IS_EMBEDDED) += embedded.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += embedded.o @@ -19,10 +20,6 @@ obj-$(CONFIG_ENV_IS_IN_ONENAND) += onenand.o obj-$(CONFIG_ENV_IS_IN_SATA) += sata.o obj-$(CONFIG_ENV_IS_IN_REMOTE) += remote.o obj-$(CONFIG_ENV_IS_IN_UBI) += ubi.o -else -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o endif
obj-$(CONFIG_$(SPL_TPL_)ENV_IS_NOWHERE) += nowhere.o
On 15/12/2019 23.29, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
tried to fix, namely that the reference to env_flags_validate from env_htab cannot be satisfied when flags.o is not built in. However, that commit got reverted by
d90fc9c3de Revert "env: solve compilation error in SPL"
Necessary, but not sufficient conditions to see this are
CONFIG_SPL=y (obviously) CONFIG_SPL_ENV_SUPPORT=n (so flags.o does not get compiled) CONFIG_SPL_LIBCOMMON_SUPPORT=y (so env/built-in.o is part of the SPL link)
Now, these are satisfied for e.g. imx6q_logic_defconfig. But that builds just fine, and spl/u-boot-spl.map lists .data.env_htab among the discarded (garbage collected) sections. Yet, on our mpc8309-derived board, we do see the build failure, so perhaps the linker works a bit differently on ppc than on ARM, or there's yet some other configuration option needed to observe the break.
Yeah, I think this is a difference in how the linker works on ppc vs arm. Doing
git grep --files-with-matches SPL=y -- configs/ | xargs grep --files-with-matches SPL_LIBCOMMON_SUPPORT=y | xargs grep --files-without-match SPL_ENV_SUPPORT | xargs head -n1
shows that all the in-tree defconfigs with the above combination are ARM boards (except for microblaze_defconfig, but I don't have such a toolchain), and they most likely all build just fine. But taking some random PPC config (say T2080QDS_NAND_defconfig) with the first and third point, and then manually disabling SPL_ENV_SUPPORT, immediately shows the break.
For reference, I have
$ ${CROSS_COMPILE}ld --version GNU ld (GNU Binutils for Ubuntu) 2.30
This is another attempt at solving it, which also cleans up env/Makefile a bit: Introduce a def_bool y symbol CONFIG_ENV_SUPPORT which complements CONFIG_(SPL/TPL)_SUPPORT. Then use CONFIG_$(SPL_TPL_)ENV_SUPPORT to decide whether to include the five basic env/*.o files. For attr.o, flags.o and callback.o, this shouldn't change anything. Also, common.o and env.o still get unconditionally built for U-boot proper. But for TPL/SPL, those two are only included if CONFIG_(SPL/TPL)_SUPPORT is set.
Any comments on this approach, apart from the spellos (I'm missing ENV_ in front of SUPPORT in two places)?
Rasmus
On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
On 15/12/2019 23.29, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
tried to fix, namely that the reference to env_flags_validate from env_htab cannot be satisfied when flags.o is not built in. However, that commit got reverted by
d90fc9c3de Revert "env: solve compilation error in SPL"
Necessary, but not sufficient conditions to see this are
CONFIG_SPL=y (obviously) CONFIG_SPL_ENV_SUPPORT=n (so flags.o does not get compiled) CONFIG_SPL_LIBCOMMON_SUPPORT=y (so env/built-in.o is part of the SPL link)
Now, these are satisfied for e.g. imx6q_logic_defconfig. But that builds just fine, and spl/u-boot-spl.map lists .data.env_htab among the discarded (garbage collected) sections. Yet, on our mpc8309-derived board, we do see the build failure, so perhaps the linker works a bit differently on ppc than on ARM, or there's yet some other configuration option needed to observe the break.
Yeah, I think this is a difference in how the linker works on ppc vs arm. Doing
git grep --files-with-matches SPL=y -- configs/ | xargs grep --files-with-matches SPL_LIBCOMMON_SUPPORT=y | xargs grep --files-without-match SPL_ENV_SUPPORT | xargs head -n1
shows that all the in-tree defconfigs with the above combination are ARM boards (except for microblaze_defconfig, but I don't have such a toolchain), and they most likely all build just fine. But taking some random PPC config (say T2080QDS_NAND_defconfig) with the first and third point, and then manually disabling SPL_ENV_SUPPORT, immediately shows the break.
For reference, I have
$ ${CROSS_COMPILE}ld --version GNU ld (GNU Binutils for Ubuntu) 2.30
Which SPL are you using on PowerPC? There's the one based CONFIG_SPL_FRAMEWORK and the one that's not. I suspect it's a framework vs not problem here rather than linker exactly.
On 10/01/2020 15.34, Tom Rini wrote:
On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
On 15/12/2019 23.29, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
Yeah, I think this is a difference in how the linker works on ppc vs arm. Doing
For reference, I have
$ ${CROSS_COMPILE}ld --version GNU ld (GNU Binutils for Ubuntu) 2.30
Which SPL are you using on PowerPC? There's the one based CONFIG_SPL_FRAMEWORK and the one that's not. I suspect it's a framework vs not problem here rather than linker exactly.
No, it's nothing to do with SPL per se. Try something completely silly such as
diff --git a/common/console.c b/common/console.c index 168ba60d0d..bbff516154 100644 --- a/common/console.c +++ b/common/console.c @@ -22,6 +22,21 @@
DECLARE_GLOBAL_DATA_PTR;
+struct foo { + int (*f)(int, int); + long a[100]; +}; +struct bar { + int a; int b; int c; +}; +extern int func9999(int a, int b); +struct foo foo9999 = { .f = func9999, }; +struct bar bar9999 = { .a = 0xa, .b = 0xb, .c = 0xc }; + +#if 0 +int func9999(int a, int b) { return a + b; } +#endif +
For an arm target, this builds just fine, and u-boot.map (and spl/u-boot-spl.map if an SPL is built) shows that foo9999 and bar9999 gets discarded just fine.
On powerpc, this fails with "undefined reference to `func9999'".
Now, changing to #if 1 instead, arm still completely eliminates both data items as well as func9999. On powerpc, the link succeeds, and bar9999 has been discarded, but both foo9999 and func9999 are present in the final image.
So it seems that the ppc linker somehow fails to eliminate unreferenced .data sections with relocation entries (it fails the same way if one changes the pointer to an "int *").
Rasmus
On 11/01/2020 23.44, Rasmus Villemoes wrote:
On 10/01/2020 15.34, Tom Rini wrote:
On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
On 15/12/2019 23.29, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
Yeah, I think this is a difference in how the linker works on ppc vs arm. Doing
For reference, I have
$ ${CROSS_COMPILE}ld --version GNU ld (GNU Binutils for Ubuntu) 2.30
Which SPL are you using on PowerPC? There's the one based CONFIG_SPL_FRAMEWORK and the one that's not. I suspect it's a framework vs not problem here rather than linker exactly.
No, it's nothing to do with SPL per se.
OK, I think I found it. On powerpc, the .fixup section contains a reference to .data.rel.env_htab, and all of .fixup is kept (because of KEEP(*(.fixup))), so env_htab cannot get garbage collected. Hence anything that section refers to must also exist. So it's not a defect in ppc-ld vs arm-ld, it's just a consequence of ppc having that .fixup section.
So, the only way to fix that is by either making sure env_flags_validate exists in the link, or avoiding having env_htab being part of the link in the first place. My patch does the latter.
Rasmus
On Sun, Jan 12, 2020 at 08:23:02PM +0000, Rasmus Villemoes wrote:
On 11/01/2020 23.44, Rasmus Villemoes wrote:
On 10/01/2020 15.34, Tom Rini wrote:
On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
On 15/12/2019 23.29, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
Yeah, I think this is a difference in how the linker works on ppc vs arm. Doing
For reference, I have
$ ${CROSS_COMPILE}ld --version GNU ld (GNU Binutils for Ubuntu) 2.30
Which SPL are you using on PowerPC? There's the one based CONFIG_SPL_FRAMEWORK and the one that's not. I suspect it's a framework vs not problem here rather than linker exactly.
No, it's nothing to do with SPL per se.
OK, I think I found it. On powerpc, the .fixup section contains a reference to .data.rel.env_htab, and all of .fixup is kept (because of KEEP(*(.fixup))), so env_htab cannot get garbage collected. Hence anything that section refers to must also exist. So it's not a defect in ppc-ld vs arm-ld, it's just a consequence of ppc having that .fixup section.
So, the only way to fix that is by either making sure env_flags_validate exists in the link, or avoiding having env_htab being part of the link in the first place. My patch does the latter.
Thanks for digging in to the root cause on this. I'll take a harder look at your patch, thanks!
On Sun, Dec 15, 2019 at 10:29:39PM +0000, Rasmus Villemoes wrote:
I'm also seeing the build failure that commit
7d4776545b env: solve compilation error in SPL
tried to fix, namely that the reference to env_flags_validate from env_htab cannot be satisfied when flags.o is not built in. However, that commit got reverted by
d90fc9c3de Revert "env: solve compilation error in SPL"
Necessary, but not sufficient conditions to see this are
CONFIG_SPL=y (obviously) CONFIG_SPL_ENV_SUPPORT=n (so flags.o does not get compiled) CONFIG_SPL_LIBCOMMON_SUPPORT=y (so env/built-in.o is part of the SPL link)
Now, these are satisfied for e.g. imx6q_logic_defconfig. But that builds just fine, and spl/u-boot-spl.map lists .data.env_htab among the discarded (garbage collected) sections. Yet, on our mpc8309-derived board, we do see the build failure, so perhaps the linker works a bit differently on ppc than on ARM, or there's yet some other configuration option needed to observe the break.
This is another attempt at solving it, which also cleans up env/Makefile a bit: Introduce a def_bool y symbol CONFIG_ENV_SUPPORT which complements CONFIG_(SPL/TPL)_SUPPORT. Then use CONFIG_$(SPL_TPL_)ENV_SUPPORT to decide whether to include the five basic env/*.o files. For attr.o, flags.o and callback.o, this shouldn't change anything. Also, common.o and env.o still get unconditionally built for U-boot proper. But for TPL/SPL, those two are only included if CONFIG_(SPL/TPL)_SUPPORT is set.
Having that symbol should also allow simplifying conditionals such as
#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
found in drivers/reset/reset-socfpga.c to just CONFIG_IS_ENABLED(ENV_SUPPORT).
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Applied to u-boot/master, thanks!
participants (2)
-
Rasmus Villemoes -
Tom Rini