[coreboot] lar copy patch

[Myles Watson]

> -----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

> > 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*. 

> >      //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.

> >      //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.  

> 
> >      //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.

> > ... 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>
> >
> 
> Regards,
> Carl-Daniel

Thanks for looking over it.

Myles