[coreboot] [PATCH] Multiboot

Robert Millan rmh at aybabtu.com
Tue Sep 16 16:35:46 CEST 2008


On Mon, Sep 15, 2008 at 10:54:20PM +0200, Carl-Daniel Hailfinger wrote:
> On 15.09.2008 21:49, Robert Millan wrote:
> > The attached patch adds a build option so that one can choose between
> > native coreboot tables and multiboot information (or both, or neither).
> >   
> 
> Have you tested it with a real preparsed payload?

Yes.  For testing, I recommend using the example Multiboot kernel from GRUB
Legacy sources.  You have to configure with --enable-example-kernel, and an
image is produced in docs/kernel.

> http://www.gnu.org/software/grub/manual/html_node/Features.html says
> that FreeBSD, NetBSD, OpenBSD, and Linux all lack multiboot compliance.
> Is that still true?

Actually, NetBSD's kernel supports Multiboot now.  This information is
part of GRUB Legacy and is outdated.

> If so, which real-world payloads are multiboot
> compliant?

See http://grub.enbug.org/MultibootSystems

Though, we probably miss many small ones.  It is common to use Multiboot in
new projects due the widespread of a readily available loader.

> > Ah, and before someone asks, yes memory map support is implemented ;-)
> 
> And it does not conform to the multiboot spec.

Yes, it does.  The relevant paragraph is in section 3.3, titled "Boot
information format", and starts with "If bit 6 in the `flags' word is set,".

I understand it's easy to miss details, specially in a new text one isn't
used to work with.  So please read that paragraph completely before
continuing with this discussion.

> Now, about the patch iself:
> - You change a few prototypes. Please provide a separate patch for that.

Attached.

> - Once multiboot support is active, payloads can't return anymore. This
> is a change in behaviour and needs documentation, review and function
> annotation (noreturn).

Actually, I didn't have a specific reason to use a jump instead of a call.
I'll change that.

> - Source code in header files. Please move to .c files.

Ok.

> - One call to get_lb_mem is removed for both multiboot and classic.
> Separate patch please.

This is related to the prototype change;  included in attached patch.

> - Any reasons for the slightly modified licence header in multiboot.c?

I believe the reason using an URL instead of a snail address is recommended
is that it is legally more solid (e.g. in case the FSF office moves or
something).  Anyway, it doesn't make a big difference;  I'll just use the
other text if you like that better.

> - Total absence of debug printk() statements. Please include at least a
> success message.

Ok.

> - Multiboot suport will not work if the payload is not preparsed.

I don't understand what you mean here.

> - Please investigate the possibility to add that multiboot code to
> libpayload so that Bayou can use it. Bayou is our recommended default
> payload chooser and it would be nice if Bayou could support multiboot.

Once base Multiboot support is in coreboot, if libpayload/bayou don't
overwrite the structures it'd be reasonably simple to reuse them by simply
passing on a pointer.  Just 3-4 lines of code I think.  I can have a look
later if you like.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: multiboot_0.diff
Type: text/x-diff
Size: 1694 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080916/abece89c/attachment.bin>


More information about the coreboot mailing list