[coreboot] [PATCH] Multiboot

[Stefan Reinauer]

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.

Glad to see your patch is able to reduce the code size though; even if
it comes at some cost.

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866