[coreboot] v3 patch rm elfboot

[ron minnich]

On Feb 16, 2008 7:07 AM, Stefan Reinauer <stepan at coresystems.de> wrote:
> * ron minnich <rminnich at gmail.com> [080215 22:09]:
> > OK, well elf is out of v3, at this point.
>
> Can we please put this back in? preparsing added so many problems and
> complications that it almost seems right to drop the idea again. The ELF
> loader wasn't all that big of an issue all the time, so I really don't
> understand the rush without having a correctly working alternative.

Let's discuss the problems it added. I have not yet had any, but if we
added problems, let's talk about what they are and what the fix might
be. I am sorry if it all seemed like a rush.

I was, myself, glad to get ELF processing out of coreboot, or at least
make it possible
not to use it.
The ELF processing in the firmware was always an issue. What I don't
like is the the following: we had to decompress the elf to some
location in memory just to find out where to put the ELF segments. It
was a multi-step process with lots of potential for failure.
So, the steps are
1. decompress elf to memory (where? Well, you have to pick a place.
How? a type of malloc)

2. parse the elf and look for errors. If there are errors, stop; you
can't boot. But you can figure this out at build time, and you should.
Figuring it out requires pre-parsing the ELF. If you have to pre-parse
the ELF, why not just put the ELF sections into LAR in the first
place?

3. Parse the ELF and figure out where to put the elf segments. If the
elf segments need to go where the decompressed data is, then the code
has to move memory around. And, note, you can't assume that the ELF
headers are at the beginning of the file -- they can be anywhere in
the file. [I believe the ELF parser in v2 got this wrong -- I had a
problem with it at some point. The ELF parser in v2 is not
sophisticated enough to deal with all the possible variations in an
ELF file. ] THIS ADDITIONAL "RELOCATE" STEP  WAS IN v2, but NOT in v3.
That's why the v3 code is so simple -- it can not handle all cases and
will fail if the ELF needs to be adjusted. elfboot.c in v3 is 215
lines, and in v2 it is almost 700. The binary for elfboot in v2 is
1600 bytes, for v3 it is 1 kbyte, and we've shown we don't need it.
Saving 1K in the boot block is a good thing. Adding back 600 bytes is
a bad thing. We would have to grow the boot block to add true ELF
support back into v3.


Compare to the pre-parsed elf:
1. Look at LAR entry, see where it goes, decompress to its
destination, done. And, the handling for payload is the same as for
all other LAR files -- no difference.

All error checking on ELF can be done when the firmware image is
created, not when the ELF is unpacked. There is not an extra
unnecessary copy step. The v2 ELF parsing is complex and I find the
code hard to understand, especially if I have not looked it in a
while. I saw the "error message and die" scenario more than once in
v2, and it never happens in v3.

To really do the v2 code correctly, we're going to have to add malloc
back. We're going to have to grow that code to handle headers in
different locations.

ELF pre-parsing is a big win in my view. I think resolving the issues
that it caused is easy -- we just have to agree on how to resolve
them. If nobody else wants to write it, I can write the code to
extract LAR entries into ELF files. It's really not that hard.

If we have decided that there is not sufficient information in a LAR
header to really recreate the ELF, then we have other options. One is
to make the LAR headers just be ELF headers. Second is to extend the
LAR headers to contain all the info we need.

But I really, really do not want to go back to using an ELF loader in
coreboot. I think removing ELF support in coreboot is the right thing
to do.

thanks

ron