[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