This is the seabios code that adds support for loading acpi tables from QEMU: it's working fine here and in fact, I am able to get exactly same tables with builtin tables and with tables coming from qemu.
Changes from v1: - simplified linker interfaces along the lines suggested by Kevin - fixed lots of bugs
Michael S. Tsirkin (5): linker: utility to patch in-memory ROM files pmm: add a way to test whether memory is in FSEG acpi: pack rsdp acpi: load and link tables from /etc/acpi/ acpi: add an option to disable builtin tables
Makefile | 2 +- src/Kconfig | 12 +++++- src/acpi.c | 39 +++++++++++++++++ src/acpi.h | 2 +- src/linker.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 72 +++++++++++++++++++++++++++++++ src/paravirt.c | 2 + src/pmm.c | 24 ++++++++--- src/util.h | 2 + 9 files changed, 278 insertions(+), 9 deletions(-) create mode 100644 src/linker.c create mode 100644 src/linker.h
Add ability for a ROM file to point to it's image in memory. When file is in memory, add utility that can patch it, storing pointers to one file within another file.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, we get: Before: Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) After: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- Makefile | 2 +- src/linker.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 72 +++++++++++++++++++++++++++++++ src/paravirt.c | 2 + src/util.h | 1 + 5 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 src/linker.c create mode 100644 src/linker.h
diff --git a/Makefile b/Makefile index 759bbbb..020fb2f 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ SRC16=$(SRCBOTH) system.c disk.c font.c SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c pmm.c coreboot.c boot.c \ acpi.c smm.c mptable.c pirtable.c smbios.c pciinit.c optionroms.c mtrr.c \ lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c \ - biostables.c xen.c bmp.c romfile.c csm.c + biostables.c xen.c bmp.c romfile.c csm.c linker.c SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
# Default compiler flags diff --git a/src/linker.c b/src/linker.c new file mode 100644 index 0000000..3caa97b --- /dev/null +++ b/src/linker.c @@ -0,0 +1,132 @@ +#include "linker.h" +#include "byteorder.h" // leXX_to_cpu/cpu_to_leXX +#include "util.h" // checksum + +static struct romfile_s *linker_loader_find(const char *name) +{ + if (name[LINKER_LOADER_FILESZ - 1]) + return NULL; + return romfile_find(name); +} + +static void linker_loader_allocate(struct linker_loader_entry_s *entry) +{ + struct zone_s *zone; + struct romfile_s *file; + void *data; + int ret; + unsigned alloc_align = le32_to_cpu(entry->alloc_align); + + if (alloc_align & (alloc_align - 1)) + goto err; + + switch (entry->alloc_zone) { + case LINKER_LOADER_ALLOC_ZONE_HIGH: + zone = &ZoneHigh; + break; + case LINKER_LOADER_ALLOC_ZONE_FSEG: + zone = &ZoneFSeg; + break; + default: + goto err; + } + if (alloc_align < MALLOC_MIN_ALIGN) + alloc_align = MALLOC_MIN_ALIGN; + file = linker_loader_find(entry->alloc_file); + if (!file || file->data) + goto err; + if (!file->size) + return; + data = pmm_malloc(zone, PMM_DEFAULT_HANDLE, file->size, alloc_align); + if (!data) { + warn_noalloc(); + return; + } + ret = file->copy(file, data, file->size); + if (ret != file->size) + goto file_err; + file->data = data; + return; + +file_err: + free(data); +err: + warn_internalerror(); +} + +static void linker_loader_add_pointer(struct linker_loader_entry_s *entry) +{ + struct romfile_s *dest_file = linker_loader_find(entry->pointer_dest_file); + struct romfile_s *src_file = linker_loader_find(entry->pointer_src_file); + unsigned offset = le32_to_cpu(entry->pointer_offset); + u64 pointer = 0; + + if (!dest_file || !src_file || !dest_file->data || !src_file->data || + offset + entry->pointer_size < offset || + offset + entry->pointer_size > dest_file->size || + entry->pointer_size < 1 || entry->pointer_size > 8 || + entry->pointer_size & (entry->pointer_size - 1)) + goto err; + + memcpy(&pointer, dest_file->data + offset, entry->pointer_size); + pointer = le64_to_cpu(pointer); + pointer += (unsigned long)src_file->data; + pointer = cpu_to_le64(pointer); + memcpy(dest_file->data + offset, &pointer, entry->pointer_size); + + return; +err: + warn_internalerror(); +} + +static void linker_loader_add_checksum(struct linker_loader_entry_s *entry) +{ + struct romfile_s *file = linker_loader_find(entry->cksum_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) + goto err; + + data = file->data + offset; + *data -= checksum(file->data + start, len); + + return; +err: + warn_internalerror(); +} + +void linker_loader_execute(const char *name) +{ + struct linker_loader_entry_s *entry; + int size, offset = 0; + void *data = romfile_loadfile(name, &size); + if (!data) + return; + + 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; + } + switch (le32_to_cpu(entry->command)) { + case LINKER_LOADER_COMMAND_ALLOCATE: + linker_loader_allocate(entry); + break; + case LINKER_LOADER_COMMAND_ADD_POINTER: + linker_loader_add_pointer(entry); + break; + case LINKER_LOADER_COMMAND_ADD_CHECKSUM: + linker_loader_add_checksum(entry); + default: + /* Skip commands that we don't recognize. */ + break; + } + } + + free(data); +} diff --git a/src/linker.h b/src/linker.h new file mode 100644 index 0000000..0c374e5 --- /dev/null +++ b/src/linker.h @@ -0,0 +1,72 @@ +#ifndef __LINKER_H +#define __LINKER_H + +#include "types.h" // u8 +#include "util.h" // romfile_s + +#define LINKER_LOADER_FILESZ 56 + +/* ROM file linker/loader interface. Linker uses little endian format */ +struct linker_loader_entry_s { + u32 command; + union { + /* + * COMMAND_ALLOCATE - allocate a table from @alloc_file + * subject to @alloc_align alignment (must be power of 2) + * and @alloc_zone (can be HIGH or FSEG) requirements. + * + * Must appear exactly once for each file, and before + * this file is referenced by any other command. + */ + struct { + char alloc_file[LINKER_LOADER_FILESZ]; + u32 alloc_align; + u8 alloc_zone; + }; + + /* + * COMMAND_ADD_POINTER - patch the table (originating from + * @dest_file) at @pointer_offset, by adding a pointer to the table + * originating from @src_file. 1,2,4 or 8 byte unsigned + * addition is used depending on @pointer_size. + */ + struct { + char pointer_dest_file[LINKER_LOADER_FILESZ]; + char pointer_src_file[LINKER_LOADER_FILESZ]; + u32 pointer_offset; + u8 pointer_size; + }; + + /* + * COMMAND_ADD_CHECKSUM - calculate checksum of the range specified by + * @cksum_start and @cksum_length fields, + * and then add the value at @cksum_offset. + * Checksum simply sums -X for each byte X in the range + * using 8-bit math. + */ + struct { + char cksum_file[LINKER_LOADER_FILESZ]; + u32 cksum_offset; + u32 cksum_start; + u32 cksum_length; + }; + + /* padding */ + char pad[124]; + }; +}; + +enum { + LINKER_LOADER_COMMAND_ALLOCATE = 0x1, + LINKER_LOADER_COMMAND_ADD_POINTER = 0x2, + LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3, +}; + +enum { + LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1, + LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2, +}; + +void linker_loader_execute(const char *name); + +#endif diff --git a/src/paravirt.c b/src/paravirt.c index d1a5d3e..9dd229d 100644 --- a/src/paravirt.c +++ b/src/paravirt.c @@ -327,6 +327,8 @@ void qemu_cfg_init(void) for (e = 0; e < count; e++) { struct QemuCfgFile qfile; qemu_cfg_read(&qfile, sizeof(qfile)); + if (!*qfile.name) + return; qemu_romfile_add(qfile.name, be16_to_cpu(qfile.select) , 0, be32_to_cpu(qfile.size)); } diff --git a/src/util.h b/src/util.h index 996c29a..7b50c38 100644 --- a/src/util.h +++ b/src/util.h @@ -436,6 +436,7 @@ 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);
On Sun, Jul 07, 2013 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
Add ability for a ROM file to point to it's image in memory. When file is in memory, add utility that can patch it, storing pointers to one file within another file.
Thanks. See my comments below.
[...]
--- /dev/null +++ b/src/linker.c
[...]
+void linker_loader_execute(const char *name) +{
- struct linker_loader_entry_s *entry;
- int size, offset = 0;
- void *data = romfile_loadfile(name, &size);
- if (!data)
return;- for (offset = 0; offset < size; offset += sizeof *entry) {
For consistent style, please treat sizeof like a function (ie, sizeof(*entry) ).
entry = data + offset;/* Check that entry fits in buffer. */if (offset + sizeof *entry > size) {warn_internalerror();break;}- switch (le32_to_cpu(entry->command)) {
case LINKER_LOADER_COMMAND_ALLOCATE:linker_loader_allocate(entry);
SeaBIOS uses 4 spaces for indentation, and no tabs.
[...]
--- a/src/util.h +++ b/src/util.h @@ -436,6 +436,7 @@ struct romfile_s { char name[128]; u32 size; int (*copy)(struct romfile_s *file, void *dest, u32 maxlen);
- void *data;
};
I'd prefer to see this tracked within the "linker" code and not in the generic romfile struct.
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.
-Kevin
On Sun, Jul 14, 2013 at 02:24:52PM -0400, Kevin O'Connor wrote:
On Sun, Jul 07, 2013 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
Add ability for a ROM file to point to it's image in memory. When file is in memory, add utility that can patch it, storing pointers to one file within another file.
Thanks. See my comments below.
[...]
--- /dev/null +++ b/src/linker.c
[...]
+void linker_loader_execute(const char *name) +{
- struct linker_loader_entry_s *entry;
- int size, offset = 0;
- void *data = romfile_loadfile(name, &size);
- if (!data)
return;- for (offset = 0; offset < size; offset += sizeof *entry) {
For consistent style, please treat sizeof like a function (ie, sizeof(*entry) ).
entry = data + offset;/* Check that entry fits in buffer. */if (offset + sizeof *entry > size) {warn_internalerror();break;}- switch (le32_to_cpu(entry->command)) {
case LINKER_LOADER_COMMAND_ALLOCATE:linker_loader_allocate(entry);SeaBIOS uses 4 spaces for indentation, and no tabs.
[...]
--- a/src/util.h +++ b/src/util.h @@ -436,6 +436,7 @@ struct romfile_s { char name[128]; u32 size; int (*copy)(struct romfile_s *file, void *dest, u32 maxlen);
- void *data;
};
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?
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.
-Kevin
romfile_loader?
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:
On Sun, Jul 07, 2013 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
Add ability for a ROM file to point to it's image in memory. When file is in memory, add utility that can patch it, storing pointers to one file within another file.
Thanks. See my comments below.
[...]
--- /dev/null +++ b/src/linker.c
[...]
+void linker_loader_execute(const char *name) +{
- struct linker_loader_entry_s *entry;
- int size, offset = 0;
- void *data = romfile_loadfile(name, &size);
- if (!data)
return;- for (offset = 0; offset < size; offset += sizeof *entry) {
For consistent style, please treat sizeof like a function (ie, sizeof(*entry) ).
entry = data + offset;/* Check that entry fits in buffer. */if (offset + sizeof *entry > size) {warn_internalerror();break;}- switch (le32_to_cpu(entry->command)) {
case LINKER_LOADER_COMMAND_ALLOCATE:linker_loader_allocate(entry);SeaBIOS uses 4 spaces for indentation, and no tabs.
[...]
--- a/src/util.h +++ b/src/util.h @@ -436,6 +436,7 @@ struct romfile_s { char name[128]; u32 size; int (*copy)(struct romfile_s *file, void *dest, u32 maxlen);
- void *data;
};
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?
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.
-Kevin
romfile_loader?
Could you respond on above please?
-- MST
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.
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
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
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; }
On Sun, Sep 22, 2013 at 02:18:45PM +0300, Michael S. Tsirkin wrote:
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
Latest version that I posted uses the approach suggested by Kevin here. It adds about 200 bytes to code size. If we want to cut that out and go back to data pointer, we can do this as a patch on top.
Pls let me know.
Will be handy for looking for RSDP.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/pmm.c | 24 ++++++++++++++++++------ src/util.h | 1 + 2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/pmm.c b/src/pmm.c index 8f993fd..8bd8983 100644 --- a/src/pmm.c +++ b/src/pmm.c @@ -120,15 +120,23 @@ addSpace(struct zone_s *zone, void *start, void *end)
// Search all zones for an allocation obtained from allocSpace() static struct allocinfo_s * +findAllocInZone(struct zone_s *zone, void *data) +{ + struct allocinfo_s *info; + hlist_for_each_entry(info, &zone->head, node) + if (info->data == data) + return info; + return NULL; +} + +static struct allocinfo_s * findAlloc(void *data) { int i; - for (i=0; i<ARRAY_SIZE(Zones); i++) { - struct allocinfo_s *info; - hlist_for_each_entry(info, &Zones[i]->head, node) { - if (info->data == data) - return info; - } + for (i = 0; i < ARRAY_SIZE(Zones); i++) { + struct allocinfo_s *info = findAllocInZone(Zones[i], data); + if (info) + return info; } return NULL; } @@ -241,6 +249,10 @@ pmm_find(u32 handle) return NULL; }
+int pmm_test_fseg(void *data) +{ + return !!findAllocInZone(&ZoneFSeg, data); +}
/**************************************************************** * 0xc0000-0xf0000 management diff --git a/src/util.h b/src/util.h index 7b50c38..44e1c1a 100644 --- a/src/util.h +++ b/src/util.h @@ -378,6 +378,7 @@ void malloc_init(void); void malloc_prepboot(void); void *pmm_malloc(struct zone_s *zone, u32 handle, u32 size, u32 align); int pmm_free(void *data); +int pmm_test_fseg(void *data); void pmm_init(void); void pmm_prepboot(void); #define PMM_DEFAULT_HANDLE 0xFFFFFFFF
rsdp might not be aligned, so mark it packed.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/acpi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/acpi.h b/src/acpi.h index f872b39..eab982a 100644 --- a/src/acpi.h +++ b/src/acpi.h @@ -47,7 +47,7 @@ struct rsdp_descriptor { /* Root System Descriptor Pointer */ u64 xsdt_physical_address; /* 64-bit physical address of XSDT */ u8 extended_checksum; /* Checksum of entire table */ u8 reserved [3]; /* Reserved field must be 0 */ -}; +} PACKED;
extern struct rsdp_descriptor *RsdpAddr;
Load files in /etc/acpi/, link them using a linker script and use for acpi tables, including the RSDP. Presense of RSDP in this directory completely disables generating and loading legacy acpi tables.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/acpi.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/acpi.c b/src/acpi.c index a81f0cb..07d311a 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -27,6 +27,7 @@ #include "config.h" // CONFIG_* #include "paravirt.h" // RamSize #include "dev-q35.h" +#include "linker.h"
#include "acpi-dsdt.hex"
@@ -599,6 +600,34 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in files loaded in FSEG */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{ + struct romfile_s *file = NULL; + struct rsdp_descriptor *rsdp = NULL; + for (;;) { + file = romfile_findprefix("", file); + if (!file) + break; + + if (!file->data || !pmm_test_fseg(file->data) || + file->size < sizeof(rsdp->signature)) + continue; + + void *data; + + for (data = file->data; + data + sizeof(*rsdp) <= file->data + file->size; + data++) { + rsdp = data; + if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE)) + return rsdp; + } + } + return NULL; +} + #define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -608,6 +637,16 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
+ linker_loader_execute("/etc/linker-script"); + + RsdpAddr = acpi_find_rsdp_rom(); + + if (RsdpAddr) { + return; + } + + dprintf(3, "generate ACPI tables\n"); + // This code is hardcoded for PIIX4 Power Management device. struct pci_device *pci = pci_find_init_device(acpi_find_tbl, NULL); if (!pci)
On Sun, Jul 07, 2013 at 06:42:43PM +0300, Michael S. Tsirkin wrote:
Load files in /etc/acpi/, link them using a linker script and use for acpi tables, including the RSDP. Presense of RSDP in this directory completely disables generating and loading legacy acpi tables.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/acpi.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/acpi.c b/src/acpi.c index a81f0cb..07d311a 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -27,6 +27,7 @@ #include "config.h" // CONFIG_* #include "paravirt.h" // RamSize #include "dev-q35.h" +#include "linker.h"
#include "acpi-dsdt.hex"
@@ -599,6 +600,34 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in files loaded in FSEG */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- struct romfile_s *file = NULL;
- struct rsdp_descriptor *rsdp = NULL;
- for (;;) {
file = romfile_findprefix("", file);if (!file)break;if (!file->data || !pmm_test_fseg(file->data) ||file->size < sizeof(rsdp->signature))continue;void *data;for (data = file->data;data + sizeof(*rsdp) <= file->data + file->size;data++) {rsdp = data;if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE))return rsdp;}- }
- return NULL;
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -608,6 +637,16 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- linker_loader_execute("/etc/linker-script");
- RsdpAddr = acpi_find_rsdp_rom();
- if (RsdpAddr) {
return;- }
I don't understand this. Why not use the presence of "/etc/linker-script" to determine if ACPI should be produced.
Also, /etc/linker-script is not very informative - how about something like "/etc/biostables".
-Kevin
On Sun, Jul 14, 2013 at 02:27:14PM -0400, Kevin O'Connor wrote:
On Sun, Jul 07, 2013 at 06:42:43PM +0300, Michael S. Tsirkin wrote:
Load files in /etc/acpi/, link them using a linker script and use for acpi tables, including the RSDP. Presense of RSDP in this directory completely disables generating and loading legacy acpi tables.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/acpi.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/acpi.c b/src/acpi.c index a81f0cb..07d311a 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -27,6 +27,7 @@ #include "config.h" // CONFIG_* #include "paravirt.h" // RamSize #include "dev-q35.h" +#include "linker.h"
#include "acpi-dsdt.hex"
@@ -599,6 +600,34 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in files loaded in FSEG */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- struct romfile_s *file = NULL;
- struct rsdp_descriptor *rsdp = NULL;
- for (;;) {
file = romfile_findprefix("", file);if (!file)break;if (!file->data || !pmm_test_fseg(file->data) ||file->size < sizeof(rsdp->signature))continue;void *data;for (data = file->data;data + sizeof(*rsdp) <= file->data + file->size;data++) {rsdp = data;if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE))return rsdp;}- }
- return NULL;
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -608,6 +637,16 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- linker_loader_execute("/etc/linker-script");
- RsdpAddr = acpi_find_rsdp_rom();
- if (RsdpAddr) {
return;- }
I don't understand this. Why not use the presence of "/etc/linker-script" to determine if ACPI should be produced.
We can do this but what do we do if linker script is there but no rsdp? Maybe just add test -e /etc/linker-script != RsdpAddr dprintf(1, "linker script found but no rsdp. Falling back on builtin tables\n");
Would this address your comment?
Also, /etc/linker-script is not very informative - how about something like "/etc/biostables".
-Kevin
Hmm bios seems redundant, and tables makes one think it's the tables themselves.
"/etc/acpi-linker" "/etc/acpi-loader"?
On Mon, Jul 15, 2013 at 11:11:59AM +0300, Michael S. Tsirkin wrote:
On Sun, Jul 14, 2013 at 02:27:14PM -0400, Kevin O'Connor wrote:
On Sun, Jul 07, 2013 at 06:42:43PM +0300, Michael S. Tsirkin wrote:
Load files in /etc/acpi/, link them using a linker script and use for acpi tables, including the RSDP. Presense of RSDP in this directory completely disables generating and loading legacy acpi tables.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/acpi.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/acpi.c b/src/acpi.c index a81f0cb..07d311a 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -27,6 +27,7 @@ #include "config.h" // CONFIG_* #include "paravirt.h" // RamSize #include "dev-q35.h" +#include "linker.h"
#include "acpi-dsdt.hex"
@@ -599,6 +600,34 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in files loaded in FSEG */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- struct romfile_s *file = NULL;
- struct rsdp_descriptor *rsdp = NULL;
- for (;;) {
file = romfile_findprefix("", file);if (!file)break;if (!file->data || !pmm_test_fseg(file->data) ||file->size < sizeof(rsdp->signature))continue;void *data;for (data = file->data;data + sizeof(*rsdp) <= file->data + file->size;data++) {rsdp = data;if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE))return rsdp;}- }
- return NULL;
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -608,6 +637,16 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- linker_loader_execute("/etc/linker-script");
- RsdpAddr = acpi_find_rsdp_rom();
- if (RsdpAddr) {
return;- }
I don't understand this. Why not use the presence of "/etc/linker-script" to determine if ACPI should be produced.
We can do this but what do we do if linker script is there but no rsdp? Maybe just add test -e /etc/linker-script != RsdpAddr dprintf(1, "linker script found but no rsdp. Falling back on builtin tables\n");
Would this address your comment?
And just to clarify, in my eyes it's preferable to use acpi_find_rsdp_rom which is based on ACPI standard than look for a specific PV interface.
For example, one can imagine a hypervisor that maps ACPI tables directly into guest memory. Looking for rsdp in fseg memory will work, relying on the linker won't.
Also, /etc/linker-script is not very informative - how about something like "/etc/biostables".
-Kevin
Hmm bios seems redundant, and tables makes one think it's the tables themselves.
"/etc/acpi-linker" "/etc/acpi-loader"?
-- MST
Serves to save a bit of memory, and is helpful for debugging (making sure tables come from qemu).
Memory stats: Enabled: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) Disabled: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/Kconfig | 12 +++++++++++- src/acpi.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig index 5882d11..ff4c9d1 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -387,10 +387,20 @@ menu "BIOS Tables" default y help Support generation of ACPI tables. + config ACPI_BUILTIN + bool "Include built-in ACPI tables" + default y + depends on ACPI + help + Include built-in ACPI tables in BIOS. + Required for QEMU 1.5 and older. + This option can be disabled for QEMU 1.6 and newer + to save some space in the ROM file. + If unsure, say Y. config ACPI_DSDT bool "Include default ACPI DSDT" default y - depends on ACPI + depends on ACPI && ACPI_BUILTIN help Include default DSDT ACPI table in BIOS. Required for QEMU 1.3 and older. diff --git a/src/acpi.c b/src/acpi.c index 07d311a..2c51fd1 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -641,7 +641,7 @@ acpi_setup(void)
RsdpAddr = acpi_find_rsdp_rom();
- if (RsdpAddr) { + if (!CONFIG_ACPI_BUILTIN || RsdpAddr) { return; }
-
Kevin O'Connor -
Michael S. Tsirkin