[PATCH 1/5] x86: fsp: Allow skipping init code when chain loading

[Bin Meng]

Hi Simon,

On Tue, Feb 4, 2020 at 12:20 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 3 Feb 2020 at 04:05, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sun, Dec 22, 2019 at 12:13 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > When U-Boot is no the first-stage bootloader much of this code is not
> > > needed and can break booting. Add checks for this to the FSP code.
> > >
> > > Rather than checking for the amount of available SDRAM, just use 1GB in
> > > this situation, which should be safe. Using 2GB may run into a memory
> > > hole on some SoCs.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > >  arch/x86/lib/fsp/fsp_dram.c     |  8 ++++++++
> > >  arch/x86/lib/fsp/fsp_graphics.c |  3 +++
> > >  arch/x86/lib/fsp2/fsp_dram.c    | 10 ++++++++++
> > >  arch/x86/lib/fsp2/fsp_init.c    |  2 +-
> > >  4 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/lib/fsp/fsp_dram.c b/arch/x86/lib/fsp/fsp_dram.c
> > > index 9ce0ddf0d3..15e82de2fe 100644
> > > --- a/arch/x86/lib/fsp/fsp_dram.c
> > > +++ b/arch/x86/lib/fsp/fsp_dram.c
> > > @@ -44,6 +44,14 @@ int dram_init_banksize(void)
> > >         phys_addr_t low_end;
> > >         uint bank;
> > >
> > > +       if (!ll_boot_init()) {
> > > +               gd->bd->bi_dram[0].start = 0;
> > > +               gd->bd->bi_dram[0].size = gd->ram_size;
> > > +
> > > +               mtrr_add_request(MTRR_TYPE_WRBACK, 0, gd->ram_size);
> > > +               return 0;
> > > +       }
> > > +
> >
> > I wonder why this change is needed. When booting from other
> > bootloader, this dram_init_banksize() will not be called, no?
>
> How about this:
>
> It is useful to be able to boot the same x86 image on a device with or
> without a first-stage bootloader. For example, with chromebook_coral, it
> is helpful for testing to be able to boot the same U-Boot (complete with
> FSP) on bare metal and from coreboot. It allows checking of things like
> CPU speed, comparing registers, ACPI tables and the like.
>
> The idea is to change ll_boot_init() to false, and rebuild without changing
> anything else.

This sounds like for a debugging purpose, instead of a supported
configuration. So when we boot from previous bootloader, we really
should use its configuration, for the coreboot case,
coreboot-x86_defconfig should be used, and we can compare the
registers, ACPI tables in that configuration, instead of "hacking"
current U-Boot codes like in this series.

Regards,
Bin