On Sun, Sep 22, 2013 at 01:49:58PM +0300, Michael S. Tsirkin wrote: > On Thu, Jul 25, 2013 at 08:06:27PM -0400, Kevin O'Connor wrote: > > On Thu, Jul 25, 2013 at 03:55:56PM +0300, Michael S. Tsirkin wrote: > > > On Mon, Jul 15, 2013 at 11:01:02AM +0300, Michael S. Tsirkin wrote: > > > > On Sun, Jul 14, 2013 at 02:24:52PM -0400, Kevin O'Connor wrote: > > > > > I'd prefer to see this tracked within the "linker" code and not in the > > > > > generic romfile struct. > > > > > > > > A way to associate a romfile instance with a value seems generally > > > > useful, no? Still, that's not too hard - it would only mean an extra > > > > linked list of > > > > > > > > struct linker { > > > > char name[56] > > > > void *data; > > > > struct hlist_node node; > > > > } > > > > > > > > is this preferable? > > > > Sure, but it's probably easier to do something like: > > > > struct linkfiles { char *name; void *data; }; > > > > void linker_loader_execute(const char *name) > > { > > int size; > > struct linker_loader_entry_s *entries = romfile_loadfile(name, &size); > > int numentries = size/sizeof(entries[0]); > > if (! entries) > > return; > > struct linkfiles *files = malloc_tmp(sizeof(files[0]) * numentries); > > > > and then just populate and use the array of filenames. > > OK I'll do this but it's more code as I can't use plain romfile_find > anymore, and have to code up my own lookup. > > > > > > Also, is there another name besides "linker" that could be used? > > > > > SeaBIOS has code to self-relocate and fixup code relocations. I think > > > > > having code in the repo called "linker" could cause confusion. > > > > > > > > > > > > > romfile_loader? > > > > Shrug. How about "tabledeploy"? > > > > -Kevin
So I tried this out, and I don't like this much: see below. Lots of code, a single data pointer in file struct seems much easier. Further, without a pointer from file to data, there's no way to find the tables in memory, so I will have to make the loader interface ACPI-specific, make it look into tables loaded for the RSDP signature and return the address of RSDP. diff --git a/src/util.h b/src/util.h index 1e883f2..d777521 100644 --- a/src/util.h +++ b/src/util.h @@ -441,7 +441,6 @@ struct romfile_s { char name[128]; u32 size; int (*copy)(struct romfile_s *file, void *dest, u32 maxlen); - void *data; }; void romfile_add(struct romfile_s *file); struct romfile_s *romfile_findprefix(const char *prefix, struct romfile_s *prev); diff --git a/src/acpi.c b/src/acpi.c index 24cb1fa..c3a3c16 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -611,14 +611,14 @@ acpi_find_rsdp_rom(void) if (!file) break; - if (!file->data || !pmm_test_fseg(file->data) || + if (/*!file->data ||*/ !pmm_test_fseg(NULL /*file->data*/) || file->size < sizeof(rsdp->signature)) continue; void *data; - for (data = file->data; - data + sizeof(*rsdp) <= file->data + file->size; + for (data = NULL /*file->data */; + data + sizeof(*rsdp) <= /* file->data */ NULL + file->size; data++) { rsdp = data; if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE)) diff --git a/src/romfile_loader.c b/src/romfile_loader.c index 6ba03ed..5e98810 100644 --- a/src/romfile_loader.c +++ b/src/romfile_loader.c @@ -2,17 +2,33 @@ #include "byteorder.h" // leXX_to_cpu/cpu_to_leXX #include "util.h" // checksum -static struct romfile_s *romfile_loader_find(const char *name) +struct romfile_loader_file { + struct romfile_s *file; + void *data; +}; +struct romfile_loader_files { + int nfiles; + struct romfile_loader_file files[]; +}; + +static struct romfile_loader_file * +romfile_loader_find(const char *name, + struct romfile_loader_files *files) { + int i; if (name[ROMFILE_LOADER_FILESZ - 1]) return NULL; - return romfile_find(name); + for (i = 0; i < files->nfiles; ++i) + if (!strcmp(files->files[i].file->name, name)) + return &files->files[i]; + return NULL; } -static void romfile_loader_allocate(struct romfile_loader_entry_s *entry) +static void romfile_loader_allocate(struct romfile_loader_entry_s *entry, + struct romfile_loader_files *files) { struct zone_s *zone; - struct romfile_s *file; + struct romfile_loader_file *file = &files->files[files->nfiles]; void *data; int ret; unsigned alloc_align = le32_to_cpu(entry->alloc_align); @@ -32,20 +48,21 @@ static void romfile_loader_allocate(struct romfile_loader_entry_s *entry) } if (alloc_align < MALLOC_MIN_ALIGN) alloc_align = MALLOC_MIN_ALIGN; - file = romfile_loader_find(entry->alloc_file); - if (!file || file->data) + if (entry->alloc_file[ROMFILE_LOADER_FILESZ - 1]) goto err; - if (!file->size) + file->file = romfile_find(entry->alloc_file); + if (!file->file || !file->file->size) return; - data = pmm_malloc(zone, PMM_DEFAULT_HANDLE, file->size, alloc_align); + data = pmm_malloc(zone, PMM_DEFAULT_HANDLE, file->file->size, alloc_align); if (!data) { warn_noalloc(); return; } - ret = file->copy(file, data, file->size); - if (ret != file->size) + ret = file->file->copy(file->file, data, file->file->size); + if (ret != file->file->size) goto file_err; file->data = data; + files->nfiles++; return; file_err: @@ -54,16 +71,20 @@ err: warn_internalerror(); } -static void romfile_loader_add_pointer(struct romfile_loader_entry_s *entry) +static void romfile_loader_add_pointer(struct romfile_loader_entry_s *entry, + struct romfile_loader_files *files) { - struct romfile_s *dest_file = romfile_loader_find(entry->pointer_dest_file); - struct romfile_s *src_file = romfile_loader_find(entry->pointer_src_file); + struct romfile_loader_file *dest_file; + struct romfile_loader_file *src_file; unsigned offset = le32_to_cpu(entry->pointer_offset); u64 pointer = 0; + dest_file = romfile_loader_find(entry->pointer_dest_file, files); + src_file = romfile_loader_find(entry->pointer_src_file, files); + if (!dest_file || !src_file || !dest_file->data || !src_file->data || offset + entry->pointer_size < offset || - offset + entry->pointer_size > dest_file->size || + offset + entry->pointer_size > dest_file->file->size || entry->pointer_size < 1 || entry->pointer_size > 8 || entry->pointer_size & (entry->pointer_size - 1)) goto err; @@ -79,15 +100,19 @@ err: warn_internalerror(); } -static void romfile_loader_add_checksum(struct romfile_loader_entry_s *entry) +static void romfile_loader_add_checksum(struct romfile_loader_entry_s *entry, + struct romfile_loader_files *files) { - struct romfile_s *file = romfile_loader_find(entry->cksum_file); + struct romfile_loader_file *file; unsigned offset = le32_to_cpu(entry->cksum_offset); unsigned start = le32_to_cpu(entry->cksum_start); unsigned len = le32_to_cpu(entry->cksum_length); u8 *data; - if (!file || !file->data || offset >= file->size || - start + len < start || start + len > file->size) + + file = romfile_loader_find(entry->cksum_file, files); + + if (!file || !file->data || offset >= file->file->size || + start + len < start || start + len > file->file->size) goto err; data = file->data + offset; @@ -101,33 +126,47 @@ err: int romfile_loader_execute(const char *name) { struct romfile_loader_entry_s *entry; - int size, offset = 0; + int size, offset = 0, nfiles = 0; + struct romfile_loader_files *files; void *data = romfile_loadfile(name, &size); if (!data) return -1; + /* Validate and count files */ for (offset = 0; offset < size; offset += sizeof(*entry)) { entry = data + offset; /* Check that entry fits in buffer. */ if (offset + sizeof(*entry) > size) { warn_internalerror(); - break; + return -1; } + + if (le32_to_cpu(entry->command) == ROMFILE_LOADER_COMMAND_ALLOCATE) { + nfiles++; + } + } + + files = malloc_tmp(sizeof(*files) + nfiles * sizeof(files->files[0])); + files->nfiles = 0; + + for (offset = 0; offset < size; offset += sizeof(*entry)) { + entry = data + offset; switch (le32_to_cpu(entry->command)) { case ROMFILE_LOADER_COMMAND_ALLOCATE: - romfile_loader_allocate(entry); + romfile_loader_allocate(entry, files); break; case ROMFILE_LOADER_COMMAND_ADD_POINTER: - romfile_loader_add_pointer(entry); + romfile_loader_add_pointer(entry, files); break; case ROMFILE_LOADER_COMMAND_ADD_CHECKSUM: - romfile_loader_add_checksum(entry); + romfile_loader_add_checksum(entry, files); default: /* Skip commands that we don't recognize. */ break; } } + free(files); free(data); return 0; }