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

[Simon Glass]

+Stefano Babic who might know


On Sat, 25 Sept 2021 at 16:36, Scott Wood <oss at buserror.net> wrote:
>
> On Sat, 2021-09-25 at 09:46 -0600, Simon Glass wrote:
> > Hi Scott,
> >
> > On Thu, 5 Aug 2021 at 13:20, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Scott,
> > >
> > > On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss at 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 at 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.
>
> I'm not sure what you need from me... as I said before, as far as I can tell
> the __image_copy_start stuff should not be needed by the SPL itself.  If you
> want to add it just for binman, there should probably be a comment to that
> effect, but I'm not the ARM maintainer (or involved in U-Boot development at
> all these days) so I can't help you get anything applied.
>
> As for why the alignment is there, it's probably just paranoia, but in any
> case, the SPL linker script probably shouldn't handle alignment in a
> gratuitously different way from the main linker script.
>
> -Scott
>
>