Besides the obvious stuff (storing the VSA as a file in the LAR) there are two points to solve which are rather interesting: - Entry point specification - Load address specification Both are not trivial because we get both specifications above from an ELF header. The VSA is not an ELF file, so we have to specify entry point and load address by hand.
The following patch allows us to specify LAR entries like "entry=33:blobs/vsa" which then have entry point addr 33. This can be extended to a loadaddr= parameter.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: LinuxBIOSv3-laroptions/util/lar/lib.c =================================================================== --- LinuxBIOSv3-laroptions/util/lar/lib.c (Revision 559) +++ LinuxBIOSv3-laroptions/util/lar/lib.c (Arbeitskopie) @@ -207,8 +207,10 @@ char *realname; char *c;
- if (strstr(name, "nocompress:") == name) { - realname = strdup(name + 11); + printf("add_files called for %s\n", name); + /* search backwards to find the last option delimiter */ + if (strrchr(name, ':')) { + realname = strdup(strrchr(name, ':') + 1); } else { realname = strdup(name); } Index: LinuxBIOSv3-laroptions/util/lar/lib.h =================================================================== --- LinuxBIOSv3-laroptions/util/lar/lib.h (Revision 559) +++ LinuxBIOSv3-laroptions/util/lar/lib.h (Arbeitskopie) @@ -56,7 +56,7 @@
/* Prototypes for in-memory LAR operations */ int lar_process_name(char *name, char **pfilename, char **ppathname, - enum compalgo *thisalgo); + enum compalgo *thisalgo, u32 *entry); u32 lar_compress(char *ptr, ssize_t size, char *temp, enum compalgo *thisalgo); int lar_add_entry(struct lar *lar, char *pathname, void *data, u32 complen, u32 reallen, u32 loadaddress, u32 entry, Index: LinuxBIOSv3-laroptions/util/lar/stream.c =================================================================== --- LinuxBIOSv3-laroptions/util/lar/stream.c (Revision 559) +++ LinuxBIOSv3-laroptions/util/lar/stream.c (Arbeitskopie) @@ -671,20 +671,25 @@ * @return 0 success, or -1 on failure (i.e. a bad name) */ int lar_process_name(char *name, char **pfilename, char **ppathname, - enum compalgo *thisalgo) + enum compalgo *thisalgo, u32 *entry) { char *filename = name, *pathname = name;
- if (!strncmp(name, "nocompress:",11)) { - filename += 11; + if (strstr(name, "nocompress:")) { *thisalgo = none; } + if (strstr(name, "entry=")) { + *entry = strtoul(strstr(name, "entry=") + 6, NULL, 0); + } + if (strrchr(name, ':')) + filename = strrchr(name, ':') + 1;
/* this is dangerous */ if (filename[0] == '.' && filename[1] == '/') filename += 2;
- pathname = strchr(filename, ':'); + /* New pathname syntax: path=file */ + pathname = strchr(filename, '=');
if (pathname != NULL) { *pathname = '\0'; @@ -783,6 +788,7 @@ u32 *walk, csum; u32 offset;
+ printf("lar_add_entry called for %s, entry is %p\n", pathname, (void *)entry); /* Find the beginning of the available space in the LAR */ offset = lar_empty_offset(lar);
@@ -848,13 +854,18 @@ u32 complen; int pathlen; u32 size; + u32 entry = 0;
+ printf("lar_add_file: name is %s\n", name); + thisalgo = algo; - lar_process_name(name, &filename, &pathname, &thisalgo); + lar_process_name(name, &filename, &pathname, &thisalgo, &entry); + printf("lar_add_file: after processing: filename=%s, pathname=%s\n", filename, pathname);
ptr = mapfile(filename, &size);
if (elfparse() && iself(ptr)) { + printf("lar_add_file: handle elf\n"); output_elf_segments(lar, pathname, ptr, size, thisalgo); return 0; } @@ -878,7 +889,7 @@
munmap(ptr, size);
- ret = lar_add_entry(lar, pathname, temp, complen, size, 0, 0, thisalgo); + ret = lar_add_entry(lar, pathname, temp, complen, size, 0, entry, thisalgo);
free(temp); return ret;
On 23/01/08 02:33 +0100, Carl-Daniel Hailfinger wrote:
Besides the obvious stuff (storing the VSA as a file in the LAR) there are two points to solve which are rather interesting:
- Entry point specification
- Load address specification
Both are not trivial because we get both specifications above from an ELF header. The VSA is not an ELF file, so we have to specify entry point and load address by hand.
The following patch allows us to specify LAR entries like "entry=33:blobs/vsa" which then have entry point addr 33. This can be extended to a loadaddr= parameter.
Nak.
Seriously, why are we making this so difficult? All Geode code needs the VSA. _Only_ Geode code needs the VSA. All Geode code will put the code in the same place and load it the same way. All the code really needs from the LAR is a blob. find_blob('blobs/vsa') - thats it. The code will know where to put it, and what to do with it. Your method takes that knowledge out of the hands of the code and puts it in the hands of the user. That is a recipe for disaster.
I admit, I was the original LAR abuser - I took it from a simple command line option to a long list of options and flags, so its slightly hypocritical for me to complain now. But I think that we need to be careful to consider the practical aspects of what we are doing. We need to make things easier for the person building the ROM; regardless of if they use a tool or roll it by hand. Forcing the user to understand complex information like load addresses and such (which even _we_ will forget and have to look up on the wiki) is not productive. It is double plus not productive when the information we are asking them to provide is already static and well known.
Lets go back to basics on this one - put the blob in the LAR (easy to do), and then the code will know how to use it. Done and done.
Thanks, Jordan
Jordan Crouse wrote:
On 23/01/08 02:33 +0100, Carl-Daniel Hailfinger wrote:
Besides the obvious stuff (storing the VSA as a file in the LAR) there are two points to solve which are rather interesting:
- Entry point specification
- Load address specification
Both are not trivial because we get both specifications above from an ELF header. The VSA is not an ELF file, so we have to specify entry point and load address by hand.
The following patch allows us to specify LAR entries like "entry=33:blobs/vsa" which then have entry point addr 33. This can be extended to a loadaddr= parameter.
Nak.
Seriously, why are we making this so difficult? All Geode code needs the VSA. _Only_ Geode code needs the VSA. All Geode code will put the code in the same place and load it the same way. All the code really needs from the LAR is a blob. find_blob('blobs/vsa') - thats it. The code will know where to put it, and what to do with it. Your method takes that knowledge out of the hands of the code and puts it in the hands of the user. That is a recipe for disaster.
Full ack. If the code can make an optimal decision, there is absolutely no reason to create a bload-interface around it for a flexibility that noone is ever going to use. We have this in v2 in so many places, one of them is the image building/size logic in the device tree description file Config.lb (calculating fallback/normal offsets etc) This has caused so many headaches, and has actively scared people away from the project.
Lets go back to basics on this one - put the blob in the LAR (easy to do), and then the code will know how to use it. Done and done.
Yes. All we need to know about the VSA binary is its start address in the lar. There is no need whatsoever to add any extra code for it, except maybe making the build process aware of the fact that it needs to pack the vsa blob into the lar.
Stefan
On 23.01.2008 17:05, Stefan Reinauer wrote:
Jordan Crouse wrote:
Lets go back to basics on this one - put the blob in the LAR (easy to do), and then the code will know how to use it. Done and done.
Yes. All we need to know about the VSA binary is its start address in the lar. There is no need whatsoever to add any extra code for it, except maybe making the build process aware of the fact that it needs to pack the vsa blob into the lar.
What about compression of the VSA?
Regards, Carl-Daniel
On Jan 24, 2008 6:34 AM, Carl-Daniel Hailfinger
What about compression of the VSA?
On v3, we have the 'use compression' option in the Kconfig dialog. Long term, I intend to use the same compression for vsa that we use for everything else, and to decompress it as is done in v2, but use whatever compression the rest of coreboot is using.
For now, I'm going to run it from ROM. Yes, it will be slower.
ron
On 24.01.2008 17:02, ron minnich wrote:
On Jan 24, 2008 6:34 AM, Carl-Daniel Hailfinger
What about compression of the VSA?
On v3, we have the 'use compression' option in the Kconfig dialog. Long term, I intend to use the same compression for vsa that we use for everything else, and to decompress it as is done in v2, but use whatever compression the rest of coreboot is using.
For now, I'm going to run it from ROM. Yes, it will be slower.
No problem. Just be careful because most VSAs you can download from somewhere are either gzip or nrv2b compressed.
Regards, Carl-Daniel
On 24/01/08 17:15 +0100, Carl-Daniel Hailfinger wrote:
On 24.01.2008 17:02, ron minnich wrote:
On Jan 24, 2008 6:34 AM, Carl-Daniel Hailfinger
What about compression of the VSA?
On v3, we have the 'use compression' option in the Kconfig dialog. Long term, I intend to use the same compression for vsa that we use for everything else, and to decompress it as is done in v2, but use whatever compression the rest of coreboot is using.
For now, I'm going to run it from ROM. Yes, it will be slower.
No problem. Just be careful because most VSAs you can download from somewhere are either gzip or nrv2b compressed.
The code is nrv2b compressed and padded, and the whole thing is then gzipped and put on the website (don't ask me why we're gziping an already mostly compressed blob). Buildrom or wget usually does the right thing in that case.
Regards, Carl-Daniel
On Jan 22, 2008 5:33 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Besides the obvious stuff (storing the VSA as a file in the LAR) there are two points to solve which are rather interesting:
- Entry point specification
- Load address specification
Both are not trivial because we get both specifications above from an ELF header. The VSA is not an ELF file, so we have to specify entry point and load address by hand.
Hi Carl-Daniel, I appreciate the thought but I am pretty certain we don't need this. Let's not do this just now. I am about to put up a patch for the first part of VSA support anyway.
thanks
ron
-
Carl-Daniel Hailfinger -
Jordan Crouse -
ron minnich -
Stefan Reinauer