[coreboot] lar copy patch
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Thu Mar 13 02:05:53 CET 2008
Hi Myles,
I think we're moving forward and one or two iterations should give us a
mergeable patch.
On 07.03.2008 17:11, Myles Watson wrote:
>> -----Original Message-----
>> From: Carl-Daniel Hailfinger [mailto:c-d.hailfinger.devel.2006 at gmx.net]
>> Sent: Thursday, March 06, 2008 6:35 PM
>> To: Myles Watson
>> Cc: Coreboot
>> Subject: Re: [coreboot] lar copy patch
>>
>> On 07.03.2008 00:47, Myles Watson wrote:
>>
>>> This patch adds copy functionality to lar via
>>>
>>> lar -L srclar.file
>>>
>>>
>> Although I appreciate the work you've done, I still feel the lar copy
>> approach is not right.
>>
>
> The other options I've seen are:
> 1. Invent a new file format
> 2. Extract to ELF minus some information
>
The "Extract to ELF" option would work if Ron had the time to do it. But
since he's busy with so many coreboot things, I can't really force him
to work on that issue as well.
>>> It allows you to use payloads, bootblocks, or other entries from one
>>> lar when creating or adding to another.
>>>
>>> changes by file:
>>> util/lar/lar.c:
>>> change usage
>>> add srcarchivename parameter for create_lar and add_lar
>>> add lar_copy functionality to create_lar and add_lar
>>> add -C keep for using the same compression as srclar
>>> util/lar/lar.h:
>>> change invalid to invalid_or_keep in algo_strings
>>>
>>> util/lar/lib.c:
>>> pull out function add_entry from add_files
>>>
>>> util/lar/lib.h:
>>> add prototypes for add_entry and lar_copy
>>> change prototype for create_lar (wonder why add_lar doesn't have
>>>
>> one)
>>
>>> fix whitespace
>>>
>>> util/lar/stream.c:
>>> pull out helper function file_in_list_where from file_in_list
>>> add lar_copy
>>>
>>> This is a test of the copy functionality of lar. I'll use C++-style
>>>
>> comments
>>
>>> // here's the original lar file
>>> [myles at orangutan svn]$ build/util/lar/lar -l build/coreboot.rom
>>> normal/option_table (932 bytes @0x50);loadaddress 0x0 entry 0x0
>>> normal/initram/segment0 (432 bytes @0x450);loadaddress 0x0 entry
>>>
>> 0x0x42
>>
>>> normal/stage2/segment0 (191792 bytes, zeroes compressed to 1 bytes
>>> @0x650);loadaddress 0x0xa540 entry 0x0x2000
>>> normal/stage2/segment1 (28172 bytes, lzma compressed to 15083 bytes
>>> @0x6b0);loadaddress 0x0x2000 entry 0x0x2000
>>> normal/stage2/segment2 (5420 bytes, lzma compressed to 312 bytes
>>> @0x41f0);loadaddress 0x0x9000 entry 0x0x2000
>>> bootblock (20480 bytes @0xfb000)
>>> Total size = 37688B 36KB (0x9338)
>>>
>>> //create a new 256K lar file called bootblock.rom which has the
>>> same bootblock as coreboot.rom
>>> [myles at orangutan svn]$ build/util/lar/lar -c -s 256K bootblock.rom -L
>>> build/coreboot.rom
>>>
>>> //create a new lar file with uncompressed normal/stage2
>>> [myles at orangutan svn]$ build/util/lar/lar -c -s 256K normal_stage2.rom
>>> -L build/coreboot.rom nocompress:normal/stage2:stage2
>>>
>>>
>> Wait. You add a file with path "stage2" as "normal/stage2"? That's wrong.
>>
>
> That's backward. The first part of the argument is the original or file
> name. I'm adding the entries from build/coreboot.rom named normal/stage2*
> and renaming them stage2*.
>
Thanks for clearing this up.
>>> //create a new lar file with lzma compressed normal/init
>>> [myles at orangutan svn]$ build/util/lar/lar -c -s 256K normal_init.rom
>>> -L build/coreboot.rom -C lzma normal/init
>>>
>>>
>> What is normal/init? That thing does not exist as path.
>>
>
> I changed the matching to greedy matching in an earlier path, since it makes
> sense to get the whole payload at once. In other words, when you extract
> normal/payload, you get normal/payload/segment0, normal/payload/segment1,
> etc. I put this in there to make that clear.
>
Sorry, had I known the greedy match didn't match at path boundaries, I
would have NACKed it. Having "normal/in" match "normal/initram/segment0"
as well as "normal/in" as well as "normal/interesting" is horrible.
>>> //create a new lar file with normal/opt compressed the same as it
>>>
>> was
>>
>>> [myles at orangutan svn]$ build/util/lar/lar -c -s 256K normal_opt.rom -L
>>> build/coreboot.rom -C keep normal/opt
>>>
>>> //create a new 1M lar using the bootblock in bootblock.rom
>>> [myles at orangutan svn]$ build/util/lar/lar -c -s 1M new.rom -L
>>>
>> bootblock.rom
>>
>> How does LAR know that you want to copy the bootblock instead of an
>> empty operation?
>>
>
> Since it is a create operation with a source lar, it copies the bootblock.
> It made sense to me that if you want a new lar starting with an old one, you
> were asking for the bootblock.
>
That's an implicit undocumented action. It will trigger a lot of head
scratching in the future.
>>> //add back in the components with the correct compression
>>> [myles at orangutan svn]$ build/util/lar/lar -a new.rom -L normal_opt.rom
>>> -C none normal/
>>> [myles at orangutan svn]$ build/util/lar/lar -a new.rom -L
>>> normal_init.rom -C none normal/init
>>>
>>> //use the verbose switch to see what you're getting
>>> [myles at orangutan svn]$ build/util/lar/lar -av new.rom -L
>>> normal_stage2.rom -C lzma stage2:normal/stage2
>>>
>>>
>> Here the stange stage2 mentioning happens again.
>>
>>
>
> This is the opposite of what happened earlier. Now I'm taking the files
> named stage2* and putting them into the new lar with base name normal/stage2
> so that the new lar will have the same format as build/coreboot.rom.
>
OK.
>>> ... adding stage2:normal/stage2
>>> stage2:normal/stage2 (lzma) matches stage2/segment0 =>
>>>
>> normal/stage2/segment0.
>>
>>> stage2:normal/stage2 (lzma) matches stage2/segment1 =>
>>>
>> normal/stage2/segment1.
>>
>>> stage2:normal/stage2 (lzma) matches stage2/segment2 =>
>>>
>> normal/stage2/segment2.
>>
>>> //diff the two files
>>> [myles at orangutan svn]$ diff new.rom build/coreboot.rom
>>> // There's no difference
>>>
>>> I think it would be nice if lar would warn you (or stop you) when you
>>> try to add a duplicate entry. Maybe that will be the next patch.
>>>
>>> Myles
>>>
>>> Signed-off-by: Myles Watson <mylesgw at gmail.com>
>>>
One thing I didn't check yet: If you specify that compression of a file
should be changed, how does this affect the "zeroes" compression?
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list