[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Simon Glass]

Hi Bin,

On Sun, 26 Apr 2020 at 19:47, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 27, 2020 at 3:45 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Thu, 23 Apr 2020 at 03:17, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > At present this function copies U-Boot from the last 1MB of ROM. This is
> > > > not the right way to do it. Instead, the binman symbol should provide the
> > > > location.
> > > >
> > > > But in any case the code should live in the caller,
> > > > spl_board_load_image(), so that the 64-bit jump function can be used
> > > > elsewhere. Move it.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > ---
> > > >
> > > >  arch/x86/cpu/i386/cpu.c | 10 ----------
> > > >  arch/x86/lib/spl.c      | 13 +++++++++++++
> > > >  2 files changed, 13 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> > > > index c8da7f10e9b..45416a9be5b 100644
> > > > --- a/arch/x86/cpu/i386/cpu.c
> > > > +++ b/arch/x86/cpu/i386/cpu.c
> > > > @@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
> > > >
> > > >         func = (func_t)ptr;
> > > >
> > > > -       /*
> > > > -        * Copy U-Boot from ROM
> > > > -        * TODO(sjg at chromium.org): Figure out a way to get the text base
> > > > -        * correctly here, and in the device-tree binman definition.
> > > > -        *
> > > > -        * Also consider using FIT so we get the correct image length and
> > > > -        * parameters.
> > > > -        */
> > > > -       memcpy((char *)target, (char *)0xfff00000, 0x100000);
> > > > -
> > > >         /* Jump to U-Boot */
> > > >         func((ulong)pgtable, 0, (ulong)target);
> > > >
> > > > diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> > > > index 90baec2a175..95a89c072d5 100644
> > > > --- a/arch/x86/lib/spl.c
> > > > +++ b/arch/x86/lib/spl.c
> > > > @@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
> > > >         spl_image->os = IH_OS_U_BOOT;
> > > >         spl_image->name = "U-Boot";
> > > >
> > > > +       if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {
> > >
> > > For the coreboot case, where does it load the image then?
> >
> > It is already loaded. The combined SPL/U-Boot image is what is loaded.
> > Then we boot into SPL and jump to U_Boot. This is how tegra works,
> > since it needs two different toolchains.
>
> So if I understand this correctly, coreboot loads U-Boot SPL + 64-bit
> U-Boot as one payload all together hence there is no need memcpy? Is
> adding a coreboot config check here is to speed up the boot time to
> avoid the memcpy? If adding memcpy(), does anything break?

In this case yes, since the U-Boot address is wrong. But I suspect it
would be possible to set it correctly so that copying is possible.

But it would be quite strange. The SPL+U-Boot is in a CBFS file and
not really under control of U-Boot. So I think trying to copy it again
would be quite fragile.

Regards,
Simon