[LinuxBIOS] patch: extending LAR, and removing elf from linuxbios (it is not needed)

[ron minnich]

On 8/28/07, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
> OK, so I was unclear. The idea was to number the payloads as well, not
> only the segments.

Let's hold off on that, it will require a lot of changes, and it is
not really what we planned. We have normal and fallback; there is
really nothing in the design for normal0, normal1, etc.

> We simply could reserve some space at the end of the header and use that
> for future extensions without having to change a version number. Only if
> the extension needs changes in earlier fields of the struct we would
> have to change the MAGIC.

It gets messy. We'd have to implement version # management in
lib/lar.c, and I think that's a different coding discussion which I
would rather not include in this patch.

> OK, but please include your explanation as code comment.

done.

> > note that the payload is now payload0, payload1, etc. I've extended
> > linuxbios to look for these. Note that you can now see all the things
> > that get loaded ;they're no longer hidden in an ELF header somewhere.
> > Elf failures are gone!
>
> Please update the comment above.

done


>
> Cosmetic (no space between * and name):
>  void *entry;
>  void *loadaddress;

fixed.

>
> >  };
> >
> >  /* Prototypes. */
> > @@ -77,5 +87,6 @@
> >  int copy_file(struct mem_file *archive, char *filename, void *where);
> >  int run_file(struct mem_file *archive, char *filename, void *where);
> >  int execute_in_place(struct mem_file *archive, char *filename);
> > -
> > +int run_address(void *f);
> > +void * load_file(struct mem_file *archive, char *filename);
>
> Same cosmetic.
>

done

> Can you add your explanation from above?

done

===================================================================
> > --- lib/lzmadecode.c  (revision 480)
> > +++ lib/lzmadecode.c  (working copy)
> > @@ -206,7 +206,6 @@
> >          RC_GET_BIT(probLit, symbol)
> >        }
> >        previousByte = (Byte)symbol;
> > -
>
> Empty line removal. Was this intentional?

fixed.

> > +             if (walk[0] != 'L')
> >                       continue;
> >
> > +             if (strcmp(&walk[1], MAGIC) != 0)
> > +                     continue;
> > +
>
> There has to be a cleaner trick. Just in case somebody has the expected
> MAGIC string in an uncompressed lar file somewhere (other sources of
> that are possible!), we still fail, but less often.

The trick here is that the 'L' is in code space. Yes, there has to be
a better way to do this.

> >               // FIXME: check checksum
> >
> >               if (strcmp(fullname, filename) == 0) {
> > +                     printk(BIOS_SPEW, "LAR: it matches %s @ %p\n", fullname, header);
>
> replace "it" with "filename"?

I revised the print messages, see what you think.

> > +                     printk(BIOS_SPEW, "start %p len %d reallen %d compression %x entry %p loadaddress %p\n",
> > +                             result->start, result->len, result->reallen, result->compression, result->entry, result->loadaddress);
>
> Any chance to break that into multiple lines?

I did what I could, I added all your cosmetic changes too.

thanks, patch attached again

ron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: noelf4.diff
Type: text/x-patch
Size: 20230 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070828/d4521452/attachment.diff>