On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote:
On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote:
On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote:
Okay I'm pretty close to posting some patches that advance this project further, but wanted to check something beforehand: there are several tables that point to other tables (for example: FADT points to DSDT). What I did is provide a list of fixups such that bios can patch in pointers without any need to understand what's what. Thoughts?
Great!
For the RSDP, RSDT, and FADT I think SeaBIOS should just update those tables to set the pointers within them and then recalculate the checksum. I don't think anything complex is needed - it's easy for SeaBIOS to recognize those special tables and modify them.
-Kevin
True, that's simple enough. My worry is we can add more such tables. For example, we can decide to switch to XSDT in the future. If you look at the latest ACPI spec, there are tons of pointer tables not implemented in seabios. If we need to add support in seabios as well as qemu for each of them, this will kind of defeat the purpose.
And just to show how simple it can be, here's that patch in the series: basically ACPI tables will be in-memory, so all we need to do to link them together is keep a pointer from file to the table. Basically all linker needs is ability to add or subtract. What I like with this approach is host is in complete control of table layout. All BIOS does is allocate memory for them, it never changes the tables, including the checksum. Note that it's coded very defensively, which makes it a bit bigger than absolutely necessary, but hopefully more future proof.
---> linker: utility to patch in-memory ROM files
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.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
---
Makefile | 2 - src/linker.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 28 +++++++++++++++++++++ src/util.h | 1 4 files changed, 108 insertions(+), 1 deletion(-)
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..5009f88 --- /dev/null +++ b/src/linker.c @@ -0,0 +1,78 @@ +#include "linker.h" +#include "byteorder.h" // le64_to_cpu + +void linker_link(const char *name) +{ + struct linker_entry_s *entry; + int offset = 0; + int size, lsrc; + void *data = romfile_loadfile(name, &size); + struct romfile_s *src, *dst; + u32 src_offset, dst_offset; + u64 val, buf; + if (!data) + return; + + for (offset = 0; offset < size; offset += entry->size) { + entry = data + offset; + /* Entry must have some data. If not - skip it. */ + if (entry->size <= sizeof *entry) + continue; + /* Check that entry fits in buffer. */ + if (offset + entry->size > size) { + warn_internalerror(); + break; + } + /* Skip any command that we don't recognize. */ + if (entry->reserved) + continue; + if (entry->bytes != 1 && + entry->bytes != 2 && + entry->bytes != 4 && + entry->bytes != 8) + continue; + if (entry->type != LINKER_ENTRY_TYPE_ADD && + entry->type != LINKER_ENTRY_TYPE_SUB) + continue; + if (entry->format != LINKER_ENTRY_FORMAT_LE) + continue; + /* Last byte must be 0 */ + if (entry->src_dst[entry->size - 1]) { + warn_internalerror(); + continue; + } + lsrc = strlen(entry->src_dst); + if (!lsrc || lsrc + 1 + sizeof *entry >= entry->size) { + warn_internalerror(); + continue; + } + src = romfile_find(entry->src_dst); + dst = romfile_find(entry->src_dst + lsrc + 1); + if (!src || !dst) { + warn_internalerror(); + continue; + } + if (!src->data || !dst->data) { + warn_internalerror(); + continue; + } + src_offset = le32_to_cpu(entry->src_offset); + dst_offset = le32_to_cpu(entry->dst_offset); + if (dst->size < dst_offset + entry->bytes) { + warn_internalerror(); + continue; + } + buf = 0; + memcpy(&buf, dst->data + dst_offset, entry->bytes); + val = ((unsigned long)src->data) + src_offset; + buf = le64_to_cpu(buf); + if (entry->type == LINKER_ENTRY_TYPE_ADD) + buf += val; + else + buf -= val; + buf = cpu_to_le64(val); + memcpy(dst->data + dst_offset, &buf, entry->bytes); + } + + free(data); +} diff --git a/src/linker.h b/src/linker.h new file mode 100644 index 0000000..e1d55ea --- /dev/null +++ b/src/linker.h @@ -0,0 +1,28 @@ +#include "types.h" // u8 +#include "util.h" // romfile_s + +/* ROM file linker interface. Linker uses little endian format */ +struct linker_entry_s { + u8 size; /* size in bytes including the header */ + u8 bytes; /* How many bytes to change. Can be 1,2,4 or 8. */ + u8 type; + u8 format; + u32 reserved; + u64 src_offset; /* Offset in source. Little endian format. */ + u64 dst_offset; /* Offset in destination. Little endian format. */ + /* Followed by source and destination, optionally padded by + * 0, up to the total of entry_len - 4 bytes. + * Strings are 0-terminated. */ + char src_dst[]; +} PACKED; + +enum linker_entry_type { + LINKER_ENTRY_TYPE_ADD = 0x0, + LINKER_ENTRY_TYPE_SUB = 0x1, +}; + +enum linker_entry_format { + LINKER_ENTRY_FORMAT_LE = 0x0, +}; + +void linker_link(const char *name); 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);