* [PATCH] Kbuild: include arch/ Makefile before other directories @ 2023-06-05 6:37 Ahmad Fatoum 2023-06-13 8:04 ` Ahmad Fatoum 2023-06-13 9:16 ` Sascha Hauer 0 siblings, 2 replies; 4+ messages in thread From: Ahmad Fatoum @ 2023-06-05 6:37 UTC (permalink / raw) To: barebox; +Cc: Ahmad Fatoum Unless overridden by SORT*, LD will place sections matched by wildcards in the order they were seen in the link. So far, this meant that arch/ initcalls and device trees compiled into barebox proper, were always last. For platforms with PBL or with only one device tree in barebox proper, this didn't matter much, but when enabling the of_manipulation selftest, a second device tree would be built into barebox on kvx, openrisc and some MIPS. Because all directories appear before arch/, this had the effect that on kvx, openrisc and some MIPS, __dtb_start would end up pointing at the test's device tree instead of the board device tree breaking the build. Switching the affected platforms to use PBL would be one fix for the issue, but there's a simple one: let's do what Linux does in its top-level Kbuild file and have arch/ not be the last in sequence. This may fause fallout for code that depended on specific ordering of initcalls or other linker-defined lists, which would need to be fixed. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- While a fix, I think this needs to sit a while in next first. --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 711cba7bed29..4fdb8f1b41af 100644 --- a/Makefile +++ b/Makefile @@ -586,11 +586,12 @@ endif # We need some generic definitions. include $(srctree)/scripts/Makefile.lib -# Objects we will link into barebox / subdirs we need to visit -common-y := common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/ test/ +# Objects we will link into barebox / subdirs we need to visit include $(srctree)/arch/$(SRCARCH)/Makefile +common-y += common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/ test/ + ifdef need-config ifdef may-sync-config # Read in dependencies to all Kconfig* files, make sure to run syncconfig if -- 2.39.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Kbuild: include arch/ Makefile before other directories 2023-06-05 6:37 [PATCH] Kbuild: include arch/ Makefile before other directories Ahmad Fatoum @ 2023-06-13 8:04 ` Ahmad Fatoum 2023-06-13 9:16 ` Sascha Hauer 1 sibling, 0 replies; 4+ messages in thread From: Ahmad Fatoum @ 2023-06-13 8:04 UTC (permalink / raw) To: barebox Hi, On 05.06.23 08:37, Ahmad Fatoum wrote: > Unless overridden by SORT*, LD will place sections matched by wildcards > in the order they were seen in the link. So far, this meant that > arch/ initcalls and device trees compiled into barebox proper, were > always last. > > For platforms with PBL or with only one device tree in barebox proper, > this didn't matter much, but when enabling the of_manipulation selftest, > a second device tree would be built into barebox on kvx, openrisc and > some MIPS. > > Because all directories appear before arch/, this had the effect that > on kvx, openrisc and some MIPS, __dtb_start would end up pointing at the > test's device tree instead of the board device tree breaking the build. > > Switching the affected platforms to use PBL would be one fix for the > issue, but there's a simple one: let's do what Linux does in its > top-level Kbuild file and have arch/ not be the last in sequence. > > This may fause fallout for code that depended on specific ordering of > initcalls or other linker-defined lists, which would need to be fixed. > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > While a fix, I think this needs to sit a while in next first. Any feedback? > --- > Makefile | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 711cba7bed29..4fdb8f1b41af 100644 > --- a/Makefile > +++ b/Makefile > @@ -586,11 +586,12 @@ endif > # We need some generic definitions. > include $(srctree)/scripts/Makefile.lib > > -# Objects we will link into barebox / subdirs we need to visit > -common-y := common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/ test/ > > +# Objects we will link into barebox / subdirs we need to visit > include $(srctree)/arch/$(SRCARCH)/Makefile > > +common-y += common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/ test/ > + > ifdef need-config > ifdef may-sync-config > # Read in dependencies to all Kconfig* files, make sure to run syncconfig if -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Kbuild: include arch/ Makefile before other directories 2023-06-05 6:37 [PATCH] Kbuild: include arch/ Makefile before other directories Ahmad Fatoum 2023-06-13 8:04 ` Ahmad Fatoum @ 2023-06-13 9:16 ` Sascha Hauer 2023-06-13 10:18 ` Ahmad Fatoum 1 sibling, 1 reply; 4+ messages in thread From: Sascha Hauer @ 2023-06-13 9:16 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox On Mon, Jun 05, 2023 at 08:37:19AM +0200, Ahmad Fatoum wrote: > Unless overridden by SORT*, LD will place sections matched by wildcards > in the order they were seen in the link. So far, this meant that > arch/ initcalls and device trees compiled into barebox proper, were > always last. > > For platforms with PBL or with only one device tree in barebox proper, > this didn't matter much, but when enabling the of_manipulation selftest, > a second device tree would be built into barebox on kvx, openrisc and > some MIPS. > > Because all directories appear before arch/, this had the effect that > on kvx, openrisc and some MIPS, __dtb_start would end up pointing at the > test's device tree instead of the board device tree breaking the build. > > Switching the affected platforms to use PBL would be one fix for the > issue, but there's a simple one: let's do what Linux does in its > top-level Kbuild file and have arch/ not be the last in sequence. > > This may fause fallout for code that depended on specific ordering of > initcalls or other linker-defined lists, which would need to be fixed. I am afraid this causes regressions that take a long time to realize and to sort out. We could do something along the following lines to pick the correct dtb. This is only meant as an example, it likely needs some sugar to work correctly in all cases. Sascha ----------------------------------8<------------------------------ diff --git a/arch/mips/boot/Makefile b/arch/mips/boot/Makefile index d1e27b5e6b..e114cc5d8a 100644 --- a/arch/mips/boot/Makefile +++ b/arch/mips/boot/Makefile @@ -3,6 +3,7 @@ obj-y += start.o obj-y += main_entry.o +CFLAGS_dtb.o += -DBUILTIN_DTB_SYMBOL=__dtb_$(patsubst "%",%,$(subst -,_,$(CONFIG_BUILTIN_DTB_NAME)))_start obj-$(CONFIG_OFDEVICE) += dtb.o pbl-y += main_entry-pbl.o diff --git a/arch/mips/boot/dtb.c b/arch/mips/boot/dtb.c index ece1494e5f..3afcd20a0e 100644 --- a/arch/mips/boot/dtb.c +++ b/arch/mips/boot/dtb.c @@ -34,7 +34,7 @@ int of_add_memory_bank(struct device_node *node, bool dump, int r, return ret; } -extern char __dtb_start[]; +extern char BUILTIN_DTB_SYMBOL[]; static int of_mips_init(void) { @@ -42,7 +42,7 @@ static int of_mips_init(void) fdt = glob_fdt; if (!fdt) - fdt = __dtb_start; + fdt = BUILTIN_DTB_SYMBOL; return barebox_register_fdt(fdt); } -- 2.39.2 -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Kbuild: include arch/ Makefile before other directories 2023-06-13 9:16 ` Sascha Hauer @ 2023-06-13 10:18 ` Ahmad Fatoum 0 siblings, 0 replies; 4+ messages in thread From: Ahmad Fatoum @ 2023-06-13 10:18 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 13.06.23 11:16, Sascha Hauer wrote: > On Mon, Jun 05, 2023 at 08:37:19AM +0200, Ahmad Fatoum wrote: >> Unless overridden by SORT*, LD will place sections matched by wildcards >> in the order they were seen in the link. So far, this meant that >> arch/ initcalls and device trees compiled into barebox proper, were >> always last. >> >> For platforms with PBL or with only one device tree in barebox proper, >> this didn't matter much, but when enabling the of_manipulation selftest, >> a second device tree would be built into barebox on kvx, openrisc and >> some MIPS. >> >> Because all directories appear before arch/, this had the effect that >> on kvx, openrisc and some MIPS, __dtb_start would end up pointing at the >> test's device tree instead of the board device tree breaking the build. >> >> Switching the affected platforms to use PBL would be one fix for the >> issue, but there's a simple one: let's do what Linux does in its >> top-level Kbuild file and have arch/ not be the last in sequence. >> >> This may fause fallout for code that depended on specific ordering of >> initcalls or other linker-defined lists, which would need to be fixed. > > I am afraid this causes regressions that take a long time to realize and > to sort out. > > We could do something along the following lines to pick the correct dtb. > This is only meant as an example, it likely needs some sugar to work > correctly in all cases. Doesn't work for kvx, which doesn't set CONFIG_BUILTIN_DTB_NAME in the defconfig. I am fine with an alternative that lets me run selftests on openrisc. Cheers, Ahmad > > Sascha > > ----------------------------------8<------------------------------ > > diff --git a/arch/mips/boot/Makefile b/arch/mips/boot/Makefile > index d1e27b5e6b..e114cc5d8a 100644 > --- a/arch/mips/boot/Makefile > +++ b/arch/mips/boot/Makefile > @@ -3,6 +3,7 @@ > obj-y += start.o > obj-y += main_entry.o > > +CFLAGS_dtb.o += -DBUILTIN_DTB_SYMBOL=__dtb_$(patsubst "%",%,$(subst -,_,$(CONFIG_BUILTIN_DTB_NAME)))_start > obj-$(CONFIG_OFDEVICE) += dtb.o > > pbl-y += main_entry-pbl.o > diff --git a/arch/mips/boot/dtb.c b/arch/mips/boot/dtb.c > index ece1494e5f..3afcd20a0e 100644 > --- a/arch/mips/boot/dtb.c > +++ b/arch/mips/boot/dtb.c > @@ -34,7 +34,7 @@ int of_add_memory_bank(struct device_node *node, bool dump, int r, > return ret; > } > > -extern char __dtb_start[]; > +extern char BUILTIN_DTB_SYMBOL[]; > > static int of_mips_init(void) > { > @@ -42,7 +42,7 @@ static int of_mips_init(void) > > fdt = glob_fdt; > if (!fdt) > - fdt = __dtb_start; > + fdt = BUILTIN_DTB_SYMBOL; > > return barebox_register_fdt(fdt); > } -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-13 10:20 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-05 6:37 [PATCH] Kbuild: include arch/ Makefile before other directories Ahmad Fatoum 2023-06-13 8:04 ` Ahmad Fatoum 2023-06-13 9:16 ` Sascha Hauer 2023-06-13 10:18 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox