On Feb 16, 2008 7:07 AM, Stefan Reinauer stepan@coresystems.de wrote:
- ron minnich rminnich@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