Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8

[Simon Glass <sjg@chromium.org>]

Hi Scott,

On Thu, 5 Aug 2021 at 13:20, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Scott,
>
> On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss@buserror.net> wrote:
> >
> > On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote:
> > > This symbol is needed for binman to locate the start of the image. Add it.
> > >
> > > Note: the existing line to bring in the .__image_copy_start symbol does
> > > not appear to do anything.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > > I have copied Scott Wood who originally added the line about the
> > > __image_copy_start in the hope that he can decide if we should remove it.
> >
> > It's been a long time since I looked at this stuff, but __image_copy_start is
> > used for relocation and that code does not seem to be in the SPL, so the
> > *(.__image_copy_start) was probably just a copy-and-paste leftover from the
> > main SPL that can go away.
> >
> > Of course, that doesn't resolve the binman issue. :-)
> >
> > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-
> > > spl.lds
> > > index 9edb662b094..2827a07590d 100644
> > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds
> > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> > > @@ -22,6 +22,7 @@ ENTRY(_start)
> > >  SECTIONS
> > >  {
> > >         .text : {
> > > +               __image_copy_start = .;
> > >                 . = ALIGN(8);
> > >                 *(.__image_copy_start)
> > >                 CPUDIR/start.o (.text*)
> >
> > If for whatever reason you did need to define the symbol this way, shouldn't
> > it be after the alignment?
>
> Well I don't want to miss out any of the image, otherwise the offsets
> would be off.
>
> But perhaps that is another question. Since this is the first section,
> it should already be aligned (to 16 I suspect). So why do we need it?

I'd like to get this applied, assuming it is correct, because at
present binman is silently ignoring problems where it cannot find
symbols. The fix for that is:

http://patchwork.ozlabs.org/project/uboot/patch/20210722210216.1.Id1246d1ff1cb5750f8c7ddde9665cf6f09615a7c@changeid/

which has been sitting there for a while.

Regards,
Simon