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

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Aug 28 18:27:51 CEST 2007


On 28.08.2007 18:11, ron minnich wrote:
> 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.

AFAIK Uwe has a patch doing exactly that.

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

I like it.
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel




More information about the coreboot mailing list