This is the seabios code that adds support for loading acpi tables from QEMU.
Changes from v3: - updated to latest bits - add option to disable loading from QEMU
Changes from v2: - addressed comments from Kevin: fixed coding style, minimized changes to existing code
Changes from v1: - simplified linker interfaces along the lines suggested by Kevin - fixed lots of bugs
Michael S. Tsirkin (5): romfile_loader: utility to patch in-memory ROM files pmm: support looking up a given pattern in FSEG acpi: pack rsdp acpi: load and link tables through romfile loader acpi: add an option to disable builtin tables
Makefile | 2 +- src/malloc.h | 1 + src/romfile_loader.h | 72 +++++++++++++++++++++ src/std/acpi.h | 2 +- src/fw/acpi.c | 30 +++++++++ src/fw/paravirt.c | 2 + src/malloc.c | 20 +++++- src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/Kconfig | 22 ++++++- 9 files changed, 323 insertions(+), 4 deletions(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
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, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \ - fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c + fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c SRC32SEG=string.c output.c pcibios.c apm.c stacks.c hw/pci.c DIRS=src src/hw src/fw vgasrc
diff --git a/src/romfile_loader.h b/src/romfile_loader.h new file mode 100644 index 0000000..15eab2a --- /dev/null +++ b/src/romfile_loader.h @@ -0,0 +1,72 @@ +#ifndef __ROMFILE_LOADER_H +#define __ROMFILE_LOADER_H + +#include "types.h" // u8 +#include "util.h" // romfile_s + +#define ROMFILE_LOADER_FILESZ 56 + +/* ROM file linker/loader interface. Linker uses little endian format */ +struct romfile_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[ROMFILE_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[ROMFILE_LOADER_FILESZ]; + char pointer_src_file[ROMFILE_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[ROMFILE_LOADER_FILESZ]; + u32 cksum_offset; + u32 cksum_start; + u32 cksum_length; + }; + + /* padding */ + char pad[124]; + }; +}; + +enum { + ROMFILE_LOADER_COMMAND_ALLOCATE = 0x1, + ROMFILE_LOADER_COMMAND_ADD_POINTER = 0x2, + ROMFILE_LOADER_COMMAND_ADD_CHECKSUM = 0x3, +}; + +enum { + ROMFILE_LOADER_ALLOC_ZONE_HIGH = 0x1, + ROMFILE_LOADER_ALLOC_ZONE_FSEG = 0x2, +}; + +int romfile_loader_execute(const char *name); + +#endif diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 357de56..84ff525 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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/romfile_loader.c b/src/romfile_loader.c new file mode 100644 index 0000000..b7cdc67 --- /dev/null +++ b/src/romfile_loader.c @@ -0,0 +1,176 @@ +#include "romfile_loader.h" +#include "byteorder.h" // leXX_to_cpu/cpu_to_leXX +#include "util.h" // checksum +#include "string.h" // strcmp +#include "romfile.h" // struct romfile_s +#include "malloc.h" // Zone*, _malloc +#include "output.h" // warn_* + +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; + 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, + struct romfile_loader_files *files) +{ + struct zone_s *zone; + struct romfile_loader_file *file = &files->files[files->nfiles]; + 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 ROMFILE_LOADER_ALLOC_ZONE_HIGH: + zone = &ZoneHigh; + break; + case ROMFILE_LOADER_ALLOC_ZONE_FSEG: + zone = &ZoneFSeg; + break; + default: + goto err; + } + if (alloc_align < MALLOC_MIN_ALIGN) + alloc_align = MALLOC_MIN_ALIGN; + if (entry->alloc_file[ROMFILE_LOADER_FILESZ - 1]) + goto err; + file->file = romfile_find(entry->alloc_file); + if (!file->file || !file->file->size) + return; + data = _malloc(zone, MALLOC_DEFAULT_HANDLE, file->file->size, alloc_align); + if (!data) { + warn_noalloc(); + return; + } + 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: + free(data); +err: + warn_internalerror(); +} + +static void romfile_loader_add_pointer(struct romfile_loader_entry_s *entry, + struct romfile_loader_files *files) +{ + 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->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 romfile_loader_add_checksum(struct romfile_loader_entry_s *entry, + struct romfile_loader_files *files) +{ + 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; + + 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; + *data -= checksum(file->data + start, len); + + return; +err: + warn_internalerror(); +} + +int romfile_loader_execute(const char *name) +{ + struct romfile_loader_entry_s *entry; + 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(); + 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, files); + break; + case ROMFILE_LOADER_COMMAND_ADD_POINTER: + romfile_loader_add_pointer(entry, files); + break; + case ROMFILE_LOADER_COMMAND_ADD_CHECKSUM: + romfile_loader_add_checksum(entry, files); + default: + /* Skip commands that we don't recognize. */ + break; + } + } + + free(files); + free(data); + return 0; +}
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;
What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files)); Also, you should check that that the malloc succeeds.
Otherwise, it looks okay to me.
-Kevin
On Tue, Sep 24, 2013 at 07:32:10PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files)); Also, you should check that that the malloc succeeds.
Otherwise, it looks okay to me.
-Kevin
Will fix, thanks a lot for the review.
On Tue, Sep 24, 2013 at 07:32:10PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files));
Wait, the reason I didn't do this is because it wastes memory: it includes chunksum and pointer entries, we really only want alloc entries. See?
Also, you should check that that the malloc succeeds.
Otherwise, it looks okay to me.
-Kevin
On Wed, Sep 25, 2013 at 02:48:21AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:32:10PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files));
Wait, the reason I didn't do this is because it wastes memory: it includes chunksum and pointer entries, we really only want alloc entries. See?
In all real-world situations we'll have megabytes of ram for "tmp" allocations. It's not a problem to over allocate here.
-Kevin
On Tue, Sep 24, 2013 at 07:55:52PM -0400, Kevin O'Connor wrote:
On Wed, Sep 25, 2013 at 02:48:21AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:32:10PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files));
Wait, the reason I didn't do this is because it wastes memory: it includes chunksum and pointer entries, we really only want alloc entries. See?
In all real-world situations we'll have megabytes of ram for "tmp" allocations. It's not a problem to over allocate here.
-Kevin
Right. But the code doesn't get simpler in fact - unless you think it's important to drop the loop counting nfiles?
On Wed, Sep 25, 2013 at 03:11:20AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:55:52PM -0400, Kevin O'Connor wrote:
In all real-world situations we'll have megabytes of ram for "tmp" allocations. It's not a problem to over allocate here.
Right. But the code doesn't get simpler in fact - unless you think it's important to drop the loop counting nfiles?
I was pointing out the loop, not the malloc call. As before, it's a nitpick.
-Kevin
On Tue, Sep 24, 2013 at 08:16:47PM -0400, Kevin O'Connor wrote:
On Wed, Sep 25, 2013 at 03:11:20AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:55:52PM -0400, Kevin O'Connor wrote:
In all real-world situations we'll have megabytes of ram for "tmp" allocations. It's not a problem to over allocate here.
Right. But the code doesn't get simpler in fact - unless you think it's important to drop the loop counting nfiles?
I was pointing out the loop, not the malloc call. As before, it's a nitpick.
-Kevin
Ah, I see what you mean then. something like this on top will do the trick. Yes, it's simpler.
diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c index c03fe48..2196a84 100644 --- a/src/fw/romfile_loader.c +++ b/src/fw/romfile_loader.c @@ -130,30 +130,23 @@ err: int romfile_loader_execute(const char *name) { struct romfile_loader_entry_s *entry; - int size, offset = 0, nfiles = 0; + int size, offset = 0, nfiles; 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(); - return -1; - } - - if (le32_to_cpu(entry->command) == ROMFILE_LOADER_COMMAND_ALLOCATE) { - nfiles++; - } + if (size % sizeof(*entry)) { + warn_invalid(); + goto err; }
+ /* (over)estimate the number of files to load. */ + nfiles = size / sizeof(*entry); files = malloc_tmp(sizeof(*files) + nfiles * sizeof(files->files[0])); if (!files) { warn_noalloc(); - return -1; + goto err; } files->nfiles = 0;
@@ -177,4 +170,8 @@ int romfile_loader_execute(const char *name) free(files); free(data); return 0; + +err: + free(data); + return -1; }
On Wed, Sep 25, 2013 at 02:48:21AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:32:10PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:25PM +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.
This is not a lot of code: together with the follow-up patch to load ACPI tables from ROM, it's about 1K extra.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Makefile | 2 +- src/romfile_loader.h | 72 +++++++++++++++++++++ src/fw/paravirt.c | 2 + src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/romfile_loader.h create mode 100644 src/romfile_loader.c
diff --git a/Makefile b/Makefile index 3984d35..ae36dba 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \ hw/usb-hub.c \ fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \ fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
- fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c
romfile_loader is a firmware file and show go in the fw/ directory.
[...]
--- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -365,6 +365,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;What is this for?
SeaBIOS uses spaces and no tabs.
[...]
+int romfile_loader_execute(const char *name) +{
- struct romfile_loader_entry_s *entry;
- 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();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;
Nitpick, but this would be much simpler if it was just: files = malloc_tmp(size / sizeof(*entry) * sizeof(*files));
Wait, the reason I didn't do this is because it wastes memory: it includes chunksum and pointer entries, we really only want alloc entries. See?
Also, the math is wrong here. What we want is header (sizeof(*files)) + size of file array element multipled by number of files.
So we can replace nfiles with size / sizeof(*entry) but that's not much simpler, is it?
Also, you should check that that the malloc succeeds.
Otherwise, it looks okay to me.
-Kevin
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/malloc.h | 1 + src/malloc.c | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/malloc.h b/src/malloc.h index af8a21d..feb8938 100644 --- a/src/malloc.h +++ b/src/malloc.h @@ -19,6 +19,7 @@ void *_malloc(struct zone_s *zone, u32 handle, u32 size, u32 align); int _free(void *data); u32 malloc_getspace(struct zone_s *zone); void *malloc_find(u32 handle); +void *malloc_find_fseg_pattern(void *pattern, unsigned pattern_size);
#define MALLOC_DEFAULT_HANDLE 0xFFFFFFFF // Minimum alignment of malloc'd memory diff --git a/src/malloc.c b/src/malloc.c index 281f41e..d150c90 100644 --- a/src/malloc.c +++ b/src/malloc.c @@ -12,7 +12,7 @@ #include "output.h" // dprintf #include "stacks.h" // wait_preempt #include "std/optionrom.h" // OPTION_ROM_ALIGN -#include "string.h" // memset +#include "string.h" // memset, memcmp
// Information on a reserved area. struct allocinfo_s { @@ -273,6 +273,24 @@ _free(void *data) return 0; }
+// Find the data block in zone matching a given pattern. +void *malloc_find_fseg_pattern(void *pattern, unsigned pattern_size) +{ + struct allocinfo_s *info; + hlist_for_each_entry(info, &ZoneFSeg.head, node) { + unsigned space = info->dataend - info->data; + int off; + + if (space < pattern_size) + continue; + for (off = 0; off < space - pattern_size; ++off) { + if (!memcmp(info->data + off, pattern, pattern_size)) + return info->data + off; + } + } + return NULL; +} + // Find the amount of free space in a given zone. u32 malloc_getspace(struct zone_s *zone)
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000. Don't modify pmm.c. Also, take a look at fw/biostables.c - as that code is already there to do this type of scanning.
-Kevin
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000.
Is this range guaranteed to be present?
Don't modify pmm.c. Also, take a look at fw/biostables.c - as that code is already there to do this type of scanning.
-Kevin
On Wed, Sep 25, 2013 at 03:15:40AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000.
Is this range guaranteed to be present?
0xf0000-0x100000 is the f-segment - it is where the bios lives. Yes, it will be there.
-Kevin
On Tue, Sep 24, 2013 at 08:15:40PM -0400, Kevin O'Connor wrote:
On Wed, Sep 25, 2013 at 03:15:40AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000.
Is this range guaranteed to be present?
0xf0000-0x100000 is the f-segment - it is where the bios lives. Yes, it will be there.
-Kevin
So will the following do? I didn't find the function you asked me to look up, OTOH malloc.c already knows about fseg start and end ...
diff --git a/src/malloc.c b/src/malloc.c index d150c90..0f5fae7 100644 --- a/src/malloc.c +++ b/src/malloc.c @@ -276,17 +276,16 @@ _free(void *data) +// Find the data block in zone matching a given pattern. +void *malloc_find_fseg_pattern(void *pattern, unsigned pattern_size) +{ + extern u8 zonefseg_start[], zonefseg_end[]; + unsigned space = zonefseg_end - zonefseg_start; + int off; + + if (space < pattern_size) + return NULL; + + for (off = 0; off < space - pattern_size; ++off) { + if (!memcmp(zonefseg_start + off, pattern, pattern_size)) + return zonefseg_start + off; + } + return NULL; +}
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000. Don't modify pmm.c. Also, take a look at fw/biostables.c - as that code is already there to do this type of scanning.
-Kevin
Hmm which function do you mean specifically? I don't see anything resembling that there ...
On Wed, Sep 25, 2013 at 03:26:44AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000. Don't modify pmm.c. Also, take a look at fw/biostables.c - as that code is already there to do this type of scanning.
-Kevin
Hmm which function do you mean specifically? I don't see anything resembling that there ...
Hrmm - that got moved around. The example I was thinking of is now in coreboot.c:scan_tables(). The goal is not to copy the table (as coreboot does), but to point out that there is already code to do scanning.
-Kevin
On Tue, Sep 24, 2013 at 08:30:13PM -0400, Kevin O'Connor wrote:
On Wed, Sep 25, 2013 at 03:26:44AM +0300, Michael S. Tsirkin wrote:
On Tue, Sep 24, 2013 at 07:35:09PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:28PM +0300, Michael S. Tsirkin wrote:
Will be used to find RSDP there.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Just scan from 0xf0000-0x100000. Don't modify pmm.c. Also, take a look at fw/biostables.c - as that code is already there to do this type of scanning.
-Kevin
Hmm which function do you mean specifically? I don't see anything resembling that there ...
Hrmm - that got moved around. The example I was thinking of is now in coreboot.c:scan_tables(). The goal is not to copy the table (as coreboot does), but to point out that there is already code to do scanning.
-Kevin
Hmm I'm not sure how to reuse anything from there ... does the patch I sent look reasonable?
rsdp might not be aligned, so mark it packed.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/std/acpi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/std/acpi.h b/src/std/acpi.h index ecfa472..6ea5b9d 100644 --- a/src/std/acpi.h +++ b/src/std/acpi.h @@ -40,7 +40,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;
/* Table structure from Linux kernel (the ACPI tables are under the BSD license) */
Load files through romfile loader and use for acpi tables. We need the RSDP pointer to hang the rest of the tables off it, to detect that we simply scan all memory in FSEG.
Add an option to disable this feature (useful for old QEMU versions). This saves about 1Kbytes.
enabled: Total size: 134932 Fixed: 61571 Free: 127212 (used 51.5% of 256KiB rom)
disabled: Total size: 133836 Fixed: 61563 Free: 128308 (used 51.1% of 256KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/fw/acpi.c | 27 +++++++++++++++++++++++++++ src/Kconfig | 10 ++++++++++ 2 files changed, 37 insertions(+)
diff --git a/src/fw/acpi.c b/src/fw/acpi.c index 0497d9b..0fb8007 100644 --- a/src/fw/acpi.c +++ b/src/fw/acpi.c @@ -32,6 +32,7 @@ #include "string.h" // memset #include "util.h" // MaxCountCPUs #include "x86.h" // readl +#include "romfile_loader.h" // romfile_loader_execute
#include "src/fw/acpi-dsdt.hex"
@@ -606,6 +607,14 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in FSEG memory */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{ + u64 rsdp = cpu_to_le64(RSDP_SIGNATURE); + return malloc_find_fseg_pattern(&rsdp, sizeof(rsdp)); +} + #define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -615,6 +624,24 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
+ if (CONFIG_ACPI_ROMFILE_LOAD) { + int loader_err = romfile_loader_execute("etc/table-loader"); + + RsdpAddr = acpi_find_rsdp_rom(); + + if (RsdpAddr) + return; + + /* If present, loader should have installed an RSDP. + * Not installed? We might still be able to continue + * using the builtin RSDP. + */ + if (!loader_err) + warn_internalerror(); + } + + 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) diff --git a/src/Kconfig b/src/Kconfig index c40cc61..b5f0d39 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -414,6 +414,16 @@ menu "BIOS Tables" default y help Support generation of ACPI tables. + config ACPI_ROMFILE_LOAD + bool "Load ACPI tables from ROM files" + default y + depends on ACPI + help + Support loading ACPI tables from ROM files. + Required for QEMU 1.7 and newer. + This option can be disabled for QEMU 1.6 and older + to save some space in the ROM file. + If unsure, say Y. config ACPI_DSDT bool "Include default ACPI DSDT" default y
On Mon, Sep 23, 2013 at 09:20:34PM +0300, Michael S. Tsirkin wrote:
Load files through romfile loader and use for acpi tables. We need the RSDP pointer to hang the rest of the tables off it, to detect that we simply scan all memory in FSEG.
Add an option to disable this feature (useful for old QEMU versions). This saves about 1Kbytes.
enabled: Total size: 134932 Fixed: 61571 Free: 127212 (used 51.5% of 256KiB rom)
disabled: Total size: 133836 Fixed: 61563 Free: 128308 (used 51.1% of 256KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/fw/acpi.c | 27 +++++++++++++++++++++++++++ src/Kconfig | 10 ++++++++++ 2 files changed, 37 insertions(+)
diff --git a/src/fw/acpi.c b/src/fw/acpi.c index 0497d9b..0fb8007 100644 --- a/src/fw/acpi.c +++ b/src/fw/acpi.c @@ -32,6 +32,7 @@ #include "string.h" // memset #include "util.h" // MaxCountCPUs #include "x86.h" // readl +#include "romfile_loader.h" // romfile_loader_execute
#include "src/fw/acpi-dsdt.hex"
@@ -606,6 +607,14 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in FSEG memory */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- u64 rsdp = cpu_to_le64(RSDP_SIGNATURE);
- return malloc_find_fseg_pattern(&rsdp, sizeof(rsdp));
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -615,6 +624,24 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- if (CONFIG_ACPI_ROMFILE_LOAD) {
int loader_err = romfile_loader_execute("etc/table-loader");RsdpAddr = acpi_find_rsdp_rom();if (RsdpAddr)return;/* If present, loader should have installed an RSDP.* Not installed? We might still be able to continue* using the builtin RSDP.*/if (!loader_err)warn_internalerror();- }
- 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)
diff --git a/src/Kconfig b/src/Kconfig index c40cc61..b5f0d39 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -414,6 +414,16 @@ menu "BIOS Tables" default y help Support generation of ACPI tables.
- config ACPI_ROMFILE_LOAD
bool "Load ACPI tables from ROM files"default ydepends on ACPIhelpSupport loading ACPI tables from ROM files.Required for QEMU 1.7 and newer.This option can be disabled for QEMU 1.6 and olderto save some space in the ROM file.If unsure, say Y.
IMO, the config option isn't needed - the size savings doesn't seem to warrant the potentially broken results if it is inadvertently disabled. That said, if you really want it, then we can put it in, but it shouldn't be ACPI_ROMFILE_LOAD as we want the same mechanism for loading smbios and mptable.
-Kevin
On Tue, Sep 24, 2013 at 07:44:35PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:34PM +0300, Michael S. Tsirkin wrote:
Load files through romfile loader and use for acpi tables. We need the RSDP pointer to hang the rest of the tables off it, to detect that we simply scan all memory in FSEG.
Add an option to disable this feature (useful for old QEMU versions). This saves about 1Kbytes.
enabled: Total size: 134932 Fixed: 61571 Free: 127212 (used 51.5% of 256KiB rom)
disabled: Total size: 133836 Fixed: 61563 Free: 128308 (used 51.1% of 256KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/fw/acpi.c | 27 +++++++++++++++++++++++++++ src/Kconfig | 10 ++++++++++ 2 files changed, 37 insertions(+)
diff --git a/src/fw/acpi.c b/src/fw/acpi.c index 0497d9b..0fb8007 100644 --- a/src/fw/acpi.c +++ b/src/fw/acpi.c @@ -32,6 +32,7 @@ #include "string.h" // memset #include "util.h" // MaxCountCPUs #include "x86.h" // readl +#include "romfile_loader.h" // romfile_loader_execute
#include "src/fw/acpi-dsdt.hex"
@@ -606,6 +607,14 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in FSEG memory */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- u64 rsdp = cpu_to_le64(RSDP_SIGNATURE);
- return malloc_find_fseg_pattern(&rsdp, sizeof(rsdp));
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -615,6 +624,24 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- if (CONFIG_ACPI_ROMFILE_LOAD) {
int loader_err = romfile_loader_execute("etc/table-loader");RsdpAddr = acpi_find_rsdp_rom();if (RsdpAddr)return;/* If present, loader should have installed an RSDP.* Not installed? We might still be able to continue* using the builtin RSDP.*/if (!loader_err)warn_internalerror();- }
- 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)
diff --git a/src/Kconfig b/src/Kconfig index c40cc61..b5f0d39 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -414,6 +414,16 @@ menu "BIOS Tables" default y help Support generation of ACPI tables.
- config ACPI_ROMFILE_LOAD
bool "Load ACPI tables from ROM files"default ydepends on ACPIhelpSupport loading ACPI tables from ROM files.Required for QEMU 1.7 and newer.This option can be disabled for QEMU 1.6 and olderto save some space in the ROM file.If unsure, say Y.IMO, the config option isn't needed - the size savings doesn't seem to warrant the potentially broken results if it is inadvertently disabled. That said, if you really want it, then we can put it in, but it shouldn't be ACPI_ROMFILE_LOAD as we want the same mechanism for loading smbios and mptable.
-Kevin
I think we want an option to generate small bios for QEMU 1.6 and back.
So maybe just call it that?
Hi,
I think we want an option to generate small bios for QEMU 1.6 and back.
Isn't disabling xhci enougth to bring it below 128k?
cheers, Gerd
On Wed, Sep 25, 2013 at 09:52:53AM +0200, Gerd Hoffmann wrote:
Hi,
I think we want an option to generate small bios for QEMU 1.6 and back.
Isn't disabling xhci enougth to bring it below 128k?
cheers, Gerd
For now yes, but we'll add more code in the future. So I propose a new flag QEMU_LEGACY that disables all new features automatically. When it's off, this will also let us drop some legacy code thinkably.
On Mi, 2013-09-25 at 11:17 +0300, Michael S. Tsirkin wrote:
On Wed, Sep 25, 2013 at 09:52:53AM +0200, Gerd Hoffmann wrote:
Hi,
I think we want an option to generate small bios for QEMU 1.6 and back.
Isn't disabling xhci enougth to bring it below 128k?
cheers, Gerd
For now yes, but we'll add more code in the future. So I propose a new flag QEMU_LEGACY that disables all new features automatically. When it's off, this will also let us drop some legacy code thinkably.
I'd rather deal with that in qemu, i.e. fill the needed stuff into roms/config.seabios-128k and roms/config.seabios-256k. We'll need those anyway, and instead of placing CONFIG_QEMU_LEGACY={y,n} there we can equally well set the options directly.
cheers, Gerd
On Wed, Sep 25, 2013 at 10:40:49AM +0200, Gerd Hoffmann wrote:
On Mi, 2013-09-25 at 11:17 +0300, Michael S. Tsirkin wrote:
On Wed, Sep 25, 2013 at 09:52:53AM +0200, Gerd Hoffmann wrote:
Hi,
I think we want an option to generate small bios for QEMU 1.6 and back.
Isn't disabling xhci enougth to bring it below 128k?
cheers, Gerd
For now yes, but we'll add more code in the future. So I propose a new flag QEMU_LEGACY that disables all new features automatically. When it's off, this will also let us drop some legacy code thinkably.
I'd rather deal with that in qemu, i.e. fill the needed stuff into roms/config.seabios-128k and roms/config.seabios-256k. We'll need those anyway, and instead of placing CONFIG_QEMU_LEGACY={y,n} there we can equally well set the options directly.
cheers, Gerd
Imagine you have QEMU 1.7. You want to update bios, so you pull latest source. Of course roms/config.seabios-128k does not know which flags to set :(
How about explicit targets? QEMU_128K, QEMU_256K ...
commit 79dad487c4beb270d4c84f70f085464e90d00466 Author: Michael S. Tsirkin mst@redhat.com Date: Wed Sep 25 12:18:58 2013 +0300
Kconfig: add explicit QEMU 128/QEMU 256 targets
Older QEMU versions need a small bios <128K, and don't need lots of modern features (no hardware for them). Add an option you can use for that, so that QEMU developers don't need to go hunting: what's required and what's not.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
diff --git a/src/Kconfig b/src/Kconfig index 07b2e6f..5980512 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -6,18 +6,32 @@ menu "General Features"
choice prompt "Build Target" - default QEMU + default QEMU_256K
config COREBOOT bool "Build for coreboot" help Configure as a coreboot payload.
- config QEMU - bool "Build for QEMU/Xen/KVM/Bochs" + config QEMU_128K + bool "Build 128K binary for legacy QEMU/Xen/KVM/Bochs" + select QEMU + select QEMU_HARDWARE + help + Configure for an emulated machine (QEMU, Xen, KVM, or Bochs). + Select this option to generate smaller BIOS, disabling + some features to fit within 128Kbyte memory buffer used + by QEMU 1.6 and earlier. + + config QEMU_256K + bool "Build 256K binary for QEMU/Xen/KVM/Bochs" + select QEMU select QEMU_HARDWARE help Configure for an emulated machine (QEMU, Xen, KVM, or Bochs). + Select this option to generate a bigger (up to 256Kbyte), more + full-featured BIOS, enabling more features useful for QEMU 1.7 and + newer.
config CSM bool "Build as Compatibilty Support Module for EFI BIOS" @@ -27,6 +41,10 @@ choice
endchoice
+ config QEMU + bool + default n + config QEMU_HARDWARE bool "Support hardware found on emulators (QEMU/Xen/KVM/Bochs)" if !QEMU default n
Hi,
Imagine you have QEMU 1.7. You want to update bios, so you pull latest source. Of course roms/config.seabios-128k does not know which flags to set :(
Hmm? That is the whole point of the file, to carry the (non-default) config options ...
Latest qemu should have a up-to-date info for latest seabios release. When updating seabios to not-yet released master you better know what you are doing, I don't think there is a need to make that bullet-proof.
cheers, Gerd
On Wed, Sep 25, 2013 at 12:24:18PM +0200, Gerd Hoffmann wrote:
Hi,
Imagine you have QEMU 1.7. You want to update bios, so you pull latest source. Of course roms/config.seabios-128k does not know which flags to set :(
Hmm? That is the whole point of the file, to carry the (non-default) config options ...
Latest qemu should have a up-to-date info for latest seabios release. When updating seabios to not-yet released master you better know what you are doing, I don't think there is a need to make that bullet-proof.
cheers, Gerd
Simply put - it's in a separate tree so it will be foreever out of sync.
Example: you want to try and reproduce a bug with latest seabios. You build it with options from which qemu? The one you will be testing on right? Wrong - it might not work.
This just does not work - it's not reasonable for QEMU to carry guest knowledge around. Instead guests should go "I want to work on that QEMU". And QEMU will supply specific interfaces.
config options is also not an interface we can support. So there will be pain when we rename some option. A simple "small bios/large bios" is an interface we should have no trouble supporting for a long time, so it will be possible for qemu to carry that around.
On Mi, 2013-09-25 at 13:51 +0300, Michael S. Tsirkin wrote:
On Wed, Sep 25, 2013 at 12:24:18PM +0200, Gerd Hoffmann wrote:
Hi,
Imagine you have QEMU 1.7. You want to update bios, so you pull latest source. Of course roms/config.seabios-128k does not know which flags to set :(
Hmm? That is the whole point of the file, to carry the (non-default) config options ...
Latest qemu should have a up-to-date info for latest seabios release. When updating seabios to not-yet released master you better know what you are doing, I don't think there is a need to make that bullet-proof.
cheers, Gerd
Simply put - it's in a separate tree so it will be foreever out of sync.
It may happen now and then. It's not like we are changing the config daily.
Example: you want to try and reproduce a bug with latest seabios. You build it with options from which qemu?
In most cases you can just go with the default config and be done with it, with cross version live migration testing being pretty much the only exception.
config options is also not an interface we can support. So there will be pain when we rename some option.
When doing it the other way around flipping an config option for -- say the 128k rom -- will be a pain:
(1) patch seabios (2) wait for next seabios release (3) update seabios in qemu to the new release
Of course you can take a shortcut and just update roms/config.seabios-128k in qemu instead. Assuming the kconfig dependencies allow to do that.
I think it is easier to just have the config in roms/config.seabios-128k ...
cheers, Gerd
On Wed, Sep 25, 2013 at 03:59:17PM +0200, Gerd Hoffmann wrote:
On Mi, 2013-09-25 at 13:51 +0300, Michael S. Tsirkin wrote:
On Wed, Sep 25, 2013 at 12:24:18PM +0200, Gerd Hoffmann wrote:
Hi,
Imagine you have QEMU 1.7. You want to update bios, so you pull latest source. Of course roms/config.seabios-128k does not know which flags to set :(
Hmm? That is the whole point of the file, to carry the (non-default) config options ...
Latest qemu should have a up-to-date info for latest seabios release. When updating seabios to not-yet released master you better know what you are doing, I don't think there is a need to make that bullet-proof.
cheers, Gerd
Simply put - it's in a separate tree so it will be foreever out of sync.
It may happen now and then. It's not like we are changing the config daily.
Example: you want to try and reproduce a bug with latest seabios. You build it with options from which qemu?
In most cases you can just go with the default config and be done with it, with cross version live migration testing being pretty much the only exception.
Well one exception is enough.
config options is also not an interface we can support. So there will be pain when we rename some option.
When doing it the other way around flipping an config option for -- say the 128k rom -- will be a pain:
I don't understand. 128K is for lkegacy machine types. It is highly unlikely you will want to add more features there.
(1) patch seabios (2) wait for next seabios release (3) update seabios in qemu to the new release
? How is this different from what we do now?
Of course you can take a shortcut and just update roms/config.seabios-128k in qemu instead. Assuming the kconfig dependencies allow to do that.
You mean for seabios developers? They can just patch kconfig. And it will be much easier to do it in seabios tree, and just use standard qemu binary. No need to dig out qemu source just to build seabios.
I think it is easier to just have the config in roms/config.seabios-128k ...
cheers, Gerd
On Tue, Sep 24, 2013 at 07:44:35PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:34PM +0300, Michael S. Tsirkin wrote:
Load files through romfile loader and use for acpi tables. We need the RSDP pointer to hang the rest of the tables off it, to detect that we simply scan all memory in FSEG.
Add an option to disable this feature (useful for old QEMU versions). This saves about 1Kbytes.
enabled: Total size: 134932 Fixed: 61571 Free: 127212 (used 51.5% of 256KiB rom)
disabled: Total size: 133836 Fixed: 61563 Free: 128308 (used 51.1% of 256KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com
src/fw/acpi.c | 27 +++++++++++++++++++++++++++ src/Kconfig | 10 ++++++++++ 2 files changed, 37 insertions(+)
diff --git a/src/fw/acpi.c b/src/fw/acpi.c index 0497d9b..0fb8007 100644 --- a/src/fw/acpi.c +++ b/src/fw/acpi.c @@ -32,6 +32,7 @@ #include "string.h" // memset #include "util.h" // MaxCountCPUs #include "x86.h" // readl +#include "romfile_loader.h" // romfile_loader_execute
#include "src/fw/acpi-dsdt.hex"
@@ -606,6 +607,14 @@ static const struct pci_device_id acpi_find_tbl[] = {
struct rsdp_descriptor *RsdpAddr;
+/* Look for RSDP signature in FSEG memory */ +struct rsdp_descriptor * +acpi_find_rsdp_rom(void) +{
- u64 rsdp = cpu_to_le64(RSDP_SIGNATURE);
- return malloc_find_fseg_pattern(&rsdp, sizeof(rsdp));
+}
#define MAX_ACPI_TABLES 20 void acpi_setup(void) @@ -615,6 +624,24 @@ acpi_setup(void)
dprintf(3, "init ACPI tables\n");
- if (CONFIG_ACPI_ROMFILE_LOAD) {
int loader_err = romfile_loader_execute("etc/table-loader");RsdpAddr = acpi_find_rsdp_rom();if (RsdpAddr)return;/* If present, loader should have installed an RSDP.* Not installed? We might still be able to continue* using the builtin RSDP.*/if (!loader_err)warn_internalerror();- }
- 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)
diff --git a/src/Kconfig b/src/Kconfig index c40cc61..b5f0d39 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -414,6 +414,16 @@ menu "BIOS Tables" default y help Support generation of ACPI tables.
- config ACPI_ROMFILE_LOAD
bool "Load ACPI tables from ROM files"default ydepends on ACPIhelpSupport loading ACPI tables from ROM files.Required for QEMU 1.7 and newer.This option can be disabled for QEMU 1.6 and olderto save some space in the ROM file.If unsure, say Y.IMO, the config option isn't needed - the size savings doesn't seem to warrant the potentially broken results if it is inadvertently disabled. That said, if you really want it, then we can put it in, but it shouldn't be ACPI_ROMFILE_LOAD as we want the same mechanism for loading smbios and mptable.
-Kevin
Okay FW_ROMFILE_LOAD then :).
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/fw/acpi.c | 3 +++ src/Kconfig | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/fw/acpi.c b/src/fw/acpi.c index 0fb8007..31404fa 100644 --- a/src/fw/acpi.c +++ b/src/fw/acpi.c @@ -640,6 +640,9 @@ acpi_setup(void) warn_internalerror(); }
+ if (!CONFIG_ACPI_BUILTIN) + return; + dprintf(3, "generate ACPI tables\n");
// This code is hardcoded for PIIX4 Power Management device. diff --git a/src/Kconfig b/src/Kconfig index b5f0d39..e405664 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -424,10 +424,20 @@ menu "BIOS Tables" This option can be disabled for QEMU 1.6 and older to save some space in the ROM file. If unsure, say Y. + 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.6 and older. + This option can be disabled for QEMU 1.7 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.
On Mon, Sep 23, 2013 at 09:20:37PM +0300, Michael S. Tsirkin wrote:
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)
I don't think this is necessary - CONFIG_ACPI can be used to disable local ACPI table generation. (If QEMU has a "romfile loader" file then SeaBIOS should use it irrespective of CONFIG_ACPI.)
-Kevin
On Tue, Sep 24, 2013 at 07:45:11PM -0400, Kevin O'Connor wrote:
On Mon, Sep 23, 2013 at 09:20:37PM +0300, Michael S. Tsirkin wrote:
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)
I don't think this is necessary - CONFIG_ACPI can be used to disable local ACPI table generation. (If QEMU has a "romfile loader" file then SeaBIOS should use it irrespective of CONFIG_ACPI.)
-Kevin
Sounds good.
-
Gerd Hoffmann -
Kevin O'Connor -
Michael S. Tsirkin