On Wed, Feb 12, 2025 at 03:11:16PM +0530, Sughosh Ganu wrote:
> On Tue, 11 Feb 2025 at 20:25, Tom Rini <tr...@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.g...@linaro.org>
> > > wrote:
> > > >
> > > > On Tue, 11 Feb 2025 at 19:24, Tom Rini <tr...@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 <a...@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 <a...@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
> > > > > > > > > > <a...@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;
}
--
Tom
signature.asc
Description: PGP signature
