| Message ID | 20241015153717.401371-6-sughosh.ganu@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2f6191526a1325b6ddb59795a093eca69dbf8976 |
| Headers | show |
| Series | Make EFI memory allocations synchronous with LMB | expand |
Hei hei, Am Tue, Oct 15, 2024 at 09:07:07PM +0530 schrieb Sughosh Ganu: > In U-Boot, LMB and EFI are two primary modules who provide memory > allocation and reservation API's. Both these modules operate with the > same regions of memory for allocations. Use the LMB memory map update > event to notify other interested listeners about a change in it's > memory map. This can then be used by the other module to keep track of > available and used memory. > > There is no need to send these notifications when the LMB module is > being unit-tested. Add a flag to the lmb structure to indicate if the > memory map is being used for tests, and suppress sending any > notifications when running these unit tests. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V3: > * Drop use of is_addr_in_ram() function > * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the > notification needs to be sent. > * s/lmb_notify/lmb_should_notify > * Put a check for EFI_LOADER in the lmb_should_notify() function This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for me. In a usual non-debug build, everything is fine. However in a debug build now I get this error: UPD include/generated/timestamp_autogenerated.h GEN Makefile Using /mnt/data/adahl/src/u-boot as source for U-Boot CC common/version.o AR common/built-in.o LD u-boot arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' My tree is based on v2025.01 release. lib/lmb.c is built because … │ Selected by [y]: │ - SYS_BOOT_RAMDISK_HIGH [=y] && (CMD_BOOTM [=y] || CMD_BOOTI [=n] || CMD_BOOTZ [=y]) && !NIOS2 [=n] && !SANDBOX [=n] && !SH [=n] && !XTENSA [=n] │ Selected by [n]: │ - EFI_LOADER [=n] && OF_LIBFDT [=y] && (ARM [=y] && (SYS_CPU [=arm926ejs]=arm1136 || SYS_CPU [=arm926ejs]=arm1176 || … So as you can see EFI_LOADER is disabled, because I don't need to run UEFI applications on my armv5te SoC (Microchip sam9x60). CMD_BOOTZ is enabled, because I boot a Linux kernel. I can not enable EFI_LOADER however, because there's nothing in the long list of "depends on" which is or can be satified on my target. I would really appreciate to be able to use a debug build again, please advice! Greets Alex
On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > Hei hei, > > Am Tue, Oct 15, 2024 at 09:07:07PM +0530 schrieb Sughosh Ganu: > > In U-Boot, LMB and EFI are two primary modules who provide memory > > allocation and reservation API's. Both these modules operate with the > > same regions of memory for allocations. Use the LMB memory map update > > event to notify other interested listeners about a change in it's > > memory map. This can then be used by the other module to keep track of > > available and used memory. > > > > There is no need to send these notifications when the LMB module is > > being unit-tested. Add a flag to the lmb structure to indicate if the > > memory map is being used for tests, and suppress sending any > > notifications when running these unit tests. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V3: > > * Drop use of is_addr_in_ram() function > > * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the > > notification needs to be sent. > > * s/lmb_notify/lmb_should_notify > > * Put a check for EFI_LOADER in the lmb_should_notify() function > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > me. In a usual non-debug build, everything is fine. However in a > debug build now I get this error: > > UPD include/generated/timestamp_autogenerated.h > GEN Makefile > Using /mnt/data/adahl/src/u-boot as source for U-Boot > CC common/version.o > AR common/built-in.o > LD u-boot > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' Can you share the details of the toolchain that you are using, where can I download it from. And the target that you build which shows this up. Thanks. -sughosh > > My tree is based on v2025.01 release. lib/lmb.c is built because … > > │ Selected by [y]: > │ - SYS_BOOT_RAMDISK_HIGH [=y] && (CMD_BOOTM [=y] || CMD_BOOTI [=n] || CMD_BOOTZ [=y]) && !NIOS2 [=n] && !SANDBOX [=n] && !SH [=n] && !XTENSA [=n] > │ Selected by [n]: > │ - EFI_LOADER [=n] && OF_LIBFDT [=y] && (ARM [=y] && (SYS_CPU [=arm926ejs]=arm1136 || SYS_CPU [=arm926ejs]=arm1176 || … > > So as you can see EFI_LOADER is disabled, because I don't need to run > UEFI applications on my armv5te SoC (Microchip sam9x60). > CMD_BOOTZ is enabled, because I boot a Linux kernel. > > I can not enable EFI_LOADER however, because there's nothing in the > long list of "depends on" which is or can be satified on my target. > > I would really appreciate to be able to use a debug build again, > please advice! > > Greets > Alex >
Hello, Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: […] > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > me. In a usual non-debug build, everything is fine. However in a > > debug build now I get this error: > > > > UPD include/generated/timestamp_autogenerated.h > > GEN Makefile > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > CC common/version.o > > AR common/built-in.o > > LD u-boot > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > Can you share the details of the toolchain that you are using, where > can I download it from. And the target that you build which shows this > up. Thanks. Although I'm actually building with ptxdist and OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, you should be able to reproduce the behaviour easily with buildman on master when building for boards sam9x60ek or sam9x60_curiosity like this: buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 My output when running the above is this: % buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 Building current source for 5 boards (5 threads, 4 jobs per thread) arm: + sam9x60_curiosity_mmc1 +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' +make[1]: *** [Makefile:1824: u-boot] Fehler 1 +make: *** [Makefile:177: sub-make] Error 2 arm: + sam9x60_curiosity_mmc +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' +make[1]: *** [Makefile:1824: u-boot] Fehler 1 +make: *** [Makefile:177: sub-make] Error 2 arm: + sam9x60ek_mmc +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' +make[1]: *** [Makefile:1824: u-boot] Fehler 1 +make: *** [Makefile:177: sub-make] Error 2 arm: + sam9x60ek_qspiflash +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' +make[1]: *** [Makefile:1824: u-boot] Fehler 1 +make: *** [Makefile:177: sub-make] Error 2 arm: + sam9x60ek_nandflash +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' +make[1]: *** [Makefile:1824: u-boot] Fehler 1 +make: *** [Makefile:177: sub-make] Error 2 0 0 5 /5 0:00:40 : sam9x60ek_nandflash Completed: 5 total built, 5 newly), duration 0:00:22, rate 0.23 Greets Alex
On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > Hello, > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > […] > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > me. In a usual non-debug build, everything is fine. However in a > > > debug build now I get this error: > > > > > > UPD include/generated/timestamp_autogenerated.h > > > GEN Makefile > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > CC common/version.o > > > AR common/built-in.o > > > LD u-boot > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > Can you share the details of the toolchain that you are using, where > > can I download it from. And the target that you build which shows this > > up. Thanks. > > Although I'm actually building with ptxdist and > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > you should be able to reproduce the behaviour easily with buildman on > master when building for boards sam9x60ek or sam9x60_curiosity like > this: > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > My output when running the above is this: Sorry, I haven't spent time debugging this issue, although this is on my todo list. Btw, since you already have a working setup, can you please reset your tree to the following commit and then check if that fixes your issue. 92e75ee47f1 ("board_r: Remove duplicate headers") -sughosh > > % buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > Building current source for 5 boards (5 threads, 4 jobs per thread) > arm: + sam9x60_curiosity_mmc1 > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > +make: *** [Makefile:177: sub-make] Error 2 > arm: + sam9x60_curiosity_mmc > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > +make: *** [Makefile:177: sub-make] Error 2 > arm: + sam9x60ek_mmc > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > +make: *** [Makefile:177: sub-make] Error 2 > arm: + sam9x60ek_qspiflash > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > +make: *** [Makefile:177: sub-make] Error 2 > arm: + sam9x60ek_nandflash > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > +make: *** [Makefile:177: sub-make] Error 2 > 0 0 5 /5 0:00:40 : sam9x60ek_nandflash > Completed: 5 total built, 5 newly), duration 0:00:22, rate 0.23 > > Greets > Alex >
Hello, Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > Hello, > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > […] > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > me. In a usual non-debug build, everything is fine. However in a > > > > debug build now I get this error: > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > GEN Makefile > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > CC common/version.o > > > > AR common/built-in.o > > > > LD u-boot > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > Can you share the details of the toolchain that you are using, where > > > can I download it from. And the target that you build which shows this > > > up. Thanks. > > > > Although I'm actually building with ptxdist and > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > you should be able to reproduce the behaviour easily with buildman on > > master when building for boards sam9x60ek or sam9x60_curiosity like > > this: > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > My output when running the above is this: > > Sorry, I haven't spent time debugging this issue, although this is on > my todo list. Btw, since you already have a working setup, can you > please reset your tree to the following commit and then check if that > fixes your issue. > > 92e75ee47f1 ("board_r: Remove duplicate headers") Does not change anything. Note: my tree is not special in any way, it's a clean mainline u-boot checkout and I'm building for a board with an in tree config. Everyone should be able to reproduce this by using configs/sam9x60_curiosity_mmc_defconfig, enabling CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain downloaded by buildman, which is also mainline u-boot tooling. My assumption is: some efi code is pulled in unconditionally here, but efi is not enabled on armv5 and it can not be enabled, efi is not needed on this target anyways. Furthermore: I assume the code is optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, which is basically building without optimizations and keeping debug symbols. Greets Alex > > -sughosh > > > > > % buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > Building current source for 5 boards (5 threads, 4 jobs per thread) > > arm: + sam9x60_curiosity_mmc1 > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > +make: *** [Makefile:177: sub-make] Error 2 > > arm: + sam9x60_curiosity_mmc > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > +make: *** [Makefile:177: sub-make] Error 2 > > arm: + sam9x60ek_mmc > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > +make: *** [Makefile:177: sub-make] Error 2 > > arm: + sam9x60ek_qspiflash > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > +make: *** [Makefile:177: sub-make] Error 2 > > arm: + sam9x60ek_nandflash > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > +make: *** [Makefile:177: sub-make] Error 2 > > 0 0 5 /5 0:00:40 : sam9x60ek_nandflash > > Completed: 5 total built, 5 newly), duration 0:00:22, rate 0.23 > > > > Greets > > Alex > >
On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > Hello, > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > Hello, > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > […] > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > debug build now I get this error: > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > GEN Makefile > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > CC common/version.o > > > > > AR common/built-in.o > > > > > LD u-boot > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > can I download it from. And the target that you build which shows this > > > > up. Thanks. > > > > > > Although I'm actually building with ptxdist and > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > you should be able to reproduce the behaviour easily with buildman on > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > this: > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > My output when running the above is this: > > > > Sorry, I haven't spent time debugging this issue, although this is on > > my todo list. Btw, since you already have a working setup, can you > > please reset your tree to the following commit and then check if that > > fixes your issue. > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > Does not change anything. > > Note: my tree is not special in any way, it's a clean mainline u-boot > checkout and I'm building for a board with an in tree config. > Everyone should be able to reproduce this by using > configs/sam9x60_curiosity_mmc_defconfig, enabling > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > downloaded by buildman, which is also mainline u-boot tooling. > > My assumption is: some efi code is pulled in unconditionally here, > but efi is not enabled on armv5 and it can not be enabled, efi is not > needed on this target anyways. Furthermore: I assume the code is > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > which is basically building without optimizations and keeping debug > symbols. Okay. Please give me some time. I will try to spend some time on the issue this week. Thanks for your quick response. -sughosh > > Greets > Alex > > > > > -sughosh > > > > > > > > % buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > Building current source for 5 boards (5 threads, 4 jobs per thread) > > > arm: + sam9x60_curiosity_mmc1 > > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > arm: + sam9x60_curiosity_mmc > > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > arm: + sam9x60ek_mmc > > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > arm: + sam9x60ek_qspiflash > > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > arm: + sam9x60ek_nandflash > > > +arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > +lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > +make[1]: *** [Makefile:1824: u-boot] Fehler 1 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > 0 0 5 /5 0:00:40 : sam9x60ek_nandflash > > > Completed: 5 total built, 5 newly), duration 0:00:22, rate 0.23 > > > > > > Greets > > > Alex > > >
On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > Hello, > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > Hello, > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > […] > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > debug build now I get this error: > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > GEN Makefile > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > CC common/version.o > > > > > > AR common/built-in.o > > > > > > LD u-boot > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > can I download it from. And the target that you build which shows this > > > > > up. Thanks. > > > > > > > > Although I'm actually building with ptxdist and > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > you should be able to reproduce the behaviour easily with buildman on > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > this: > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > My output when running the above is this: > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > my todo list. Btw, since you already have a working setup, can you > > > please reset your tree to the following commit and then check if that > > > fixes your issue. > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > Does not change anything. > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > checkout and I'm building for a board with an in tree config. > > Everyone should be able to reproduce this by using > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > downloaded by buildman, which is also mainline u-boot tooling. > > > > My assumption is: some efi code is pulled in unconditionally here, > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > needed on this target anyways. Furthermore: I assume the code is > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > which is basically building without optimizations and keeping debug > > symbols. > > Okay. Please give me some time. I will try to spend some time on the > issue this week. Thanks for your quick response. Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler optimizations that we normally rely on to avoid having to #ifdef the code. In this case, lmb_should_notify() causes this to normally be optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the compiler will do the right thing. The "fix" is to instead make most of the body of lmb_map_update_notify be guarded with #if CONFIG_IS_ENABLED(EFI_LOADER) ... #endif
On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > Hello, > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > Hello, > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > […] > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > GEN Makefile > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > CC common/version.o > > > > > > > AR common/built-in.o > > > > > > > LD u-boot > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > can I download it from. And the target that you build which shows this > > > > > > up. Thanks. > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > this: > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > My output when running the above is this: > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > my todo list. Btw, since you already have a working setup, can you > > > > please reset your tree to the following commit and then check if that > > > > fixes your issue. > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > Does not change anything. > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > checkout and I'm building for a board with an in tree config. > > > Everyone should be able to reproduce this by using > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > needed on this target anyways. Furthermore: I assume the code is > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > which is basically building without optimizations and keeping debug > > > symbols. > > > > Okay. Please give me some time. I will try to spend some time on the > > issue this week. Thanks for your quick response. > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > optimizations that we normally rely on to avoid having to #ifdef the > code. In this case, lmb_should_notify() causes this to normally be > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > compiler will do the right thing. The "fix" is to instead make most of > the body of lmb_map_update_notify be guarded with #if > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif Yes, that would be the obvious thing to do. But I suspect that because there are no optimisations with the said flag, it might be needed to even protect the function calls with the ifdefs. Which I am hoping to avoid. -sughosh > > -- > Tom
On Tue, Feb 11, 2025 at 07:30:12PM +0530, Sughosh Ganu wrote: > On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > Hello, > > > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > Hello, > > > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > […] > > > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > > GEN Makefile > > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > > CC common/version.o > > > > > > > > AR common/built-in.o > > > > > > > > LD u-boot > > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > > can I download it from. And the target that you build which shows this > > > > > > > up. Thanks. > > > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > > this: > > > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > > > My output when running the above is this: > > > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > > my todo list. Btw, since you already have a working setup, can you > > > > > please reset your tree to the following commit and then check if that > > > > > fixes your issue. > > > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > > > Does not change anything. > > > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > > checkout and I'm building for a board with an in tree config. > > > > Everyone should be able to reproduce this by using > > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > > needed on this target anyways. Furthermore: I assume the code is > > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > > which is basically building without optimizations and keeping debug > > > > symbols. > > > > > > Okay. Please give me some time. I will try to spend some time on the > > > issue this week. Thanks for your quick response. > > > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > > optimizations that we normally rely on to avoid having to #ifdef the > > code. In this case, lmb_should_notify() causes this to normally be > > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > > compiler will do the right thing. The "fix" is to instead make most of > > the body of lmb_map_update_notify be guarded with #if > > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif > > Yes, that would be the obvious thing to do. But I suspect that because > there are no optimisations with the said flag, it might be needed to > even protect the function calls with the ifdefs. Which I am hoping to > avoid. If we guard the body of the function with #if (not if (...) { ... }) then cpp will still do the right thing :)
On Tue, 11 Feb 2025 at 19:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > Hello, > > > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > Hello, > > > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > […] > > > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > > GEN Makefile > > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > > CC common/version.o > > > > > > > > AR common/built-in.o > > > > > > > > LD u-boot > > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > > can I download it from. And the target that you build which shows this > > > > > > > up. Thanks. > > > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > > this: > > > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > > > My output when running the above is this: > > > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > > my todo list. Btw, since you already have a working setup, can you > > > > > please reset your tree to the following commit and then check if that > > > > > fixes your issue. > > > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > > > Does not change anything. > > > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > > checkout and I'm building for a board with an in tree config. > > > > Everyone should be able to reproduce this by using > > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > > needed on this target anyways. Furthermore: I assume the code is > > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > > which is basically building without optimizations and keeping debug > > > > symbols. > > > > > > Okay. Please give me some time. I will try to spend some time on the > > > issue this week. Thanks for your quick response. > > > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > > optimizations that we normally rely on to avoid having to #ifdef the > > code. In this case, lmb_should_notify() causes this to normally be > > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > > compiler will do the right thing. The "fix" is to instead make most of > > the body of lmb_map_update_notify be guarded with #if > > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif > > Yes, that would be the obvious thing to do. But I suspect that because > there are no optimisations with the said flag, it might be needed to > even protect the function calls with the ifdefs. Which I am hoping to > avoid. If we cannot do without using ifdefs at the function call sites, what is your opinion on disabling lmb with the CC_OPTIMIZE_FOR_DEBUG flag enabled? Maybe this can be converted into a config flag. There are existing configurations like xilinix mini which too are disabling lmb. Thoughts? -sughosh > > -sughosh > > > > > -- > > Tom
On Tue, Feb 11, 2025 at 07:40:40PM +0530, Sughosh Ganu wrote: > On Tue, 11 Feb 2025 at 19:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > Hello, > > > > > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > […] > > > > > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > > > GEN Makefile > > > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > > > CC common/version.o > > > > > > > > > AR common/built-in.o > > > > > > > > > LD u-boot > > > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > > > can I download it from. And the target that you build which shows this > > > > > > > > up. Thanks. > > > > > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > > > this: > > > > > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > > > > > My output when running the above is this: > > > > > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > > > my todo list. Btw, since you already have a working setup, can you > > > > > > please reset your tree to the following commit and then check if that > > > > > > fixes your issue. > > > > > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > > > > > Does not change anything. > > > > > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > > > checkout and I'm building for a board with an in tree config. > > > > > Everyone should be able to reproduce this by using > > > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > > > needed on this target anyways. Furthermore: I assume the code is > > > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > > > which is basically building without optimizations and keeping debug > > > > > symbols. > > > > > > > > Okay. Please give me some time. I will try to spend some time on the > > > > issue this week. Thanks for your quick response. > > > > > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > > > optimizations that we normally rely on to avoid having to #ifdef the > > > code. In this case, lmb_should_notify() causes this to normally be > > > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > > > compiler will do the right thing. The "fix" is to instead make most of > > > the body of lmb_map_update_notify be guarded with #if > > > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif > > > > Yes, that would be the obvious thing to do. But I suspect that because > > there are no optimisations with the said flag, it might be needed to > > even protect the function calls with the ifdefs. Which I am hoping to > > avoid. > > If we cannot do without using ifdefs at the function call sites, what > is your opinion on disabling lmb with the CC_OPTIMIZE_FOR_DEBUG flag > enabled? Maybe this can be converted into a config flag. There are > existing configurations like xilinix mini which too are disabling lmb. > Thoughts? It should just be a single set of #ifdef ... #endif, and possibly adding a __maybe_unused.
On Tue, 11 Feb 2025 at 20:25, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Feb 11, 2025 at 07:40:40PM +0530, Sughosh Ganu wrote: > > On Tue, 11 Feb 2025 at 19:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > > > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > Hello, > > > > > > > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > > > […] > > > > > > > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > > > > GEN Makefile > > > > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > > > > CC common/version.o > > > > > > > > > > AR common/built-in.o > > > > > > > > > > LD u-boot > > > > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > > > > can I download it from. And the target that you build which shows this > > > > > > > > > up. Thanks. > > > > > > > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > > > > this: > > > > > > > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > > > > > > > My output when running the above is this: > > > > > > > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > > > > my todo list. Btw, since you already have a working setup, can you > > > > > > > please reset your tree to the following commit and then check if that > > > > > > > fixes your issue. > > > > > > > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > > > > > > > Does not change anything. > > > > > > > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > > > > checkout and I'm building for a board with an in tree config. > > > > > > Everyone should be able to reproduce this by using > > > > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > > > > needed on this target anyways. Furthermore: I assume the code is > > > > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > > > > which is basically building without optimizations and keeping debug > > > > > > symbols. > > > > > > > > > > Okay. Please give me some time. I will try to spend some time on the > > > > > issue this week. Thanks for your quick response. > > > > > > > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > > > > optimizations that we normally rely on to avoid having to #ifdef the > > > > code. In this case, lmb_should_notify() causes this to normally be > > > > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > > > > compiler will do the right thing. The "fix" is to instead make most of > > > > the body of lmb_map_update_notify be guarded with #if > > > > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif > > > > > > Yes, that would be the obvious thing to do. But I suspect that because > > > there are no optimisations with the said flag, it might be needed to > > > even protect the function calls with the ifdefs. Which I am hoping to > > > avoid. > > > > If we cannot do without using ifdefs at the function call sites, what > > is your opinion on disabling lmb with the CC_OPTIMIZE_FOR_DEBUG flag > > enabled? Maybe this can be converted into a config flag. There are > > existing configurations like xilinix mini which too are disabling lmb. > > Thoughts? > > It should just be a single set of #ifdef ... #endif, and possibly adding > a __maybe_unused. Like I suspected, just a single set of #ifdef ... #endif does not help. Get link errors like such arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `_lmb_alloc_base': /data_sda/linaro_work/lmb_efi_fixes/lmb_qemu_test_setup/u-boot/lib/lmb.c:736:(.text._lmb_alloc_base+0x66): undefined reference to `lmb_map_update_notify' arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_add': /data_sda/linaro_work/lmb_efi_fixes/lmb_qemu_test_setup/u-boot/lib/lmb.c:664:(.text.lmb_add+0x20): undefined reference to `lmb_map_update_notify' arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_free_flags': /data_sda/linaro_work/lmb_efi_fixes/lmb_qemu_test_setup/u-boot/lib/lmb.c:676:(.text.lmb_free_flags+0x1e): undefined reference to `lmb_map_update_notify' arm-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_reserve': /data_sda/linaro_work/lmb_efi_fixes/lmb_qemu_test_setup/u-boot/lib/lmb.c:696:(.text.lmb_reserve+0x2a): undefined reference to `lmb_map_update_notify' make: *** [Makefile:1824: u-boot] Error 1 This is with the CONFIG_CC_OPTIMIZE_FOR_DEBUG enabled. -sughosh > > -- > Tom
On Wed, Feb 12, 2025 at 03:11:16PM +0530, Sughosh Ganu wrote: > On Tue, 11 Feb 2025 at 20:25, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Feb 11, 2025 at 07:40:40PM +0530, Sughosh Ganu wrote: > > > On Tue, 11 Feb 2025 at 19:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > On Tue, 11 Feb 2025 at 19:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Tue, Feb 11, 2025 at 04:41:09PM +0530, Sughosh Ganu wrote: > > > > > > On Tue, 11 Feb 2025 at 16:34, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > Am Tue, Feb 11, 2025 at 04:23:13PM +0530 schrieb Sughosh Ganu: > > > > > > > > On Mon, 20 Jan 2025 at 15:43, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > Am Mon, Jan 20, 2025 at 02:40:38PM +0530 schrieb Sughosh Ganu: > > > > > > > > > > On Fri, 17 Jan 2025 at 16:40, Alexander Dahl <ada@thorsis.com> wrote: > > > > > > > > > > > > > > > > > > […] > > > > > > > > > > > > > > > > > > > > This changeset breaks build with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y for > > > > > > > > > > > me. In a usual non-debug build, everything is fine. However in a > > > > > > > > > > > debug build now I get this error: > > > > > > > > > > > > > > > > > > > > > > UPD include/generated/timestamp_autogenerated.h > > > > > > > > > > > GEN Makefile > > > > > > > > > > > Using /mnt/data/adahl/src/u-boot as source for U-Boot > > > > > > > > > > > CC common/version.o > > > > > > > > > > > AR common/built-in.o > > > > > > > > > > > LD u-boot > > > > > > > > > > > arm-v5te-linux-gnueabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify': > > > > > > > > > > > /mnt/data/adahl/src/u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg' > > > > > > > > > > > > > > > > > > > > Can you share the details of the toolchain that you are using, where > > > > > > > > > > can I download it from. And the target that you build which shows this > > > > > > > > > > up. Thanks. > > > > > > > > > > > > > > > > > > Although I'm actually building with ptxdist and > > > > > > > > > OSELAS.Toolchain-2023.07.1/arm-v5te-linux-gnueabi for a custom board, > > > > > > > > > you should be able to reproduce the behaviour easily with buildman on > > > > > > > > > master when building for boards sam9x60ek or sam9x60_curiosity like > > > > > > > > > this: > > > > > > > > > > > > > > > > > > buildman -o ~/build/u-boot/buildman -e -a CC_OPTIMIZE_FOR_DEBUG sam9x60 > > > > > > > > > > > > > > > > > > My output when running the above is this: > > > > > > > > > > > > > > > > Sorry, I haven't spent time debugging this issue, although this is on > > > > > > > > my todo list. Btw, since you already have a working setup, can you > > > > > > > > please reset your tree to the following commit and then check if that > > > > > > > > fixes your issue. > > > > > > > > > > > > > > > > 92e75ee47f1 ("board_r: Remove duplicate headers") > > > > > > > > > > > > > > Does not change anything. > > > > > > > > > > > > > > Note: my tree is not special in any way, it's a clean mainline u-boot > > > > > > > checkout and I'm building for a board with an in tree config. > > > > > > > Everyone should be able to reproduce this by using > > > > > > > configs/sam9x60_curiosity_mmc_defconfig, enabling > > > > > > > CC_OPTIMIZE_FOR_DEBUG and building with the default toolchain > > > > > > > downloaded by buildman, which is also mainline u-boot tooling. > > > > > > > > > > > > > > My assumption is: some efi code is pulled in unconditionally here, > > > > > > > but efi is not enabled on armv5 and it can not be enabled, efi is not > > > > > > > needed on this target anyways. Furthermore: I assume the code is > > > > > > > optimized out when doing a normal build without CC_OPTIMIZE_FOR_DEBUG, > > > > > > > which is basically building without optimizations and keeping debug > > > > > > > symbols. > > > > > > > > > > > > Okay. Please give me some time. I will try to spend some time on the > > > > > > issue this week. Thanks for your quick response. > > > > > > > > > > Yes, the problem is that CC_OPTIMIZE_FOR_DEBUG disables compiler > > > > > optimizations that we normally rely on to avoid having to #ifdef the > > > > > code. In this case, lmb_should_notify() causes this to normally be > > > > > optimized away as CONFIG_IS_ENABLED(EFI_LOADER) will be false and the > > > > > compiler will do the right thing. The "fix" is to instead make most of > > > > > the body of lmb_map_update_notify be guarded with #if > > > > > CONFIG_IS_ENABLED(EFI_LOADER) ... #endif > > > > > > > > Yes, that would be the obvious thing to do. But I suspect that because > > > > there are no optimisations with the said flag, it might be needed to > > > > even protect the function calls with the ifdefs. Which I am hoping to > > > > avoid. > > > > > > If we cannot do without using ifdefs at the function call sites, what > > > is your opinion on disabling lmb with the CC_OPTIMIZE_FOR_DEBUG flag > > > enabled? Maybe this can be converted into a config flag. There are > > > existing configurations like xilinix mini which too are disabling lmb. > > > Thoughts? > > > > It should just be a single set of #ifdef ... #endif, and possibly adding > > a __maybe_unused. > > Like I suspected, just a single set of #ifdef ... #endif does not > help. Get link errors like such For sam9x60ek_mmc_config with CC_OPTIMIZE_FOR_DEBUG this is all I needed: diff --git a/lib/lmb.c b/lib/lmb.c index 7ca44591e1d7..1e9e9f9a3321 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -430,7 +430,7 @@ long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) static struct lmb lmb; -static bool lmb_should_notify(u32 flags) +static __maybe_unused bool lmb_should_notify(u32 flags) { return !lmb.test && !(flags & LMB_NONOTIFY) && CONFIG_IS_ENABLED(EFI_LOADER); @@ -439,6 +439,7 @@ static bool lmb_should_notify(u32 flags) static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, u32 flags) { +#if CONFIG_IS_ENABLED(EFI_LOADER) u64 efi_addr; u64 pages; efi_status_t status; @@ -466,6 +467,7 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, return -1; } unmap_sysmem((void *)(uintptr_t)efi_addr); +#endif return 0; }
diff --git a/include/efi_loader.h b/include/efi_loader.h index 511281e150e..ac203957181 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -784,6 +784,20 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, uint32_t *descriptor_version); /* Adds a range into the EFI memory map */ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); + +/** + * efi_add_memory_map_pg() - add pages to the memory map + * + * @start: start address, must be a multiple of EFI_PAGE_SIZE + * @pages: number of pages to add + * @memory_type: type of memory added + * @overlap_only_ram: region may only overlap RAM + * Return: status code + */ +efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, + int memory_type, + bool overlap_only_ram); + /* Adds a conventional range into the EFI memory map */ efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, u64 ram_top); diff --git a/include/lmb.h b/include/lmb.h index ec477c1f51d..837002121d9 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -46,10 +46,12 @@ struct lmb_region { * * @free_mem: List of free memory regions * @used_mem: List of used/reserved memory regions + * @test: Is structure being used for LMB tests */ struct lmb { struct alist free_mem; struct alist used_mem; + bool test; }; /** diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index aa1da21f9f9..41501e9d41b 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -264,7 +264,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, * @overlap_only_ram: region may only overlap RAM * Return: status code */ -static efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, +efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, int memory_type, bool overlap_only_ram) { diff --git a/lib/lmb.c b/lib/lmb.c index 0504a7b3407..e7167f858f0 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -8,6 +8,7 @@ #include <alist.h> #include <efi_loader.h> +#include <event.h> #include <image.h> #include <mapmem.h> #include <lmb.h> @@ -22,11 +23,52 @@ DECLARE_GLOBAL_DATA_PTR; +#define MAP_OP_RESERVE (u8)0x1 +#define MAP_OP_FREE (u8)0x2 +#define MAP_OP_ADD (u8)0x3 + #define LMB_ALLOC_ANYWHERE 0 #define LMB_ALIST_INITIAL_SIZE 4 static struct lmb lmb; +static bool lmb_should_notify(enum lmb_flags flags) +{ + return !lmb.test && !(flags & LMB_NONOTIFY) && + CONFIG_IS_ENABLED(EFI_LOADER); +} + +static int __maybe_unused lmb_map_update_notify(phys_addr_t addr, + phys_size_t size, + u8 op) +{ + u64 efi_addr; + u64 pages; + efi_status_t status; + + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) { + log_err("Invalid map update op received (%d)\n", op); + return -1; + } + + efi_addr = (uintptr_t)map_sysmem(addr, 0); + pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); + efi_addr &= ~EFI_PAGE_MASK; + + status = efi_add_memory_map_pg(efi_addr, pages, + op == MAP_OP_RESERVE ? + EFI_BOOT_SERVICES_DATA : + EFI_CONVENTIONAL_MEMORY, + false); + if (status != EFI_SUCCESS) { + log_err("%s: LMB Map notify failure %lu\n", __func__, + status & ~EFI_ERROR_MASK); + return -1; + } else { + return 0; + } +} + static void lmb_print_region_flags(enum lmb_flags flags) { u64 bitpos; @@ -473,9 +515,17 @@ static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, /* This routine may be called with relocation disabled. */ long lmb_add(phys_addr_t base, phys_size_t size) { + long ret; struct alist *lmb_rgn_lst = &lmb.free_mem; - return lmb_add_region(lmb_rgn_lst, base, size); + ret = lmb_add_region(lmb_rgn_lst, base, size); + if (ret) + return ret; + + if (lmb_should_notify(LMB_NONE)) + return lmb_map_update_notify(base, size, MAP_OP_ADD); + + return 0; } static long __lmb_free(phys_addr_t base, phys_size_t size) @@ -529,11 +579,6 @@ static long __lmb_free(phys_addr_t base, phys_size_t size) rgn[i].flags); } -long lmb_free(phys_addr_t base, phys_size_t size) -{ - return __lmb_free(base, size); -} - /** * lmb_free_flags() - Free up a region of memory * @base: Base Address of region to be freed @@ -545,16 +590,38 @@ long lmb_free(phys_addr_t base, phys_size_t size) * Return: 0 if successful, -1 on failure */ long lmb_free_flags(phys_addr_t base, phys_size_t size, - __always_unused uint flags) + uint flags) { - return __lmb_free(base, size); + long ret; + + ret = __lmb_free(base, size); + if (ret < 0) + return ret; + + if (lmb_should_notify(flags)) + return lmb_map_update_notify(base, size, MAP_OP_FREE); + + return ret; +} + +long lmb_free(phys_addr_t base, phys_size_t size) +{ + return lmb_free_flags(base, size, LMB_NONE); } long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) { + long ret = 0; struct alist *lmb_rgn_lst = &lmb.used_mem; - return lmb_add_region_flags(lmb_rgn_lst, base, size, flags); + ret = lmb_add_region_flags(lmb_rgn_lst, base, size, flags); + if (ret < 0) + return -1; + + if (lmb_should_notify(flags)) + return lmb_map_update_notify(base, size, MAP_OP_RESERVE); + + return ret; } long lmb_reserve(phys_addr_t base, phys_size_t size) @@ -586,6 +653,8 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size) static phys_addr_t __lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, enum lmb_flags flags) { + u8 op; + int ret; long i, rgn; phys_addr_t base = 0; phys_addr_t res_base; @@ -616,6 +685,15 @@ static phys_addr_t __lmb_alloc_base(phys_size_t size, ulong align, if (lmb_add_region_flags(&lmb.used_mem, base, size, flags) < 0) return 0; + + if (lmb_should_notify(flags)) { + op = MAP_OP_RESERVE; + ret = lmb_map_update_notify(base, size, + op); + if (ret) + return ret; + } + return base; } @@ -785,7 +863,7 @@ int lmb_is_reserved_flags(phys_addr_t addr, int flags) return 0; } -static int lmb_setup(void) +static int lmb_setup(bool test) { bool ret; @@ -803,6 +881,8 @@ static int lmb_setup(void) return -ENOMEM; } + lmb.test = test; + return 0; } @@ -822,7 +902,7 @@ int lmb_init(void) { int ret; - ret = lmb_setup(); + ret = lmb_setup(false); if (ret) { log_info("Unable to init LMB\n"); return ret; @@ -850,7 +930,7 @@ int lmb_push(struct lmb *store) int ret; *store = lmb; - ret = lmb_setup(); + ret = lmb_setup(true); if (ret) return ret;
In U-Boot, LMB and EFI are two primary modules who provide memory allocation and reservation API's. Both these modules operate with the same regions of memory for allocations. Use the LMB memory map update event to notify other interested listeners about a change in it's memory map. This can then be used by the other module to keep track of available and used memory. There is no need to send these notifications when the LMB module is being unit-tested. Add a flag to the lmb structure to indicate if the memory map is being used for tests, and suppress sending any notifications when running these unit tests. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V3: * Drop use of is_addr_in_ram() function * Drop use of CONFIG_MEM_MAP_UPDATE_NOTIFY symbol to check if the notification needs to be sent. * s/lmb_notify/lmb_should_notify * Put a check for EFI_LOADER in the lmb_should_notify() function include/efi_loader.h | 14 +++++ include/lmb.h | 2 + lib/efi_loader/efi_memory.c | 2 +- lib/lmb.c | 104 +++++++++++++++++++++++++++++++----- 4 files changed, 109 insertions(+), 13 deletions(-)