[coreboot] lar copy patch

Myles Watson mylesgw at gmail.com
Thu Mar 13 04:35:03 CET 2008



> -----Original Message-----
> From: Carl-Daniel Hailfinger [mailto:c-d.hailfinger.devel.2006 at gmx.net]
> Sent: Wednesday, March 12, 2008 7:06 PM
> To: Myles Watson
> Cc: 'Coreboot'
> Subject: Re: [coreboot] lar copy patch
> 
> Hi Myles,
> 
> I think we're moving forward and one or two iterations should give us a
> mergeable patch.

I appreciate the feedback.

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


Sure.

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

This is easy to fix.  It was a toss-up to me because there are so few
entries in a lar that there will be few cases where one would choose to name
something normal/in and normal/interesting and not want them both out.

I also wondered about having it only match if the nesting wasn't too deep.
For example normal wouldn't match normal/payload/segment0, but would match
normal/initram.  I could argue that one either way.

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

I could document it with a warning when no bootblock is specified, so that
it's clear what's happening.

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

It doesn't.  Zeroes is a special case, and doesn't get changed.

Thanks,
Myles






More information about the coreboot mailing list