[coreboot] patch: add romfs to V2

[Carl-Daniel Hailfinger]

On 31.03.2009 00:52, ron minnich wrote:
> This patch adds Jordan's ROMFS utility to v2.
>
> This patch adds Jordan's romtool support for v2. 
> There are a few changes. The 20K bootblock size restriction is gone. 
>
> ROMFS has been tested and works on v2 with qemu and kontron. Once this 
> patch is in, those patches will follow. 
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
>   

IIRC this was posted in December and did not get any reviews.

I do agree that v2 benefits from a ROM management scheme, considering
how many problems people have with payload handling in v2.

This is not even near a complete review, but I wanted to get some
comments on the code out there.
AFAICS the proposed ROMFS code will break on the Gigabyte GA-M57SLI-S4
v2.x. Since the M57SLI is one of the best supported recent boards out
there, I'd like to make supporting or not supporting one of its variants
an explicit rather than an implicit choice. Short summary of the issue:
ROMs larger than 512 kB or so can only be accessed in 3 byte chunks.
That means you have to read every header in chunks. Direct comparisons
in memory are not possible.
Besides that, I agree with Peter on naming when he wrote:
> It absolutely must get a different name. Several file systems exist
> in Linux already with names that are similar enough to cause a lot of
> unneccessary confusion.

And finally, I'd like to make sure we don't try to support multiple ROM
management schemes. That would be madness. If ROMFS is the future, can
we please use it in v3 as well?

The "pointer to romfs_header at the end of the ROM" idea was rejected
twice when I tried to get that in in v3. I still believe it is a good idea.
struct romfs_header is missing a struct member containing the lowest
accessible ROM address so you can switch to accessor functions for lower
addresses. This is needed if we ever want to support the M57SLI v2.x
with ROMFS.
romfs_header.version does not have to be endian-independent. If you can
read the magic correctly as a number, you got the endianness right. No
point in working around problems which do not exist.
"'pad' rounds the header to 32 bits and reserves a little room for later
use." Actually, that should read "32 bytes".
Can we use some magic other than "ORBC"?


Sorry for not giving a more complete review. I'm working against a
deadline for a paper and an unrelated presentation. I'll try to get back
to this sometime later.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/