| Message ID | 20211014124803.v3.7.Id5595981cd99201c6a2d8b714254d775436a3483@changeid |
|---|---|
| State | Accepted |
| Commit | 262cfb5b15420a1aea465745a821e684b3dfa153 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | pxe: Refactoring to tidy up and prepare for bootflow | expand |
OK Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST Reviewed-by: Artem Lapkin <email2tema@gmail.com> Tested-by: Artem Lapkin <email2tema@gmail.com>
On Thu, Oct 14, 2021 at 9:50 PM Simon Glass <sjg@chromium.org> wrote: > > Move the header file into the main include/ directory so we can use it > from the bootmethod code. Move the C file into boot/ since it relates to > booting. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > boot/Makefile | 3 +++ > {cmd => boot}/pxe_utils.c | 0 > cmd/Makefile | 4 ++-- > cmd/sysboot.c | 2 +- > {cmd => include}/pxe_utils.h | 0 > 5 files changed, 6 insertions(+), 3 deletions(-) > rename {cmd => boot}/pxe_utils.c (100%) > rename {cmd => include}/pxe_utils.h (100%) > > diff --git a/boot/Makefile b/boot/Makefile > index a19e85cf6c8..2938c3f1458 100644 > --- a/boot/Makefile > +++ b/boot/Makefile > @@ -14,6 +14,9 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o > obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o > obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o > > +obj-$(CONFIG_CMD_PXE) += pxe_utils.o > +obj-$(CONFIG_CMD_SYSBOOT) += pxe_utils.o > + > endif > > obj-y += image.o image-board.o > diff --git a/cmd/pxe_utils.c b/boot/pxe_utils.c > similarity index 100% > rename from cmd/pxe_utils.c > rename to boot/pxe_utils.c > diff --git a/cmd/Makefile b/cmd/Makefile > index ed3669411e6..891819ae0f6 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -123,7 +123,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o > obj-$(CONFIG_CMD_PMC) += pmc.o > obj-$(CONFIG_CMD_PSTORE) += pstore.o > obj-$(CONFIG_CMD_PWM) += pwm.o > -obj-$(CONFIG_CMD_PXE) += pxe.o pxe_utils.o > +obj-$(CONFIG_CMD_PXE) += pxe.o > obj-$(CONFIG_CMD_WOL) += wol.o > obj-$(CONFIG_CMD_QFW) += qfw.o > obj-$(CONFIG_CMD_READ) += read.o > @@ -145,7 +145,7 @@ obj-$(CONFIG_CMD_SETEXPR_FMT) += printf.o > obj-$(CONFIG_CMD_SPI) += spi.o > obj-$(CONFIG_CMD_STRINGS) += strings.o > obj-$(CONFIG_CMD_SMC) += smccc.o > -obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o > +obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o > obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o > obj-$(CONFIG_CMD_TERMINAL) += terminal.o > obj-$(CONFIG_CMD_TIME) += time.o > diff --git a/cmd/sysboot.c b/cmd/sysboot.c > index 85fa5d8aa01..b81255e155a 100644 > --- a/cmd/sysboot.c > +++ b/cmd/sysboot.c > @@ -4,7 +4,7 @@ > #include <command.h> > #include <env.h> > #include <fs.h> > -#include "pxe_utils.h" > +#include <pxe_utils.h> > > static char *fs_argv[5]; > > diff --git a/cmd/pxe_utils.h b/include/pxe_utils.h > similarity index 100% > rename from cmd/pxe_utils.h > rename to include/pxe_utils.h > -- > 2.33.0.1079.g6e70778dc9-goog > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
On Thu, Oct 14, 2021 at 12:48:00PM -0600, Simon Glass wrote: > Move the header file into the main include/ directory so we can use it > from the bootmethod code. Move the C file into boot/ since it relates to > booting. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Artem Lapkin <email2tema@gmail.com> > Tested-by: Artem Lapkin <email2tema@gmail.com> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com> Applied to u-boot/master, thanks!
On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > Move the header file into the main include/ directory so we can use it > from the bootmethod code. Move the C file into boot/ since it relates to > booting. > +cc lokeshvutla@ti.com Simon, I can't explain why, but with git bisect, it appears this patch breaks my omap3_logic board (DM3730) by making it wrongly think there is 4GB of RAM, when in reality there is only 256MB. We have both 256MB and 512MB parts, and the automatic memory detection has always 'just worked' in the past. With this patch now, I see: U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit DRAM: 4 GiB <hang> With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe global"), it properly detects the RAM and fully boots. U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit DRAM: 256 MiB NAND: 512 MiB MMC: OMAP SD/MMC: 0 Loading Environment from NAND... OK OMAP die ID: 619e00029ff800000168300f1502501f Net: eth0: ethernet@08000000 Hit any key to stop autoboot: 0 OMAP Logic # I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all defined, so I am having a hard time understanding why this would change behavior or stomp on the the structure that knows the memory size. If I jump ahead to the current 'master' 531c0089457:("Merge branch '2022-02-08-TI-platform-updates') and revert this patch, my board boots correctly again, but I am struggling to understand why. Do you have any suggestions for me to try? thanks, adam > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > boot/Makefile | 3 +++ > {cmd => boot}/pxe_utils.c | 0 > cmd/Makefile | 4 ++-- > cmd/sysboot.c | 2 +- > {cmd => include}/pxe_utils.h | 0 > 5 files changed, 6 insertions(+), 3 deletions(-) > rename {cmd => boot}/pxe_utils.c (100%) > rename {cmd => include}/pxe_utils.h (100%) > > diff --git a/boot/Makefile b/boot/Makefile > index a19e85cf6c8..2938c3f1458 100644 > --- a/boot/Makefile > +++ b/boot/Makefile > @@ -14,6 +14,9 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o > obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o > obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o > > +obj-$(CONFIG_CMD_PXE) += pxe_utils.o > +obj-$(CONFIG_CMD_SYSBOOT) += pxe_utils.o > + > endif > > obj-y += image.o image-board.o > diff --git a/cmd/pxe_utils.c b/boot/pxe_utils.c > similarity index 100% > rename from cmd/pxe_utils.c > rename to boot/pxe_utils.c > diff --git a/cmd/Makefile b/cmd/Makefile > index ed3669411e6..891819ae0f6 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -123,7 +123,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o > obj-$(CONFIG_CMD_PMC) += pmc.o > obj-$(CONFIG_CMD_PSTORE) += pstore.o > obj-$(CONFIG_CMD_PWM) += pwm.o > -obj-$(CONFIG_CMD_PXE) += pxe.o pxe_utils.o > +obj-$(CONFIG_CMD_PXE) += pxe.o > obj-$(CONFIG_CMD_WOL) += wol.o > obj-$(CONFIG_CMD_QFW) += qfw.o > obj-$(CONFIG_CMD_READ) += read.o > @@ -145,7 +145,7 @@ obj-$(CONFIG_CMD_SETEXPR_FMT) += printf.o > obj-$(CONFIG_CMD_SPI) += spi.o > obj-$(CONFIG_CMD_STRINGS) += strings.o > obj-$(CONFIG_CMD_SMC) += smccc.o > -obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o > +obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o > obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o > obj-$(CONFIG_CMD_TERMINAL) += terminal.o > obj-$(CONFIG_CMD_TIME) += time.o > diff --git a/cmd/sysboot.c b/cmd/sysboot.c > index 85fa5d8aa01..b81255e155a 100644 > --- a/cmd/sysboot.c > +++ b/cmd/sysboot.c > @@ -4,7 +4,7 @@ > #include <command.h> > #include <env.h> > #include <fs.h> > -#include "pxe_utils.h" > +#include <pxe_utils.h> > > static char *fs_argv[5]; > > diff --git a/cmd/pxe_utils.h b/include/pxe_utils.h > similarity index 100% > rename from cmd/pxe_utils.h > rename to include/pxe_utils.h > -- > 2.33.0.1079.g6e70778dc9-goog >
On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > Move the header file into the main include/ directory so we can use it > > from the bootmethod code. Move the C file into boot/ since it relates to > > booting. > > > +cc lokeshvutla@ti.com > > Simon, > > I can't explain why, but with git bisect, it appears this patch breaks > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > of RAM, when in reality there is only 256MB. We have both 256MB and > 512MB parts, and the automatic memory detection has always 'just > worked' in the past. > > With this patch now, I see: > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > DRAM: 4 GiB > <hang> > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > global"), it properly detects the RAM and fully boots. > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > DRAM: 256 MiB > NAND: 512 MiB > MMC: OMAP SD/MMC: 0 > Loading Environment from NAND... OK > OMAP die ID: 619e00029ff800000168300f1502501f > Net: eth0: ethernet@08000000 > Hit any key to stop autoboot: 0 > OMAP Logic # > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > defined, so I am having a hard time understanding why this would > change behavior or stomp on the the structure that knows the memory > size. > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > '2022-02-08-TI-platform-updates') and revert this patch, my board > boots correctly again, but I am struggling to understand why. > > Do you have any suggestions for me to try? I would suggest objdump disassemble U-Boot before/after and see what functions have changed.
Hi, On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > Move the header file into the main include/ directory so we can use it > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > booting. > > > > > +cc lokeshvutla@ti.com > > > > Simon, > > > > I can't explain why, but with git bisect, it appears this patch breaks > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > of RAM, when in reality there is only 256MB. We have both 256MB and > > 512MB parts, and the automatic memory detection has always 'just > > worked' in the past. > > > > With this patch now, I see: > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > DRAM: 4 GiB > > <hang> > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > global"), it properly detects the RAM and fully boots. > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > DRAM: 256 MiB > > NAND: 512 MiB > > MMC: OMAP SD/MMC: 0 > > Loading Environment from NAND... OK > > OMAP die ID: 619e00029ff800000168300f1502501f > > Net: eth0: ethernet@08000000 > > Hit any key to stop autoboot: 0 > > OMAP Logic # > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > defined, so I am having a hard time understanding why this would > > change behavior or stomp on the the structure that knows the memory > > size. > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > boots correctly again, but I am struggling to understand why. > > > > Do you have any suggestions for me to try? > > I would suggest objdump disassemble U-Boot before/after and see what > functions have changed. Keep an eye out for a BSS variable that is used before relocation, perhaps? Regards, Simon
On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > booting. > > > > > > > +cc lokeshvutla@ti.com > > > > > > Simon, > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > 512MB parts, and the automatic memory detection has always 'just > > > worked' in the past. > > > > > > With this patch now, I see: > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > DRAM: 4 GiB > > > <hang> > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > global"), it properly detects the RAM and fully boots. > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > DRAM: 256 MiB > > > NAND: 512 MiB > > > MMC: OMAP SD/MMC: 0 > > > Loading Environment from NAND... OK > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > Net: eth0: ethernet@08000000 > > > Hit any key to stop autoboot: 0 > > > OMAP Logic # > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > defined, so I am having a hard time understanding why this would > > > change behavior or stomp on the the structure that knows the memory > > > size. > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > boots correctly again, but I am struggling to understand why. + Marek Behún > > > > > > Do you have any suggestions for me to try? > > > > I would suggest objdump disassemble U-Boot before/after and see what > > functions have changed. > > Keep an eye out for a BSS variable that is used before relocation, perhaps? I am still investigating, but disabling LTO appears to fix the issue for me. I'd like to keep LTO, so I'm going to attempt to focus on the differences in the affected functions and how this patch makes LTO behave differently. The disassembly of U-Boot is large, so it's going to take me a bit of time to investigate. If someone has any LTO-related suggestions that I could try, I'd be open to try them too. adam > > Regards, > Simon
On Thu, Feb 10, 2022 at 7:56 AM Adam Ford <aford173@gmail.com> wrote: > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > Hi, > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > booting. > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > Simon, > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > 512MB parts, and the automatic memory detection has always 'just > > > > worked' in the past. > > > > > > > > With this patch now, I see: > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > DRAM: 4 GiB > > > > <hang> > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > DRAM: 256 MiB > > > > NAND: 512 MiB > > > > MMC: OMAP SD/MMC: 0 > > > > Loading Environment from NAND... OK > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > Net: eth0: ethernet@08000000 > > > > Hit any key to stop autoboot: 0 > > > > OMAP Logic # > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > defined, so I am having a hard time understanding why this would > > > > change behavior or stomp on the the structure that knows the memory > > > > size. > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > boots correctly again, but I am struggling to understand why. > + Marek Behún > > > > > > > > > Do you have any suggestions for me to try? > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > functions have changed. > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > I am still investigating, but disabling LTO appears to fix the issue > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > differences in the affected functions and how this patch makes LTO > behave differently. > > The disassembly of U-Boot is large, so it's going to take me a bit of > time to investigate. If someone has any LTO-related suggestions that > I could try, I'd be open to try them too. > > adam > > > > Regards, > > Simon
Hi Adam, On Thu, 10 Feb 2022 at 06:57, Adam Ford <aford173@gmail.com> wrote: > > On Thu, Feb 10, 2022 at 7:56 AM Adam Ford <aford173@gmail.com> wrote: > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi, > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > booting. > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > Simon, > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > worked' in the past. > > > > > > > > > > With this patch now, I see: > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 4 GiB > > > > > <hang> > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 256 MiB > > > > > NAND: 512 MiB > > > > > MMC: OMAP SD/MMC: 0 > > > > > Loading Environment from NAND... OK > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > Net: eth0: ethernet@08000000 > > > > > Hit any key to stop autoboot: 0 > > > > > OMAP Logic # > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > defined, so I am having a hard time understanding why this would > > > > > change behavior or stomp on the the structure that knows the memory > > > > > size. > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > boots correctly again, but I am struggling to understand why. > > + Marek Behún > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > functions have changed. > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > I am still investigating, but disabling LTO appears to fix the issue > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > differences in the affected functions and how this patch makes LTO > > behave differently. > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > time to investigate. If someone has any LTO-related suggestions that > > I could try, I'd be open to try them too. Another thing that might help is to put your revert in a branch and then: buildman -b <branch> <board> then buildman -b <branch> <board> -sB which will show function changes. But more directly, the DRAM calculation should be something you can print out and debug (perhaps with DEBUG_UART) and see where it is going wrong. Regards, Simon
On Thu, Feb 10, 2022 at 8:32 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Adam, > > On Thu, 10 Feb 2022 at 06:57, Adam Ford <aford173@gmail.com> wrote: > > > > On Thu, Feb 10, 2022 at 7:56 AM Adam Ford <aford173@gmail.com> wrote: > > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi, > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > booting. > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > Simon, > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > worked' in the past. > > > > > > > > > > > > With this patch now, I see: > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > DRAM: 4 GiB > > > > > > <hang> > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > DRAM: 256 MiB > > > > > > NAND: 512 MiB > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > Loading Environment from NAND... OK > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > Net: eth0: ethernet@08000000 > > > > > > Hit any key to stop autoboot: 0 > > > > > > OMAP Logic # > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > defined, so I am having a hard time understanding why this would > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > size. > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > boots correctly again, but I am struggling to understand why. > > > + Marek Behún > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > functions have changed. > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > differences in the affected functions and how this patch makes LTO > > > behave differently. > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > time to investigate. If someone has any LTO-related suggestions that > > > I could try, I'd be open to try them too. > > Another thing that might help is to put your revert in a branch and then: > > buildman -b <branch> <board> > > then > > buildman -b <branch> <board> -sB > > which will show function changes. > > But more directly, the DRAM calculation should be something you can > print out and debug (perhaps with DEBUG_UART) and see where it is > going wrong. I have some meetings, but I'll try that this weekend. I was able to narrow the specific file to pxe_utils.o. If I add 1 line to the Makefile, the issue goes away: CFLAGS_REMOVE_pxe_utils.o := $(LTO_CFLAGS) Would this be acceptable? I haven't tested this, but I am guessing any OMAP3 board with LTO would be affected (maybe others), and I have 5. :-( adam > > Regards, > Simon
On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > Hi, > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > booting. > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > Simon, > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > 512MB parts, and the automatic memory detection has always 'just > > > > worked' in the past. > > > > > > > > With this patch now, I see: > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > DRAM: 4 GiB > > > > <hang> > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > DRAM: 256 MiB > > > > NAND: 512 MiB > > > > MMC: OMAP SD/MMC: 0 > > > > Loading Environment from NAND... OK > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > Net: eth0: ethernet@08000000 > > > > Hit any key to stop autoboot: 0 > > > > OMAP Logic # > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > defined, so I am having a hard time understanding why this would > > > > change behavior or stomp on the the structure that knows the memory > > > > size. > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > boots correctly again, but I am struggling to understand why. > + Marek Behún > > > > > > > > > Do you have any suggestions for me to try? > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > functions have changed. > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > I am still investigating, but disabling LTO appears to fix the issue > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > differences in the affected functions and how this patch makes LTO > behave differently. > > The disassembly of U-Boot is large, so it's going to take me a bit of > time to investigate. If someone has any LTO-related suggestions that > I could try, I'd be open to try them too. Wait, the disassembly is large, or the differences between the disassembly, before/after this change alone, are large? It's feeling like just how we remove the LTO flags from arch/arm/mach-omap2/omap3/board.o something else might also need that, OR digging more to find out issue LTO is exposing in terms of variables being in the data and not BSS section and needing initialization or similar.
On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi, > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > booting. > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > Simon, > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > worked' in the past. > > > > > > > > > > With this patch now, I see: > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 4 GiB > > > > > <hang> > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 256 MiB > > > > > NAND: 512 MiB > > > > > MMC: OMAP SD/MMC: 0 > > > > > Loading Environment from NAND... OK > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > Net: eth0: ethernet@08000000 > > > > > Hit any key to stop autoboot: 0 > > > > > OMAP Logic # > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > defined, so I am having a hard time understanding why this would > > > > > change behavior or stomp on the the structure that knows the memory > > > > > size. > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > boots correctly again, but I am struggling to understand why. > > + Marek Behún > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > functions have changed. > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > I am still investigating, but disabling LTO appears to fix the issue > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > differences in the affected functions and how this patch makes LTO > > behave differently. > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > time to investigate. If someone has any LTO-related suggestions that > > I could try, I'd be open to try them too. > > Wait, the disassembly is large, or the differences between the > disassembly, before/after this change alone, are large? It's feeling I will be the first to admit thatI am not very good with the assembly side of things, but this is what I did: git checkout master make CROSS_COMPILE=arm-linux-gnueabihf- -j8 arm-linux-gnueabihf-objdump -S u-boot > broken.dump git revert 262cfb5b15420a1aea465745a821e684b3dfa153 make CROSS_COMPILE=arm-linux-gnueabihf- -j8 arm-linux-gnueabihf-objdump -S u-boot > working.dump diff --side-by-side --suppress-common-lines broken.dump working.dump > broken-working.diff cat -n broken-working.diff The broken-working.diff file with common lines suppressed is 236256 lines long. When I disable LTO for just pxe_utils.o and redo the same exercise, the diff file with common-lines removed is 266573 lines long. Maybe I am not using objdump correctly. I am not all that familiar with this code either, so I am not sure which variables should be in BSS. I did a search in both working and non-working dumps to look for keyworks like BSS, but from what I can tell, both have similar functions: gd->mon_len = (ulong)&__bss_end - (ulong)_start; /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len = (ulong)&__bss_end - (ulong)_start; * reserve memory for U-Boot code, data & bss 8011051a <clear_bss>: #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) CLEAR_BSS #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) CLEAR_BSS CLEAR_BSS When I grepped for mon_len, both sets of dumps looked nearly identical: aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); 80112724 <setup_mon_len>: static int setup_mon_len(void) gd->mon_len = (ulong)&__bss_end - (ulong)_start; 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len = (ulong)&__bss_end - (ulong)_start; gd->ram_top = board_get_usable_ram_top(gd->mon_len); gd->relocaddr -= gd->mon_len; gd->mon_len >> 10, gd->relocaddr); ip = mon_lengths[yleap]; aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); 80110398 <setup_mon_len>: static int setup_mon_len(void) gd->mon_len = (ulong)&__bss_end - (ulong)_start; 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len = (ulong)&__bss_end - (ulong)_start; gd->ram_top = board_get_usable_ram_top(gd->mon_len); gd->relocaddr -= gd->mon_len; gd->mon_len >> 10, gd->relocaddr); ip = mon_lengths[yleap]; aford@aford-OptiPlex-7050:~/src/u-boot$ Since I think I narrowed it down to the pxe_utils.o file, I thought I'd do an objdump of both the working and non-working versions of pxe_utils.o and this is where it got interesting. With LTO building pxe_utils.o, the dump looks empty: arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump cat pxe-notworking.dump boot/pxe_utils.o: file format elf32-littlearm ^-- no actual code dump If I take the working version of this same file without LTO enabled and do a dump, and it's 2291 lines long and full of functions. I tried adding some __used to the non-static function names, but that didn't appear to make any difference to the objdump of pxe_utils.o adam > like just how we remove the LTO flags from > arch/arm/mach-omap2/omap3/board.o something else might also need that, > OR digging more to find out issue LTO is exposing in terms of variables > being in the data and not BSS section and needing initialization or > similar. > > -- > Tom
On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi, > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > booting. > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > Simon, > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > worked' in the past. > > > > > > > > > > > > With this patch now, I see: > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > DRAM: 4 GiB > > > > > > <hang> > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > DRAM: 256 MiB > > > > > > NAND: 512 MiB > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > Loading Environment from NAND... OK > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > Net: eth0: ethernet@08000000 > > > > > > Hit any key to stop autoboot: 0 > > > > > > OMAP Logic # > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > defined, so I am having a hard time understanding why this would > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > size. > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > boots correctly again, but I am struggling to understand why. > > > + Marek Behún > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > functions have changed. > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > differences in the affected functions and how this patch makes LTO > > > behave differently. > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > time to investigate. If someone has any LTO-related suggestions that > > > I could try, I'd be open to try them too. > > > > Wait, the disassembly is large, or the differences between the > > disassembly, before/after this change alone, are large? It's feeling > > I will be the first to admit thatI am not very good with the assembly > side of things, but this is what I did: > > git checkout master > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > broken-working.diff > cat -n broken-working.diff > > The broken-working.diff file with common lines suppressed is 236256 lines long. OK, I just use '-d' and not '-S', which might help a little bit. But you're probably going to still need to edit the dumps and just globally change all of the addresses to 'XXXXXXXX' so that you'll end up hopefully only seeing where functions were optimized differently. But it might well end up being a bit trickier than that. > When I disable LTO for just pxe_utils.o and redo the same exercise, > the diff file with common-lines removed is 266573 lines long. > > Maybe I am not using objdump correctly. I am not all that familiar > with this code either, so I am not sure which variables should be in > BSS. I did a search in both working and non-working dumps to look for > keyworks like BSS, but from what I can tell, both have similar > functions: > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > * reserve memory for U-Boot code, data & bss > 8011051a <clear_bss>: > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > CLEAR_BSS > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > CLEAR_BSS > CLEAR_BSS > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > 80112724 <setup_mon_len>: > static int setup_mon_len(void) > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > gd->relocaddr -= gd->mon_len; > gd->mon_len >> 10, gd->relocaddr); > ip = mon_lengths[yleap]; > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > 80110398 <setup_mon_len>: > static int setup_mon_len(void) > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > gd->relocaddr -= gd->mon_len; > gd->mon_len >> 10, gd->relocaddr); > ip = mon_lengths[yleap]; > aford@aford-OptiPlex-7050:~/src/u-boot$ > > Since I think I narrowed it down to the pxe_utils.o file, I thought > I'd do an objdump of both the working and non-working versions of > pxe_utils.o and this is where it got interesting. > > With LTO building pxe_utils.o, the dump looks empty: > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > cat pxe-notworking.dump > > boot/pxe_utils.o: file format elf32-littlearm > > ^-- no actual code dump > If I take the working version of this same file without LTO enabled > and do a dump, and it's 2291 lines long and full of functions. > > I tried adding some __used to the non-static function names, but that > didn't appear to make any difference to the objdump of pxe_utils.o I feel like it can't be pxe_utils.o itself but rather how LTO is behaving before/after that change and sorting the object files differently. If modifying the dumps like I suggested above doesn't lead to more clues, and it doesn't seem to matter what toolchain is used (are you using the gcc-11 from kernel.org that we use in docker and buildman?), I'll try and look as well.
On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > booting. > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > worked' in the past. > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > DRAM: 4 GiB > > > > > > > <hang> > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > DRAM: 256 MiB > > > > > > > NAND: 512 MiB > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > Loading Environment from NAND... OK > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > OMAP Logic # > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > size. > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > boots correctly again, but I am struggling to understand why. > > > > + Marek Behún > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > functions have changed. > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > differences in the affected functions and how this patch makes LTO > > > > behave differently. > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > time to investigate. If someone has any LTO-related suggestions that > > > > I could try, I'd be open to try them too. > > > > > > Wait, the disassembly is large, or the differences between the > > > disassembly, before/after this change alone, are large? It's feeling > > > > I will be the first to admit thatI am not very good with the assembly > > side of things, but this is what I did: > > > > git checkout master > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > broken-working.diff > > cat -n broken-working.diff > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > OK, I just use '-d' and not '-S', which might help a little bit. But > you're probably going to still need to edit the dumps and just globally > change all of the addresses to 'XXXXXXXX' so that you'll end up > hopefully only seeing where functions were optimized differently. But > it might well end up being a bit trickier than that. It looks like none of the object files are showing any content with objdump when LTO is enabled. With a little google search, it appears we need lto-dump. I have some more meetings, but I'll try to spend some more time on it this weekend. > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > the diff file with common-lines removed is 266573 lines long. > > > > Maybe I am not using objdump correctly. I am not all that familiar > > with this code either, so I am not sure which variables should be in > > BSS. I did a search in both working and non-working dumps to look for > > keyworks like BSS, but from what I can tell, both have similar > > functions: > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > * reserve memory for U-Boot code, data & bss > > 8011051a <clear_bss>: > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > CLEAR_BSS > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > CLEAR_BSS > > CLEAR_BSS > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > 80112724 <setup_mon_len>: > > static int setup_mon_len(void) > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > gd->relocaddr -= gd->mon_len; > > gd->mon_len >> 10, gd->relocaddr); > > ip = mon_lengths[yleap]; > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > 80110398 <setup_mon_len>: > > static int setup_mon_len(void) > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > gd->relocaddr -= gd->mon_len; > > gd->mon_len >> 10, gd->relocaddr); > > ip = mon_lengths[yleap]; > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > I'd do an objdump of both the working and non-working versions of > > pxe_utils.o and this is where it got interesting. > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > cat pxe-notworking.dump > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > ^-- no actual code dump > > If I take the working version of this same file without LTO enabled > > and do a dump, and it's 2291 lines long and full of functions. > > > > I tried adding some __used to the non-static function names, but that > > didn't appear to make any difference to the objdump of pxe_utils.o > > I feel like it can't be pxe_utils.o itself but rather how LTO is > behaving before/after that change and sorting the object files > differently. If modifying the dumps like I suggested above doesn't lead That's what I was thinking too. > to more clues, and it doesn't seem to matter what toolchain is used (are > you using the gcc-11 from kernel.org that we use in docker and > buildman?), I'll try and look as well. I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > -- > Tom
On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > > booting. > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > DRAM: 4 GiB > > > > > > > > <hang> > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > DRAM: 256 MiB > > > > > > > > NAND: 512 MiB > > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > > Loading Environment from NAND... OK > > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > > OMAP Logic # > > > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > > size. > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > > boots correctly again, but I am struggling to understand why. > > > > > + Marek Behún > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > > functions have changed. > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > > differences in the affected functions and how this patch makes LTO > > > > > behave differently. > > > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > > time to investigate. If someone has any LTO-related suggestions that > > > > > I could try, I'd be open to try them too. > > > > > > > > Wait, the disassembly is large, or the differences between the > > > > disassembly, before/after this change alone, are large? It's feeling > > > > > > I will be the first to admit thatI am not very good with the assembly > > > side of things, but this is what I did: > > > > > > git checkout master > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > > broken-working.diff > > > cat -n broken-working.diff > > > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > > > OK, I just use '-d' and not '-S', which might help a little bit. But > > you're probably going to still need to edit the dumps and just globally > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > hopefully only seeing where functions were optimized differently. But > > it might well end up being a bit trickier than that. > > It looks like none of the object files are showing any content with > objdump when LTO is enabled. With a little google search, it appears > we need lto-dump. I have some more meetings, but I'll try to spend > some more time on it this weekend. > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > > the diff file with common-lines removed is 266573 lines long. > > > > > > Maybe I am not using objdump correctly. I am not all that familiar > > > with this code either, so I am not sure which variables should be in > > > BSS. I did a search in both working and non-working dumps to look for > > > keyworks like BSS, but from what I can tell, both have similar > > > functions: > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > * reserve memory for U-Boot code, data & bss > > > 8011051a <clear_bss>: > > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > > CLEAR_BSS > > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > > CLEAR_BSS > > > CLEAR_BSS > > > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > 80112724 <setup_mon_len>: > > > static int setup_mon_len(void) > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > gd->relocaddr -= gd->mon_len; > > > gd->mon_len >> 10, gd->relocaddr); > > > ip = mon_lengths[yleap]; > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > 80110398 <setup_mon_len>: > > > static int setup_mon_len(void) > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > gd->relocaddr -= gd->mon_len; > > > gd->mon_len >> 10, gd->relocaddr); > > > ip = mon_lengths[yleap]; > > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > > I'd do an objdump of both the working and non-working versions of > > > pxe_utils.o and this is where it got interesting. > > > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > > cat pxe-notworking.dump > > > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > > > ^-- no actual code dump > > > If I take the working version of this same file without LTO enabled > > > and do a dump, and it's 2291 lines long and full of functions. > > > > > > I tried adding some __used to the non-static function names, but that > > > didn't appear to make any difference to the objdump of pxe_utils.o > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > behaving before/after that change and sorting the object files > > differently. If modifying the dumps like I suggested above doesn't lead > > That's what I was thinking too. > > > to more clues, and it doesn't seem to matter what toolchain is used (are > > you using the gcc-11 from kernel.org that we use in docker and > > buildman?), I'll try and look as well. > > I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: > > Thread model: posix > Supported LTO compression algorithms: zlib zstd > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) OK. FWIW, if it's easier to build and test, I would suggest also trying CFLAGS_REMOVE_xxx.o := $(LTO_CFLAGS) for all of the obj files under arch/arm/ and board/ and then if that also works correctly, re-adding the flags a directory, then file, at a time until you've narrowed it down.
On Fri, Feb 11, 2022 at 10:44 AM Tom Rini <trini@konsulko.com> wrote: > > On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > > > booting. > > > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > DRAM: 4 GiB > > > > > > > > > <hang> > > > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > DRAM: 256 MiB > > > > > > > > > NAND: 512 MiB > > > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > > > Loading Environment from NAND... OK > > > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > > > OMAP Logic # > > > > > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > > > size. > > > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > > > boots correctly again, but I am struggling to understand why. > > > > > > + Marek Behún > > > > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > > > functions have changed. > > > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > > > differences in the affected functions and how this patch makes LTO > > > > > > behave differently. > > > > > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > > > time to investigate. If someone has any LTO-related suggestions that > > > > > > I could try, I'd be open to try them too. > > > > > > > > > > Wait, the disassembly is large, or the differences between the > > > > > disassembly, before/after this change alone, are large? It's feeling > > > > > > > > I will be the first to admit thatI am not very good with the assembly > > > > side of things, but this is what I did: > > > > > > > > git checkout master > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > > > broken-working.diff > > > > cat -n broken-working.diff > > > > > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > > > > > OK, I just use '-d' and not '-S', which might help a little bit. But > > > you're probably going to still need to edit the dumps and just globally > > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > > hopefully only seeing where functions were optimized differently. But > > > it might well end up being a bit trickier than that. > > > > It looks like none of the object files are showing any content with > > objdump when LTO is enabled. With a little google search, it appears > > we need lto-dump. I have some more meetings, but I'll try to spend > > some more time on it this weekend. > > > > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > > > the diff file with common-lines removed is 266573 lines long. > > > > > > > > Maybe I am not using objdump correctly. I am not all that familiar > > > > with this code either, so I am not sure which variables should be in > > > > BSS. I did a search in both working and non-working dumps to look for > > > > keyworks like BSS, but from what I can tell, both have similar > > > > functions: > > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > * reserve memory for U-Boot code, data & bss > > > > 8011051a <clear_bss>: > > > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > > > CLEAR_BSS > > > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > > > CLEAR_BSS > > > > CLEAR_BSS > > > > > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > 80112724 <setup_mon_len>: > > > > static int setup_mon_len(void) > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > gd->relocaddr -= gd->mon_len; > > > > gd->mon_len >> 10, gd->relocaddr); > > > > ip = mon_lengths[yleap]; > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > 80110398 <setup_mon_len>: > > > > static int setup_mon_len(void) > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > gd->relocaddr -= gd->mon_len; > > > > gd->mon_len >> 10, gd->relocaddr); > > > > ip = mon_lengths[yleap]; > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > > > I'd do an objdump of both the working and non-working versions of > > > > pxe_utils.o and this is where it got interesting. > > > > > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > > > cat pxe-notworking.dump > > > > > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > > > > > ^-- no actual code dump > > > > If I take the working version of this same file without LTO enabled > > > > and do a dump, and it's 2291 lines long and full of functions. > > > > > > > > I tried adding some __used to the non-static function names, but that > > > > didn't appear to make any difference to the objdump of pxe_utils.o > > > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > > behaving before/after that change and sorting the object files > > > differently. If modifying the dumps like I suggested above doesn't lead > > > > That's what I was thinking too. > > > > > to more clues, and it doesn't seem to matter what toolchain is used (are > > > you using the gcc-11 from kernel.org that we use in docker and > > > buildman?), I'll try and look as well. > > > > I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: > > > > Thread model: posix > > Supported LTO compression algorithms: zlib zstd > > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > OK. FWIW, if it's easier to build and test, I would suggest also trying > CFLAGS_REMOVE_xxx.o := $(LTO_CFLAGS) > for all of the obj files under arch/arm/ and board/ and then if that > also works correctly, re-adding the flags a directory, then file, at a > time until you've narrowed it down. If I'm understanding this correctly, does this mean that you think it's the stuff that's calling the pxe_utils.o and not the pxe_utils.o itself? Doing the CFLAGS_REMOVE on the pxe_utils.o fixes the issue. adam > > -- > Tom
On Fri, Feb 11, 2022 at 11:10:43AM -0600, Adam Ford wrote: > On Fri, Feb 11, 2022 at 10:44 AM Tom Rini <trini@konsulko.com> wrote: > > > > On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > > > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > > > > booting. > > > > > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > DRAM: 4 GiB > > > > > > > > > > <hang> > > > > > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > DRAM: 256 MiB > > > > > > > > > > NAND: 512 MiB > > > > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > > > > Loading Environment from NAND... OK > > > > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > > > > OMAP Logic # > > > > > > > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > > > > size. > > > > > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > > > > boots correctly again, but I am struggling to understand why. > > > > > > > + Marek Behún > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > > > > functions have changed. > > > > > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > > > > differences in the affected functions and how this patch makes LTO > > > > > > > behave differently. > > > > > > > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > > > > time to investigate. If someone has any LTO-related suggestions that > > > > > > > I could try, I'd be open to try them too. > > > > > > > > > > > > Wait, the disassembly is large, or the differences between the > > > > > > disassembly, before/after this change alone, are large? It's feeling > > > > > > > > > > I will be the first to admit thatI am not very good with the assembly > > > > > side of things, but this is what I did: > > > > > > > > > > git checkout master > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > > > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > > > > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > > > > broken-working.diff > > > > > cat -n broken-working.diff > > > > > > > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > > > > > > > OK, I just use '-d' and not '-S', which might help a little bit. But > > > > you're probably going to still need to edit the dumps and just globally > > > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > > > hopefully only seeing where functions were optimized differently. But > > > > it might well end up being a bit trickier than that. > > > > > > It looks like none of the object files are showing any content with > > > objdump when LTO is enabled. With a little google search, it appears > > > we need lto-dump. I have some more meetings, but I'll try to spend > > > some more time on it this weekend. > > > > > > > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > > > > the diff file with common-lines removed is 266573 lines long. > > > > > > > > > > Maybe I am not using objdump correctly. I am not all that familiar > > > > > with this code either, so I am not sure which variables should be in > > > > > BSS. I did a search in both working and non-working dumps to look for > > > > > keyworks like BSS, but from what I can tell, both have similar > > > > > functions: > > > > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > * reserve memory for U-Boot code, data & bss > > > > > 8011051a <clear_bss>: > > > > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > > > > CLEAR_BSS > > > > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > > > > CLEAR_BSS > > > > > CLEAR_BSS > > > > > > > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > 80112724 <setup_mon_len>: > > > > > static int setup_mon_len(void) > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > > > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > gd->relocaddr -= gd->mon_len; > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > ip = mon_lengths[yleap]; > > > > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > 80110398 <setup_mon_len>: > > > > > static int setup_mon_len(void) > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > > > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > gd->relocaddr -= gd->mon_len; > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > ip = mon_lengths[yleap]; > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > > > > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > > > > I'd do an objdump of both the working and non-working versions of > > > > > pxe_utils.o and this is where it got interesting. > > > > > > > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > > > > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > > > > cat pxe-notworking.dump > > > > > > > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > > > > > > > ^-- no actual code dump > > > > > If I take the working version of this same file without LTO enabled > > > > > and do a dump, and it's 2291 lines long and full of functions. > > > > > > > > > > I tried adding some __used to the non-static function names, but that > > > > > didn't appear to make any difference to the objdump of pxe_utils.o > > > > > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > > > behaving before/after that change and sorting the object files > > > > differently. If modifying the dumps like I suggested above doesn't lead > > > > > > That's what I was thinking too. > > > > > > > to more clues, and it doesn't seem to matter what toolchain is used (are > > > > you using the gcc-11 from kernel.org that we use in docker and > > > > buildman?), I'll try and look as well. > > > > > > I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: > > > > > > Thread model: posix > > > Supported LTO compression algorithms: zlib zstd > > > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > > > OK. FWIW, if it's easier to build and test, I would suggest also trying > > CFLAGS_REMOVE_xxx.o := $(LTO_CFLAGS) > > for all of the obj files under arch/arm/ and board/ and then if that > > also works correctly, re-adding the flags a directory, then file, at a > > time until you've narrowed it down. > > If I'm understanding this correctly, does this mean that you think > it's the stuff that's calling the pxe_utils.o and not the pxe_utils.o > itself? > Doing the CFLAGS_REMOVE on the pxe_utils.o fixes the issue. I think that the change here caused LTO to shuffle files around when linking the resulting binary and exposed a previously existing issue. The closer to root cause is some other early object, likely related to DDR detection, is doing something "special" and failing under LTO. Not LTO'ing that object would be a starting point to seeing what a more proper root fix is.
On Fri, Feb 11, 2022 at 11:13 AM Tom Rini <trini@konsulko.com> wrote: > > On Fri, Feb 11, 2022 at 11:10:43AM -0600, Adam Ford wrote: > > On Fri, Feb 11, 2022 at 10:44 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > > > > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > > > > > booting. > > > > > > > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > > DRAM: 4 GiB > > > > > > > > > > > <hang> > > > > > > > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > > DRAM: 256 MiB > > > > > > > > > > > NAND: 512 MiB > > > > > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > > > > > Loading Environment from NAND... OK > > > > > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > > > > > OMAP Logic # > > > > > > > > > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > > > > > size. > > > > > > > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > > > > > boots correctly again, but I am struggling to understand why. > > > > > > > > + Marek Behún > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > > > > > functions have changed. > > > > > > > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > > > > > differences in the affected functions and how this patch makes LTO > > > > > > > > behave differently. > > > > > > > > > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > > > > > time to investigate. If someone has any LTO-related suggestions that > > > > > > > > I could try, I'd be open to try them too. > > > > > > > > > > > > > > Wait, the disassembly is large, or the differences between the > > > > > > > disassembly, before/after this change alone, are large? It's feeling > > > > > > > > > > > > I will be the first to admit thatI am not very good with the assembly > > > > > > side of things, but this is what I did: > > > > > > > > > > > > git checkout master > > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > > > > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > > > > > > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > > > > > broken-working.diff > > > > > > cat -n broken-working.diff > > > > > > > > > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > > > > > > > > > OK, I just use '-d' and not '-S', which might help a little bit. But > > > > > you're probably going to still need to edit the dumps and just globally > > > > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > > > > hopefully only seeing where functions were optimized differently. But > > > > > it might well end up being a bit trickier than that. > > > > > > > > It looks like none of the object files are showing any content with > > > > objdump when LTO is enabled. With a little google search, it appears > > > > we need lto-dump. I have some more meetings, but I'll try to spend > > > > some more time on it this weekend. > > > > > > > > > > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > > > > > the diff file with common-lines removed is 266573 lines long. > > > > > > > > > > > > Maybe I am not using objdump correctly. I am not all that familiar > > > > > > with this code either, so I am not sure which variables should be in > > > > > > BSS. I did a search in both working and non-working dumps to look for > > > > > > keyworks like BSS, but from what I can tell, both have similar > > > > > > functions: > > > > > > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > * reserve memory for U-Boot code, data & bss > > > > > > 8011051a <clear_bss>: > > > > > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > > > > > CLEAR_BSS > > > > > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > > > > > CLEAR_BSS > > > > > > CLEAR_BSS > > > > > > > > > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > > 80112724 <setup_mon_len>: > > > > > > static int setup_mon_len(void) > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > > > > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > > gd->relocaddr -= gd->mon_len; > > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > > ip = mon_lengths[yleap]; > > > > > > > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > > 80110398 <setup_mon_len>: > > > > > > static int setup_mon_len(void) > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > > > > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > > gd->relocaddr -= gd->mon_len; > > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > > ip = mon_lengths[yleap]; > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > > > > > > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > > > > > I'd do an objdump of both the working and non-working versions of > > > > > > pxe_utils.o and this is where it got interesting. > > > > > > > > > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > > > > > > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > > > > > cat pxe-notworking.dump > > > > > > > > > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > > > > > > > > > ^-- no actual code dump > > > > > > If I take the working version of this same file without LTO enabled > > > > > > and do a dump, and it's 2291 lines long and full of functions. > > > > > > > > > > > > I tried adding some __used to the non-static function names, but that > > > > > > didn't appear to make any difference to the objdump of pxe_utils.o > > > > > > > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > > > > behaving before/after that change and sorting the object files > > > > > differently. If modifying the dumps like I suggested above doesn't lead > > > > > > > > That's what I was thinking too. > > > > > > > > > to more clues, and it doesn't seem to matter what toolchain is used (are > > > > > you using the gcc-11 from kernel.org that we use in docker and > > > > > buildman?), I'll try and look as well. > > > > > > > > I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: > > > > > > > > Thread model: posix > > > > Supported LTO compression algorithms: zlib zstd > > > > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > > > > > OK. FWIW, if it's easier to build and test, I would suggest also trying > > > CFLAGS_REMOVE_xxx.o := $(LTO_CFLAGS) > > > for all of the obj files under arch/arm/ and board/ and then if that > > > also works correctly, re-adding the flags a directory, then file, at a > > > time until you've narrowed it down. > > > > If I'm understanding this correctly, does this mean that you think > > it's the stuff that's calling the pxe_utils.o and not the pxe_utils.o > > itself? > > Doing the CFLAGS_REMOVE on the pxe_utils.o fixes the issue. > > I think that the change here caused LTO to shuffle files around when > linking the resulting binary and exposed a previously existing issue. > The closer to root cause is some other early object, likely related to > DDR detection, is doing something "special" and failing under LTO. Not > LTO'ing that object would be a starting point to seeing what a more > proper root fix is. Tom, Thanks for the suggestions. I have it booting again by making some functions __used while making others static and eliminating some function prototypes in header files. The file changes are mostly around the ddr initialization and sys_info.c. My next question is who is the TI maintainer? I thought it used to be Lokesh, but his e-mail bounced and get_maintainer script just returns the u-boot mailing list. thanks, adam > > -- > Tom
On Fri, Feb 11, 2022 at 07:09:45PM -0600, Adam Ford wrote: > On Fri, Feb 11, 2022 at 11:13 AM Tom Rini <trini@konsulko.com> wrote: > > > > On Fri, Feb 11, 2022 at 11:10:43AM -0600, Adam Ford wrote: > > > On Fri, Feb 11, 2022 at 10:44 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > > > > > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > > > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > > > > > > > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > > > > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Move the header file into the main include/ directory so we can use it > > > > > > > > > > > > > from the bootmethod code. Move the C file into boot/ since it relates to > > > > > > > > > > > > > booting. > > > > > > > > > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch breaks > > > > > > > > > > > > my omap3_logic board (DM3730) by making it wrongly think there is 4GB > > > > > > > > > > > > of RAM, when in reality there is only 256MB. We have both 256MB and > > > > > > > > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -0600) > > > > > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > > > DRAM: 4 GiB > > > > > > > > > > > > <hang> > > > > > > > > > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -0600) > > > > > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > > > > > > > > DRAM: 256 MiB > > > > > > > > > > > > NAND: 512 MiB > > > > > > > > > > > > MMC: OMAP SD/MMC: 0 > > > > > > > > > > > > Loading Environment from NAND... OK > > > > > > > > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > > > > > > > > Net: eth0: ethernet@08000000 > > > > > > > > > > > > Hit any key to stop autoboot: 0 > > > > > > > > > > > > OMAP Logic # > > > > > > > > > > > > > > > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT all > > > > > > > > > > > > defined, so I am having a hard time understanding why this would > > > > > > > > > > > > change behavior or stomp on the the structure that knows the memory > > > > > > > > > > > > size. > > > > > > > > > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branch > > > > > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my board > > > > > > > > > > > > boots correctly again, but I am struggling to understand why. > > > > > > > > > + Marek Behún > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see what > > > > > > > > > > > functions have changed. > > > > > > > > > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relocation, perhaps? > > > > > > > > > > > > > > > > > > I am still investigating, but disabling LTO appears to fix the issue > > > > > > > > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > > > > > > > > differences in the affected functions and how this patch makes LTO > > > > > > > > > behave differently. > > > > > > > > > > > > > > > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > > > > > > > > time to investigate. If someone has any LTO-related suggestions that > > > > > > > > > I could try, I'd be open to try them too. > > > > > > > > > > > > > > > > Wait, the disassembly is large, or the differences between the > > > > > > > > disassembly, before/after this change alone, are large? It's feeling > > > > > > > > > > > > > > I will be the first to admit thatI am not very good with the assembly > > > > > > > side of things, but this is what I did: > > > > > > > > > > > > > > git checkout master > > > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > > > arm-linux-gnueabihf-objdump -S u-boot > broken.dump > > > > > > > git revert 262cfb5b15420a1aea465745a821e684b3dfa153 > > > > > > > make CROSS_COMPILE=arm-linux-gnueabihf- -j8 > > > > > > > arm-linux-gnueabihf-objdump -S u-boot > working.dump > > > > > > > > > > > > > > diff --side-by-side --suppress-common-lines broken.dump working.dump > > > > > > > > broken-working.diff > > > > > > > cat -n broken-working.diff > > > > > > > > > > > > > > The broken-working.diff file with common lines suppressed is 236256 lines long. > > > > > > > > > > > > OK, I just use '-d' and not '-S', which might help a little bit. But > > > > > > you're probably going to still need to edit the dumps and just globally > > > > > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > > > > > hopefully only seeing where functions were optimized differently. But > > > > > > it might well end up being a bit trickier than that. > > > > > > > > > > It looks like none of the object files are showing any content with > > > > > objdump when LTO is enabled. With a little google search, it appears > > > > > we need lto-dump. I have some more meetings, but I'll try to spend > > > > > some more time on it this weekend. > > > > > > > > > > > > > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercise, > > > > > > > the diff file with common-lines removed is 266573 lines long. > > > > > > > > > > > > > > Maybe I am not using objdump correctly. I am not all that familiar > > > > > > > with this code either, so I am not sure which variables should be in > > > > > > > BSS. I did a search in both working and non-working dumps to look for > > > > > > > keyworks like BSS, but from what I can tell, both have similar > > > > > > > functions: > > > > > > > > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ > > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > * reserve memory for U-Boot code, data & bss > > > > > > > 8011051a <clear_bss>: > > > > > > > #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) > > > > > > > CLEAR_BSS > > > > > > > #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) > > > > > > > CLEAR_BSS > > > > > > > CLEAR_BSS > > > > > > > > > > > > > > When I grepped for mon_len, both sets of dumps looked nearly identical: > > > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump > > > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > > > 80112724 <setup_mon_len>: > > > > > > > static int setup_mon_len(void) > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > 80112726: 4903 ldr r1, [pc, #12] ; (80112734 <setup_mon_len+0x10>) > > > > > > > 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 <setup_mon_len+0x14>) > > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > > > gd->relocaddr -= gd->mon_len; > > > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > > > ip = mon_lengths[yleap]; > > > > > > > > > > > > > > > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump > > > > > > > lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); > > > > > > > 80110398 <setup_mon_len>: > > > > > > > static int setup_mon_len(void) > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 <setup_mon_len+0x10>) > > > > > > > 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac <setup_mon_len+0x14>) > > > > > > > gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; > > > > > > > gd->mon_len = (ulong)&__bss_end - (ulong)_start; > > > > > > > gd->ram_top = board_get_usable_ram_top(gd->mon_len); > > > > > > > gd->relocaddr -= gd->mon_len; > > > > > > > gd->mon_len >> 10, gd->relocaddr); > > > > > > > ip = mon_lengths[yleap]; > > > > > > > aford@aford-OptiPlex-7050:~/src/u-boot$ > > > > > > > > > > > > > > Since I think I narrowed it down to the pxe_utils.o file, I thought > > > > > > > I'd do an objdump of both the working and non-working versions of > > > > > > > pxe_utils.o and this is where it got interesting. > > > > > > > > > > > > > > With LTO building pxe_utils.o, the dump looks empty: > > > > > > > > > > > > > > arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump > > > > > > > cat pxe-notworking.dump > > > > > > > > > > > > > > boot/pxe_utils.o: file format elf32-littlearm > > > > > > > > > > > > > > ^-- no actual code dump > > > > > > > If I take the working version of this same file without LTO enabled > > > > > > > and do a dump, and it's 2291 lines long and full of functions. > > > > > > > > > > > > > > I tried adding some __used to the non-static function names, but that > > > > > > > didn't appear to make any difference to the objdump of pxe_utils.o > > > > > > > > > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > > > > > behaving before/after that change and sorting the object files > > > > > > differently. If modifying the dumps like I suggested above doesn't lead > > > > > > > > > > That's what I was thinking too. > > > > > > > > > > > to more clues, and it doesn't seem to matter what toolchain is used (are > > > > > > you using the gcc-11 from kernel.org that we use in docker and > > > > > > buildman?), I'll try and look as well. > > > > > > > > > > I am using GCC 11, but I'm using the version that come with Ubuntu 21.10: > > > > > > > > > > Thread model: posix > > > > > Supported LTO compression algorithms: zlib zstd > > > > > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > > > > > > > OK. FWIW, if it's easier to build and test, I would suggest also trying > > > > CFLAGS_REMOVE_xxx.o := $(LTO_CFLAGS) > > > > for all of the obj files under arch/arm/ and board/ and then if that > > > > also works correctly, re-adding the flags a directory, then file, at a > > > > time until you've narrowed it down. > > > > > > If I'm understanding this correctly, does this mean that you think > > > it's the stuff that's calling the pxe_utils.o and not the pxe_utils.o > > > itself? > > > Doing the CFLAGS_REMOVE on the pxe_utils.o fixes the issue. > > > > I think that the change here caused LTO to shuffle files around when > > linking the resulting binary and exposed a previously existing issue. > > The closer to root cause is some other early object, likely related to > > DDR detection, is doing something "special" and failing under LTO. Not > > LTO'ing that object would be a starting point to seeing what a more > > proper root fix is. > > Tom, > Thanks for the suggestions. I have it booting again by making some > functions __used while making others static and eliminating some > function prototypes in header files. > The file changes are mostly around the ddr initialization and sys_info.c. > > My next question is who is the TI maintainer? I thought it used to be > Lokesh, but his e-mail bounced and get_maintainer script just returns > the u-boot mailing list. Hunh, I guess I need to fix the entry since, hey, it's me. Thanks for digging in here!
diff --git a/boot/Makefile b/boot/Makefile index a19e85cf6c8..2938c3f1458 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -14,6 +14,9 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o +obj-$(CONFIG_CMD_PXE) += pxe_utils.o +obj-$(CONFIG_CMD_SYSBOOT) += pxe_utils.o + endif obj-y += image.o image-board.o diff --git a/cmd/pxe_utils.c b/boot/pxe_utils.c similarity index 100% rename from cmd/pxe_utils.c rename to boot/pxe_utils.c diff --git a/cmd/Makefile b/cmd/Makefile index ed3669411e6..891819ae0f6 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -123,7 +123,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o obj-$(CONFIG_CMD_PMC) += pmc.o obj-$(CONFIG_CMD_PSTORE) += pstore.o obj-$(CONFIG_CMD_PWM) += pwm.o -obj-$(CONFIG_CMD_PXE) += pxe.o pxe_utils.o +obj-$(CONFIG_CMD_PXE) += pxe.o obj-$(CONFIG_CMD_WOL) += wol.o obj-$(CONFIG_CMD_QFW) += qfw.o obj-$(CONFIG_CMD_READ) += read.o @@ -145,7 +145,7 @@ obj-$(CONFIG_CMD_SETEXPR_FMT) += printf.o obj-$(CONFIG_CMD_SPI) += spi.o obj-$(CONFIG_CMD_STRINGS) += strings.o obj-$(CONFIG_CMD_SMC) += smccc.o -obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o +obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o obj-$(CONFIG_CMD_TERMINAL) += terminal.o obj-$(CONFIG_CMD_TIME) += time.o diff --git a/cmd/sysboot.c b/cmd/sysboot.c index 85fa5d8aa01..b81255e155a 100644 --- a/cmd/sysboot.c +++ b/cmd/sysboot.c @@ -4,7 +4,7 @@ #include <command.h> #include <env.h> #include <fs.h> -#include "pxe_utils.h" +#include <pxe_utils.h> static char *fs_argv[5]; diff --git a/cmd/pxe_utils.h b/include/pxe_utils.h similarity index 100% rename from cmd/pxe_utils.h rename to include/pxe_utils.h
Move the header file into the main include/ directory so we can use it from the bootmethod code. Move the C file into boot/ since it relates to booting. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) boot/Makefile | 3 +++ {cmd => boot}/pxe_utils.c | 0 cmd/Makefile | 4 ++-- cmd/sysboot.c | 2 +- {cmd => include}/pxe_utils.h | 0 5 files changed, 6 insertions(+), 3 deletions(-) rename {cmd => boot}/pxe_utils.c (100%) rename {cmd => include}/pxe_utils.h (100%)