[coreboot] [PATCH] Multiboot
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
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.
> - 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.
> - 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.
> - 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.
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...
Size: 1694 bytes
Desc: not available
More information about the coreboot