jordan.crouse at amd.com
Wed Sep 17 18:18:28 CEST 2008
On 17/09/08 17:05 +0200, Stefan Reinauer wrote:
> Robert Millan wrote:
> > On Wed, Sep 17, 2008 at 11:50:07AM +0200, Peter Stuge wrote:
> >> Robert Millan wrote:
> >>> Void-ify a few return types that assume presence of a native
> >>> coreboot table (and weren't actually used for anything).
> >>> Signed-off-by: Robert Millan <rmh at aybabtu.com>
> >> Sorry, but I still don't understand the motivation for this. Tell me
> >> why it's needed and I'll probably ack straight away. :)
> > With my Multiboot patch, one can no longer assume native coreboot tables
> > unconditionally, since Multiboot might be used instead. coreboot_table.c,
> > which provides get_lb_mem(), might not be linked in.
> > However, arch_write_tables() uses get_lb_mem() unconditionally as its return
> > value. But I observed that this value isn't actually used for anything. The
> > caller (write_tables()) in turn uses it as return value, and then it is
> > discarded by stage2().
> > So, rather than introducing a bunch of ifdefs to conditionalize this, my
> > patches opt to remove it. This way both build options get the size benefit.
> > Also, IMHO it didn't make much sense for a function that generates different
> > types of data structures (write_tables()) to have a return value that is
> > specific to only one of these.
> Just my 2ct:
> I have a bit of a hick-up with removing the coreboot table, as this
> renders utility like flashrom (in some cases) and nvramtool (in all
> cases) pretty much unusable.
> Also, I think bootloader stuff is libpayload/filo/grub2 territory.
I'm not groking the resistance to these tables. The coreboot tables
are good, but they have the distinct disadvantage of not being groked
by anything out side of our little ecosystem of payloads. Tables are
cheap, and it doesn't hurt us at all to include as many as we can.
More information about the coreboot