I realized during the discussion that there is a fundamental improvement we can make to cbfstool. This change I am detailing is compatible with existing coreboot libraries. I describe it below and attach the patch.
I have made a very simple mod to cbfstool that is compatible with the src/lib/ code in coreboot. I.e. the tool changes but the coreboot code does not.
Currently, as cbfstool manages the ROM, there are files and empty space. To allocate files, the code does, first, a walk of the headers and, if that fails, does a brute-force search of the rest of the space.
We all agree that the brute-force search is not a good idea. What I realized is that we should actually have 'empty space' as a cbfs file. This simple change greatly simplifies everything else.
But how do we get a new file? Simple: we find an empty file and use it, splitting it if necessary.
Result: all cbfs operations are now on headers, and nothing else. No walking of the ROM trying to find a header.
So I've made a slight change. Instead of an "empty space" area with no valid headers, I've made a header for the empty space.
Now creation of a new cbfs follows these steps: - set up the boot block - create a file, of type CBFS_COMPONENT_NULL, that contains the empty space. CBFS_COMPONENT_NULL was already defined in cbfs.h
Here's an example:
[rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs create 1048576 2048 (cbfstool) E: Unable to open (null): Bad address [rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs print testcbfs: 1024 kB, bootblocksize 2048, romsize 1048576, offset 0x0 Alignment: 16 bytes
Name Offset Type Size 0x0 0xffffffff 1046456
Note that the EMPTY area (type CBFS_COMPONENT_NULL) consumes the file area of the cbfs archive.
So how do we create a new cbfs file in the archive?
It's easy: walk the files and find a file of type CBFS_COMPONENT_NULL, which is as large or larger than the file you are trying to create. Then you use that cbfs file. - if the cbfs file is the same size as the NULL cbfs file, then it's easy: use it - if the cbfs file is smaller than the NULL file, you split the NULL cbfs file into two parts.
note that this works in the base case: the base case is that the whole storage is CBFS_COMPONENT_NULL.
Here's an example of adding a file. [rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs add-stage testfixed t [rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs print testcbfs: 1024 kB, bootblocksize 2048, romsize 1048576, offset 0x0 Alignment: 16 bytes
Name Offset Type Size t 0x0 stage 23176 0x5ab0 0xffffffff 1023240
Note that the NULL split and got smaller. But the entire ROM is still contained by the two files. To walk this entire rom will require two FLASH accesses -- no more.
Add another file: [rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs add-stage testfixed tt [rminnich@xcpu2 cbfstool]$ ./cbfstool testcbfs print testcbfs: 1024 kB, bootblocksize 2048, romsize 1048576, offset 0x0 Alignment: 16 bytes
Name Offset Type Size t 0x0 stage 23176 tt 0x5ab0 stage 23176 0xb560 0xffffffff 1000024 [rminnich@xcpu2 cbfstool]$
So, taking current ROMs as an example, I can reduce FLASH accesses for cbfs from (potentially) thousands to (typically) less than 10.
Note that I have changed deletion. To delete a file, we just burn the type to 0. This allows us to make a file unavailabe in FLASH without a block erase, and to put a new file of same name with, it is hoped, no block erase either.
Comments welcome. This change should be tested by anyone who can give it a try.
ron
On Fri, May 8, 2009 at 9:53 AM, ron minnich rminnich@gmail.com wrote:
I realized during the discussion that there is a fundamental improvement we can make to cbfstool. This change I am detailing is compatible with existing coreboot libraries. I describe it below and attach the patch.
I have made a very simple mod to cbfstool that is compatible with the src/lib/ code in coreboot. I.e. the tool changes but the coreboot code does not.
I think it is a step in the right direction. Could you add the function prototype to cbfstool.h?
Acked-by: Myles Watson mylesgw@gmail.com
Myles
Index: cbfstool/cbfstool.h =================================================================== --- cbfstool.orig/cbfstool.h 2009-05-08 13:07:42.000000000 -0600 +++ cbfstool/cbfstool.h 2009-05-08 13:07:43.000000000 -0600 @@ -71,6 +71,7 @@ struct cbfs_file *rom_find_first(struct rom *); struct cbfs_file *rom_find_next(struct rom *, struct cbfs_file *); int rom_add(struct rom *rom, const char *name, void *, int size, int type); +int rom_set_header(struct rom *rom, struct cbfs_file *c, const char *name, int size, int type); int rom_extract(struct rom *rom, const char *name, void **buf, int *size); int rom_remove(struct rom *rom, const char *name); int rom_used_space(struct rom *rom);
On Fri, May 8, 2009 at 12:08 PM, Myles Watson mylesgw@gmail.com wrote:
I think it is a step in the right direction. Could you add the function prototype to cbfstool.h?
Acked-by: Myles Watson mylesgw@gmail.com
Myles
With your prototype added: Committed revision 4261.
ron
On Fri, May 08, 2009 at 08:53:42AM -0700, ron minnich wrote:
Result: all cbfs operations are now on headers, and nothing else. No walking of the ROM trying to find a header.
Thanks Ron!
I did need the fixup patch you sent earlier today, but I've confirmed the result works with the existing SeaBIOS code.
-Kevin