[PATCH V4 1/8] spl: guard u_boot_any with X86

[Peng Fan (OSS)]

> Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
> 
> On 21/05/2022 15:05, Tom Rini wrote:
> > On Sat, May 21, 2022 at 08:33:56AM +0000, Peng Fan wrote:
> >>> Subject: Re: [PATCH V4 1/8] spl: guard u_boot_any with X86
> >>>
> >>> On Fri, May 20, 2022 at 10:10:40PM +0800, Peng Fan (OSS) wrote:
> >>>
> >>>> From: Peng Fan <peng.fan at nxp.com>
> >>>>
> >>>> set the symbol as weak not work if LTO is enabled. Since u_boot_any
> >>>> is only used on X86 for now, so guard it with X86, otherwise build
> >>>> break if we use BINMAN_SYMBOLS on i.MX.
> >>>>
> >>>> Tested-by: Tim Harvey <tharvey at gateworks.com> #imx8m[m,n,p]-venice
> >>>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> >>>> ---
> >>>>  common/spl/spl.c     | 8 ++++++--
> >>>>  common/spl/spl_ram.c | 4 ++++
> >>>>  2 files changed, 10 insertions(+), 2 deletions(-)
> >>>
> >>> I think we long term need to figure this out and address it so LTO
> >>> works.  But for now can you please guard this with a test on LTO
> >>> instead, so it's clear where the problem is?
> >>
> >> Sorry, I could not get your point about guard with a test on LTO.
> >>
> >> Actually binman weak symbol will report a warning log if there is no
> >> u_boot_any binman symbol. Since only X86 use it, I guard with X86.
> >
> > Why are you mentioning LTO in the commit message?  When I read the
> > commit message it sounds like you're saying the problem is that LTO
> > doesn't like how this symbol is handled, but if LTO was disabled,
> > everything would be fine.  If it's not LTO-related, please re-word the
> > message instead.
> 
> It looks like we should be able to change things in common/spl/spl.c to:
> 
>     #if CONFIG_IS_ENABLED(BINMAN_SYMBOLS)
>     /* See spl.h for information about this */
>     binman_sym_declare_optional(ulong, u_boot_any, image_pos);
>     binman_sym_declare_optional(ulong, u_boot_any, size);
>     #endif
> 
> which would mark the symbol as 'weak' and turn the error into a warning on
> the binman side. But that is somehow being undone by LTO.
> 
> I'm trying to build for imx8mm-beacon with that change instead of this patch.
> With CONFIG_LTO=y, build fails and spl/u-boot-spl.sym has:
> 
> > 00000000007fbe28 l     O .binman_sym	0000000000000008
> _binman_u_boot_any_prop_image_pos
> 
> Looks like the size symbol is optimized out. With CONFIG_LTO unset, the build
> succeeds and the same file has:
> 
> > 00000000007fe90c  w    O .binman_sym	0000000000000008
> _binman_u_boot_any_prop_image_pos
> > 00000000007fe904  w    O .binman_sym	0000000000000008
> _binman_u_boot_any_prop_size
> 
> I don't know much about linking stuff, so this is as deep as I could dig...

Thanks for the detailed sharing. Yes, this is the issue I try to work around. Some
i.MX boards has LTO set, this leads me to use X86 as an extra guard.

Thanks,
Peng.