[U-Boot] [PATCH 4/4] x86: Add infrastructure to extract an e820 table from the coreboot tables

[Gabe Black]

On Fri, Dec 2, 2011 at 1:14 PM, Graeme Russ <graeme.russ at gmail.com> wrote:

> Hi Gabe,
>
> On 30/11/11 17:07, Gabe Black wrote:
> > Signed-off-by: Gabe Black <gabeblack at chromium.org>
> > ---
> >  arch/x86/cpu/coreboot/sdram.c |   32 ++++++++++++++++++++++++++------
> >  arch/x86/include/asm/zimage.h |    5 +++++
> >  arch/x86/lib/zimage.c         |   10 ++++++++++
> >  3 files changed, 41 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/cpu/coreboot/sdram.c
> b/arch/x86/cpu/coreboot/sdram.c
> > index b5b086b..ce73467 100644
> > --- a/arch/x86/cpu/coreboot/sdram.c
> > +++ b/arch/x86/cpu/coreboot/sdram.c
> > @@ -23,6 +23,8 @@
> >   */
> >
> >  #include <common.h>
> > +#include <malloc.h>
> > +#include <asm/e820.h>
> >  #include <asm/u-boot-x86.h>
> >  #include <asm/global_data.h>
> >  #include <asm/ic/coreboot/sysinfo.h>
> > @@ -30,18 +32,36 @@
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +unsigned install_e820_map(unsigned max_entries, struct e820entry
> *entries)
> > +{
> > +     int i;
> > +
> > +     unsigned num_entries = min(lib_sysinfo.n_memranges, max_entries);
> > +     if (num_entries < lib_sysinfo.n_memranges) {
> > +             printf("Warning: Limiting e820 map to %d entries.\n",
> > +                     num_entries);
> > +     }
> > +     for (i = 0; i < num_entries; i++) {
> > +             struct memrange *memrange = &lib_sysinfo.memrange[i];
> > +
> > +             entries[i].addr = memrange->base;
> > +             entries[i].size = memrange->size;
> > +             entries[i].type = memrange->type;
> > +     }
> > +     return num_entries;
> > +}
> > +
> >  int dram_init_f(void)
> >  {
> >       int i;
> >       phys_size_t ram_size = 0;
> > +
> >       for (i = 0; i < lib_sysinfo.n_memranges; i++) {
> > -             unsigned long long end = \
> > -                     lib_sysinfo.memrange[i].base +
> > -                     lib_sysinfo.memrange[i].size;
> > -             if (lib_sysinfo.memrange[i].type == CB_MEM_RAM &&
> > -                     end > ram_size) {
> > +             struct memrange *memrange = &lib_sysinfo.memrange[i];
> > +             unsigned long long end = memrange->base + memrange->size;
> > +
> > +             if (memrange->type == CB_MEM_RAM && end > ram_size)
> >                       ram_size = end;
> > -             }
> >       }
> >       gd->ram_size = ram_size;
> >       if (ram_size == 0)
>
> Please fold these changes to dram_init_f() into the second patch
>
> > diff --git a/arch/x86/include/asm/zimage.h
> b/arch/x86/include/asm/zimage.h
> > index a02637f..b172048 100644
> > --- a/arch/x86/include/asm/zimage.h
> > +++ b/arch/x86/include/asm/zimage.h
> > @@ -24,6 +24,8 @@
> >  #ifndef _ASM_ZIMAGE_H_
> >  #define _ASM_ZIMAGE_H_
> >
> > +#include <asm/e820.h>
> > +
> >  /* linux i386 zImage/bzImage header. Offsets relative to
> >   * the start of the image */
> >
> > @@ -65,6 +67,9 @@
> >  #define BZIMAGE_LOAD_ADDR  0x100000
> >  #define ZIMAGE_LOAD_ADDR   0x10000
> >
> > +/* Implementation defined function to install an e820 map. */
> > +unsigned install_e820_map(unsigned max_entries, struct e820entry *);
> > +
> >  void *load_zimage(char *image, unsigned long kernel_size,
> >                 unsigned long initrd_addr, unsigned long initrd_size,
> >                 int auto_boot);
> > diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
> > index 6843ff6..1fde13f 100644
> > --- a/arch/x86/lib/zimage.c
> > +++ b/arch/x86/lib/zimage.c
> > @@ -51,6 +51,16 @@
> >
> >  #define COMMAND_LINE_SIZE    2048
> >
> > +unsigned generic_install_e820_map(unsigned max_entries,
> > +                               struct e820entry *entries)
> > +{
> > +     return 0;
> > +}
> > +
> > +unsigned install_e820_map(unsigned max_entries,
> > +                       struct e820entry *entries)
> > +     __attribute__((weak, alias("generic_install_e820_map")));
> > +
> >  static void build_command_line(char *command_line, int auto_boot)
> >  {
> >       char *env_command_line;
>
> I think all of the e820 code can be moved into your 32-bit boot protocol
> patch series
>
> Regards,
>
> Graeme
>


The changes which gather e820 information from the coreboot tables don't
really have anything specifically to do with the zboot command. The e820
information could be gathered another way (or generated on the spot like
you're doing) and the e820 info could be consumed by something else like
the bootm command. They are not a single logical change and should not be
merged.

Gabe