AFAIK not all architectures start executing code at the top of ROM, some start at the bottom (low address). In that case, we can't start the archive with a LAR header, but we can end it with one or postfix the first archive member with a LAR header. To achieve that, the patch redefines offset as signed instead of unsigned and allows the lar header after the boot block to point to the boot block at the beginning. That way, even bottom boot architectures can use the existing header format unless the first LAR member is bigger than 2 GB. This patch also introduces a 2 GB limit for file names in LARchives and a 2 GB limit for unused space between header and data. There is no new restriction for member lengths or archive length.
Current LAR archives look like this:
Hn=header n -=padding Dn=data n H0D0D0D0H1D1----H2D2D2 where the last bytes of D2 are executed after poweron.
The patch (together with signed offset) makes archives like the below one possible: D0D0D0H0H1D1----H2D2D2 (header after boot block points to boot block)
The new archive format possibility allows placing a boot block at the beginning without sacrificing the goal of one common archive format. However, there are two potential problems: - Endless loops if we aren't careful when walking the LAR. I think I got all corner cases right. - "Nobody will need more than 640kB", in our case it would be 2GB boot blocks.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: LinuxBIOSv3/include/lar.h =================================================================== --- LinuxBIOSv3/include/lar.h (Revision 503) +++ LinuxBIOSv3/include/lar.h (Arbeitskopie) @@ -52,17 +52,24 @@
#include <types.h>
-/* see note in lib/lar.c as to why this is ARCHIVE and not LARCHIVE */ #define MAGIC "LARCHIVE" #define MAX_PATHLEN 1024 -/* NOTE -- This and the user-mode lar.h are NOT IN SYNC. Be careful. */ +/* NOTE -- This and the user-mode lar.h may NOT be IN SYNC. Be careful. */ struct lar_header { char magic[8]; u32 len; u32 reallen; u32 checksum; u32 compchecksum; - u32 offset; + /* The offset is signed to allow header after data for the first member + * of the LAR. This is needed if code execution starts at bottom of + * flash instead of at top of flash (x86). Please note that a + * header-after-data LAR member can't be larger than 2^31 bytes. + * Right now, we are about five orders of magnitude away from such a + * beast. + * Filenames are limited to 2^31-1-sizeof(lar_header)-1 bytes. + * "Nobody will ever need more than 640k" */ + s32 offset; /* Compression: * 0 = no compression * 1 = lzma Index: LinuxBIOSv3/lib/lar.c =================================================================== --- LinuxBIOSv3/lib/lar.c (Revision 503) +++ LinuxBIOSv3/lib/lar.c (Arbeitskopie) @@ -49,7 +49,7 @@ * Given a file name in the LAR , search for it, and fill out a return struct with the * result. * @param archive A descriptor for current archive. This is actually a mem_file type, - * which is a machine-dependent representation of hte actual archive. In particular, + * which is a machine-dependent representation of the actual archive. In particular, * things like u32 are void * in the mem_file struct. * @param filename filename to find * @param result pointer to mem_file struct which is filled in if the file is found @@ -90,7 +90,7 @@ * Unsigned pointer arithmetic that wraps to zero can be messy. */ for (walk = archive->start; - (walk < (char *)(archive->start + archive->len - sizeof(struct lar_header))) && + (walk <= (char *)(archive->start + archive->len - sizeof(struct lar_header))) && (walk >= (char *)archive->start); walk += 16) { if (strncmp(walk, MAGIC, 8) != 0) continue; @@ -98,12 +98,14 @@ header = (struct lar_header *)walk; fullname = walk + sizeof(struct lar_header);
- printk(BIOS_SPEW, "LAR: search for %s\n", fullname); + printk(BIOS_SPEW, "LAR: seen member %s\n", fullname); // FIXME: check checksum
if (strcmp(fullname, filename) == 0) { printk(BIOS_SPEW, "LAR: CHECK %s @ %p\n", fullname, header); - result->start = walk + ntohl(header->offset); + /* In the header-before-member case offset is at least + * sizeof(header) + strlen(filename) + 1 */ + result->start = walk + (s32)ntohl(header->offset); result->len = ntohl(header->len); result->reallen = ntohl(header->reallen); result->compression = ntohl(header->compression); @@ -115,11 +117,41 @@ result->compression, result->entry, result->loadaddress); return 0; } - // skip file - walk += (ntohl(header->len) + ntohl(header->offset) - - 1) & 0xfffffff0; + /* skip file: + * The next iteration of the for loop will add 16 to walk, so + * we now add offset (from header start to data start) and len + * (member length), subtract 1 (to get the address of the last + * byte of the member) and round this down to the next 16 byte + * boundary. + * In the case of consecutive archive members with header- + * before-member structure, the next iteration of the loop will + * start exactly at the beginning of the next header. + * In the case of header-after-member (e.g. for bottom boot + * architectures) the calculation below will effectively cause + * walk < header. To elaborate a bit more, in this case + * (header->offset + header->len - 1) will evaluate to a value + * between -1 (header directly after file), -16 (file, 15 bytes + * pad, header), and even smaller values if there is 16 bytes + * or more of padding between member and header. The outer + * expression will usually evaluate to 0xfffffff0, cause the + * expected overflow with unsigned arithmetic and result in a + * decrease of walk. That condition can be checked. */ +#warning FIXME: This loop will explode if this code is ever compiled in 64bit mode +#warning because of the hardcoded 0xfffffff0. + walk += (ntohl(header->offset) + ntohl(header->len) - 1) + & 0xfffffff0; + /* If we have header-after-member, walk < header is true. + * Go forward instead by starting at header, adding header size + * and strlen(fullname). The result of this calculation is the + * position of the terminating \0 of fullname. Round that + * address down to the next 16 byte boundary. */ + if (walk < (char *)header) { + walk = (char *)header; + walk += (sizeof(struct lar_header) + strlen(fullname)) + & 0xfffffff0; + } } - printk(BIOS_SPEW, "NO FILE FOUND\n"); + printk(BIOS_SPEW, "LAR: NO FILE FOUND\n"); return 1; }
Index: LinuxBIOSv3/util/lar/lar.h =================================================================== --- LinuxBIOSv3/util/lar/lar.h (Revision 503) +++ LinuxBIOSv3/util/lar/lar.h (Arbeitskopie) @@ -60,15 +60,24 @@ typedef uint64_t u64; typedef uint32_t u32; typedef uint8_t u8; +typedef int32_t s32;
-/* NOTE -- This and the linuxbios lar.h are NOT IN SYNC. Be careful. */ +/* NOTE -- This and the user-mode lar.h may NOT be IN SYNC. Be careful. */ struct lar_header { char magic[8]; u32 len; u32 reallen; u32 checksum; u32 compchecksum; - u32 offset; + /* The offset is signed to allow header after data for the first member + * of the LAR. This is needed if code execution starts at bottom of + * flash instead of at top of flash (x86). Please note that a + * header-after-data LAR member can't be larger than 2^31 bytes. + * Right now, we are about five orders of magnitude away from such a + * beast. + * Filenames are limited to 2^31-1-sizeof(lar_header)-1 bytes. + * "Nobody will ever need more than 640k" */ + s32 offset; /* Compression: * 0 = no compression * 1 = lzma
On Fri, Oct 12, 2007 at 12:38:07AM +0200, Carl-Daniel Hailfinger wrote:
The patch (together with signed offset) makes archives like the below one possible: D0D0D0H0H1D1----H2D2D2 (header after boot block points to boot
..
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
I like this idea a lot. I have not tested the code but I will say..
Acked-by: Peter Stuge peter@stuge.se
Supported by the reasoning that if this breaks anything, it is still not too late to fix it. We can't be afraid to touch our own code.
//Peter
* Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net [071012 00:38]:
AFAIK not all architectures start executing code at the top of ROM, some start at the bottom (low address).
Are you working on porting LinuxBIOS to such an architecture currently?
In that case, we can't start the archive with a LAR header, but we can end it with one or postfix the first archive member with a LAR header.
I think this idea is massively over-engineered and makes things too complex.
Just add a jump into the "bootblock" member of the lar the first thing before the archive on those architectures that start their execution at the beginning of flash rather than anywhere in the middle.
This would be a perfectly valid lar, as lar copes with non-lar-data between the members. It was designed to do that. Lar could even produce that jump once it supports non-x86 architectures. (Obviously lar's bootblock handling can never be 100% generic)
Also, the bootblock knows where the lar sits, per design. So it will never have to scan in any direction, risking to loop indefinitely.
Let's keep things simple so that every idiot can understand it, even 6 months later.
Stefan
On Fri, Oct 12, 2007 at 11:55:26AM +0200, Stefan Reinauer wrote:
Just add a jump into the "bootblock" member of the lar the first thing before the archive on those architectures that start their execution at the beginning of flash rather than anywhere in the middle.
Just that it's "unfair" to only be able to identify top bootblocks.
This would be a perfectly valid lar, as lar copes with non-lar-data between the members. It was designed to do that.
Excellent point.
Lar could even produce that jump once it supports non-x86 architectures. (Obviously lar's bootblock handling can never be 100% generic)
Maybe it can - if lar is only used to manage locations and sizes rather than actual instructions. The rope only has two ends..
Let's keep things simple so that every idiot can understand it, even 6 months later.
You made a very good point, I still like the signed idea but it isn't at all needed in the short term. When we get popular LAR use on bottom bootblock archs we may have even better ideas.
//Peter
On 12.10.2007 12:21, Peter Stuge wrote:
On Fri, Oct 12, 2007 at 11:55:26AM +0200, Stefan Reinauer wrote:
- Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net [071012 00:38]:
AFAIK not all architectures start executing code at the top of ROM, some start at the bottom (low address).
Are you working on porting LinuxBIOS to such an architecture currently?
No, but I'd like to make architectural decisions before they become a problem in porting when time is short. Rapidly hacked solutions often do not achieve long-term code understandability.
Just add a jump into the "bootblock" member of the lar the first thing before the archive on those architectures that start their execution at the beginning of flash rather than anywhere in the middle.
Just that it's "unfair" to only be able to identify top bootblocks.
But then we would need special code sequence emission for bottom boot architectures. That is neither portable nor can it be handled automatically, i.e. you have to specify the instruction architecture when creating/modifying a LAR archive.
This would be a perfectly valid lar, as lar copes with non-lar-data between the members. It was designed to do that.
Excellent point.
It is a valid lar, but you have to add special casing all over the lar utility when reading/modifying such archives or you will lose that information.
Lar could even produce that jump once it supports non-x86 architectures. (Obviously lar's bootblock handling can never be 100% generic)
Maybe it can - if lar is only used to manage locations and sizes rather than actual instructions. The rope only has two ends..
I'd like lar to be a pure archive tool, not a code sequence emission tool as well. That way, you don't have to care about the destination architecture, only about its top/bottom boot block property.
Let's keep things simple so that every idiot can understand it, even 6 months later.
You made a very good point, I still like the signed idea but it isn't at all needed in the short term. When we get popular LAR use on bottom bootblock archs we may have even better ideas.
Maybe. I think the code and the comments are both very clear on methodology and purpose. The log message could be added to the lar documentation, but otherwise I think the code is explained better than it was before. Besides that, the idea has been met with approval on IRC a few weeks ago (IIRC Jordan Crouse was the one poking me to document the old/new limits better). Ron wrote in a mail: "OK, I like signed offset, but let's let stefan weigh in. It's critical that we get this right."
Stefan: Your complaint about my solution being over-engineered has some truth in it, but the solution carefully avoids big special casing and code bloat. The only added code for all new functionality is
+ if (walk < (char *)header) { + walk = (char *)header; + walk += (sizeof(struct lar_header) + strlen(fullname)) + & 0xfffffff0; + }
and all other changes are either better code comments or functionally equivalent (whitespace / argument reordering) transformations. I doubt there is a smaller and cleaner solution for bottom boot architectures.
Regards, Carl-Daniel
On Fri, Oct 12, 2007 at 02:54:41PM +0200, Carl-Daniel Hailfinger wrote:
The only added code for all new functionality is
if (walk < (char *)header) {walk = (char *)header;walk += (sizeof(struct lar_header) + strlen(fullname))& 0xfffffff0;}and all other changes are either better code comments or functionally equivalent (whitespace / argument reordering) transformations.
Please make several patches out of that. Easier to review then. :)
//Peter
On 12.10.2007 15:05, Peter Stuge wrote:
On Fri, Oct 12, 2007 at 02:54:41PM +0200, Carl-Daniel Hailfinger wrote:
The only added code for all new functionality is
if (walk < (char *)header) {walk = (char *)header;walk += (sizeof(struct lar_header) + strlen(fullname))& 0xfffffff0;}and all other changes are either better code comments or functionally equivalent (whitespace / argument reordering) transformations.
Please make several patches out of that. Easier to review then. :)
Will do. Thanks for the review.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Peter Stuge -
Stefan Reinauer