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

[Gabe Black]

On Sat, Dec 3, 2011 at 4:52 PM, Graeme Russ <graeme.russ at gmail.com> wrote:

> Hi Gabe,
>
> Last nit, I promise, and then I'll apply it all to u-boot-x86/next
> >
> > +/* Implementation defined function to install an e820 map. */
> > +unsigned install_e820_map(unsigned max_entries, struct e820entry *);
> > +
>
> Should be u8 as boot_params.e820_entries is u8 (__u8 really, but we don't
> use the kernel data types)
>
>
I don't think this is a good idea. The value here isn't consumed by the
boot_params structure, it's taken from the size of the e820 array there and
consumed by whatever function is written to copy over e820 entries. That
function doesn't really care about the boot_params structure at all. It
could be used to fill in entries for, say, some future boot protocol,
debugging where you want to see all entries beyond the first 256 (a lot,
but EFI tends to have a lot), or maybe I want to boot some other OS that
doesn't use a u8 to determine the size of the e820 table. There isn't any
danger of the size being mismatched because it's called with the size of
the array being filled, and the boot_params structure is smart enough to
use a data type that's sized appropriately for the array.

Gabe