Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes: - Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) - Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) - After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) - Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
BIOS still has limited ability to parse the tables, for the following purposes: - locate resume vector - allocate RSDP in FSEG - allocate FACS at an aligned address
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 | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 50 +++++++++++++++++++++++++++++++++ src/util.h | 1 + 4 files changed, 142 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..223d2db --- /dev/null +++ b/src/linker.c @@ -0,0 +1,90 @@ +#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 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->reserved1 || entry->reserved2) + continue; + if (entry->bytes != 1 && + entry->bytes != 2 && + entry->bytes != 4 && + entry->bytes != 8) + continue; + if (entry->shift > 63) + continue; + if (entry->type >= LINKER_ENTRY_TYPE_MAX) + 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) { + warn_internalerror(); + continue; + } + src = romfile_find(entry->src_dst); + if (!src) { + warn_internalerror(); + continue; + } + if (!src->data) { + warn_internalerror(); + continue; + } + if (lsrc + 1 + sizeof *entry >= entry->size) { + warn_internalerror(); + continue; + } + dst = romfile_find(entry->src_dst + lsrc + 1); + if (!dst) { + warn_internalerror(); + continue; + } + if (!dst->data) { + warn_internalerror(); + continue; + } + 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 = ((u64)(unsigned long)src->data) >> entry->shift; + 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..d8f4a79 --- /dev/null +++ b/src/linker.h @@ -0,0 +1,50 @@ +#ifndef __LINKER_H +#define __LINKER_H + +#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 shift; /* Shift source address right by this many bits. 0-63. */ + u8 type; + u8 format; + u8 reserved1; + u16 reserved2; + 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; + +/* Note: align types must appear before all other types. */ +enum linker_entry_type { + /* + * increment value in DST by the address of source + * useful e.g. to fill in pointer values in ACPI tables + */ + + LINKER_ENTRY_TYPE_ADD = 0x0, + /* + * Decrement value in DST by the address of source + * useful e.g. to fix up checksum values in ACPI tables + */ + LINKER_ENTRY_TYPE_SUB = 0x1, + /* + * The following move source so must come first, + * before sorce is used by other operations. + */ + /* Last entry. Not a valid type. */ + LINKER_ENTRY_TYPE_MAX, +}; + +enum linker_entry_format { + LINKER_ENTRY_FORMAT_LE = 0x0, +}; + +void linker_link(const char *name); + +#endif 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);
Load files in /etc/acpi/ and use for acpi tables. Any files in this directory completely disable generating and loading legacy acpi tables.
Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/acpi.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/src/acpi.c b/src/acpi.c index 58cd6d7..16ea9f4 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"
@@ -603,8 +604,72 @@ acpi_setup(void) if (! CONFIG_ACPI) return;
+ int acpi_generate = 1; + dprintf(3, "init ACPI tables\n");
+ struct romfile_s *file = NULL; + for (;;) { + file = romfile_findprefix("/etc/acpi/", file); + if (!file) + break; + + /* + * Disable ACPI table generation. All ACPI tables must come from + * etc/acpi/ romfile entries. + */ + acpi_generate = 0; + + if (!file->size) + continue; + + void *data = malloc_high(file->size); + if (!data) { + warn_noalloc(); + break; + } + int ret = file->copy(file, data, file->size); + if (ret < file->size) { + free(data); + continue; + } + /* Handle RSDP and FACS quirks */ + if (file->size >= 8) { + struct rsdp_descriptor *rsdp = data; + struct acpi_table_header *hdr = data; + /* RSDP is in FSEG memory. */ + if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE)) { + data = malloc_fseg(file->size); + if (!data) { + warn_noalloc(); + break; + } + memcpy(data, rsdp, file->size); + free(rsdp); + /* Store Rsdp pointer for use by find_fadt. */ + RsdpAddr = data; + } else if (hdr->signature == FACS_SIGNATURE) { + /* FACS is aligned to a 64 bit boundary. */ + data = memalign_high(64, file->size); + if (!data) { + warn_noalloc(); + break; + } + memcpy(data, hdr, file->size); + free(hdr); + } + } + file->data = data; + } + + linker_link("/etc/linker-script"); + + if (!acpi_generate) { + 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) @@ -630,7 +695,7 @@ acpi_setup(void) if (pci->device == PCI_DEVICE_ID_INTEL_ICH9_LPC) ACPI_INIT_TABLE(build_mcfg_q35());
- struct romfile_s *file = NULL; + file = NULL; for (;;) { file = romfile_findprefix("acpi/", file); if (!file)
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 3c80132..1b54b83 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 16ea9f4..b03b2ba 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -664,7 +664,7 @@ acpi_setup(void)
linker_link("/etc/linker-script");
- if (!acpi_generate) { + if (!CONFIG_ACPI_BUILTIN || !acpi_generate) { return; }
With ACPI moved out of SeaBIOS to QEMU, how will ACPI work when using SeaBIOS without QEMU?
Like if using SeaBIOS with Boch, KVM or Coreboot?
On Thu, Apr 25, 2013 at 11:02 AM, Michael S. Tsirkin mst@redhat.com wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB
rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
BIOS still has limited ability to parse the tables, for the following purposes: - locate resume vector - allocate RSDP in FSEG - allocate FACS at an aligned address
-- MST
Michael S. Tsirkin (3): linker: utility to patch in-memory ROM files acpi: load and link tables from /etc/acpi/ acpi: add an option to disable builtin tables
Makefile | 2 +- src/Kconfig | 12 +++++++- src/acpi.c | 67 +++++++++++++++++++++++++++++++++++++++++++- src/linker.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 50 +++++++++++++++++++++++++++++++++ src/util.h | 1 + 6 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 src/linker.c create mode 100644 src/linker.h
-- MST
SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
- bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
- further, qemu could do the copy of tables into that address directly
thoughts?
BIOS still has limited ability to parse the tables, for the following purposes:
- locate resume vector
- allocate RSDP in FSEG
- allocate FACS at an aligned address
-- MST
Michael S. Tsirkin (3): linker: utility to patch in-memory ROM files acpi: load and link tables from /etc/acpi/ acpi: add an option to disable builtin tables
Makefile | 2 +- src/Kconfig | 12 +++++++- src/acpi.c | 67 +++++++++++++++++++++++++++++++++++++++++++- src/linker.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/linker.h | 50 +++++++++++++++++++++++++++++++++ src/util.h | 1 + 6 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 src/linker.c create mode 100644 src/linker.h
-- MST
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
-Kevin
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
-- Gleb.
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
I don't care much.
-- Gleb.
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
I don't care much.
-- Gleb.
-- Gleb.
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
I don't care much.
-- Gleb.
-- Gleb.
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: > Untested yet, but I thought I'd share the > BIOS bits so we can agree on direction. > > In particular check out ROM sizes: > - Before patchset with DSDT enabled > Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) > - Before patchset with DSDT disabled > Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) > - After patchset: > Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) > - Legacy disabled at build time: > Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) > > As can be seen from this, most size savings come > from dropping DSDT, but we do save a bit by removing > other tables. Of course the real reason to move tables to QEMU > is so that ACPI can better match hardware. > > This patchset adds an option to move all code for formatting acpi tables > out of BIOS. With this, QEMU has full control over the table layout. > All tables are loaded from the new "/etc/acpi/" directory. > Any entries in this directory cause BIOS to disable > ACPI table generation completely. > A generic linker script, controlled by QEMU, is > loaded from "/etc/linker-script". It is used to > patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted.
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
-- Gleb.
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: > On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: > > Untested yet, but I thought I'd share the > > BIOS bits so we can agree on direction. > > > > In particular check out ROM sizes: > > - Before patchset with DSDT enabled > > Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) > > - Before patchset with DSDT disabled > > Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) > > - After patchset: > > Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) > > - Legacy disabled at build time: > > Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) > > > > As can be seen from this, most size savings come > > from dropping DSDT, but we do save a bit by removing > > other tables. Of course the real reason to move tables to QEMU > > is so that ACPI can better match hardware. > > > > This patchset adds an option to move all code for formatting acpi tables > > out of BIOS. With this, QEMU has full control over the table layout. > > All tables are loaded from the new "/etc/acpi/" directory. > > Any entries in this directory cause BIOS to disable > > ACPI table generation completely. > > A generic linker script, controlled by QEMU, is > > loaded from "/etc/linker-script". It is used to > > patch in table pointers and checksums. > > After some thought, there are two additional > options worth considering, in that they simplify > bios code somewhat: > > - bios could get size from qemu, allocate a buffer > (e.g. could be one buffer for all tables) > and pass the address to qemu. > qemu does all the patching > > - further, qemu could do the copy of tables into > that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
+1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted.
Why would it be corrupted?
In any case, FACS has a hardware signature value for just such a case. If we know VM can not be resumed on new QEMU, we can change the signature and it will cold-boot instead.
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
-- Gleb.
I'm not arguing with that. Convince Anthony please.
On Wed, May 08, 2013 at 02:07:24PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: > On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: > > On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: > > > Untested yet, but I thought I'd share the > > > BIOS bits so we can agree on direction. > > > > > > In particular check out ROM sizes: > > > - Before patchset with DSDT enabled > > > Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) > > > - Before patchset with DSDT disabled > > > Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) > > > - After patchset: > > > Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) > > > - Legacy disabled at build time: > > > Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) > > > > > > As can be seen from this, most size savings come > > > from dropping DSDT, but we do save a bit by removing > > > other tables. Of course the real reason to move tables to QEMU > > > is so that ACPI can better match hardware. > > > > > > This patchset adds an option to move all code for formatting acpi tables > > > out of BIOS. With this, QEMU has full control over the table layout. > > > All tables are loaded from the new "/etc/acpi/" directory. > > > Any entries in this directory cause BIOS to disable > > > ACPI table generation completely. > > > A generic linker script, controlled by QEMU, is > > > loaded from "/etc/linker-script". It is used to > > > patch in table pointers and checksums. > > > > After some thought, there are two additional > > options worth considering, in that they simplify > > bios code somewhat: > > > > - bios could get size from qemu, allocate a buffer > > (e.g. could be one buffer for all tables) > > and pass the address to qemu. > > qemu does all the patching > > > > - further, qemu could do the copy of tables into > > that address directly > > This seems more complex than necessary to me. > > The important task is to get the tables generated in QEMU - I'd focus > on getting the tables generated in QEMU (one table per fw_cfg "file"). > Once that is done, the SeaBIOS side can be easily implemented, and we > can add any enhancements on top if we feel it is necessary. > +1. This "copy of tables into that address directly" is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted.
Why would it be corrupted?
Because ACPI tables are stored in the memory marked as "ACPI data" (IIRC seabios mark them as reserved). Guests do not save reserved memory during S4 (don't know about "ACPI data", but if guest does not copy tables from it to another location I doubt it saves the memory, anyway ACPI spec does not mandate it), so what happens on resume if the memory grows? Part of it, that was not marked as reserved before S4, is re-written with whatever data guest had there and rest contains now corrupted ACPI tables that BIOS put there during boot. We can hope that guest is smart enough to see that memory map changed and refuse to resume or we can put ACPI tables into NVS memory which has to be saved and restored during S4 by OSPM.
In any case, FACS has a hardware signature value for just such a case. If we know VM can not be resumed on new QEMU, we can change the signature and it will cold-boot instead.
Nice, does Linux check it?
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
-- Gleb.
I'm not arguing with that. Convince Anthony please.
Convince him in what? That fw_cfg is broken vrt migration and there are cases that will fail _today_ without any ACPI related changes? This is knows for ages.
-- Gleb.
On Wed, May 08, 2013 at 02:35:44PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 02:07:24PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote: > On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: > > On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: > > > On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: > > > > Untested yet, but I thought I'd share the > > > > BIOS bits so we can agree on direction. > > > > > > > > In particular check out ROM sizes: > > > > - Before patchset with DSDT enabled > > > > Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) > > > > - Before patchset with DSDT disabled > > > > Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) > > > > - After patchset: > > > > Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) > > > > - Legacy disabled at build time: > > > > Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) > > > > > > > > As can be seen from this, most size savings come > > > > from dropping DSDT, but we do save a bit by removing > > > > other tables. Of course the real reason to move tables to QEMU > > > > is so that ACPI can better match hardware. > > > > > > > > This patchset adds an option to move all code for formatting acpi tables > > > > out of BIOS. With this, QEMU has full control over the table layout. > > > > All tables are loaded from the new "/etc/acpi/" directory. > > > > Any entries in this directory cause BIOS to disable > > > > ACPI table generation completely. > > > > A generic linker script, controlled by QEMU, is > > > > loaded from "/etc/linker-script". It is used to > > > > patch in table pointers and checksums. > > > > > > After some thought, there are two additional > > > options worth considering, in that they simplify > > > bios code somewhat: > > > > > > - bios could get size from qemu, allocate a buffer > > > (e.g. could be one buffer for all tables) > > > and pass the address to qemu. > > > qemu does all the patching > > > > > > - further, qemu could do the copy of tables into > > > that address directly > > > > This seems more complex than necessary to me. > > > > The important task is to get the tables generated in QEMU - I'd focus > > on getting the tables generated in QEMU (one table per fw_cfg "file"). > > Once that is done, the SeaBIOS side can be easily implemented, and we > > can add any enhancements on top if we feel it is necessary. > > > +1. This "copy of tables into that address directly" is just an ad-hoc PV > isa DMA device in disguise. Such device was refused when libguestfs > asked for it, and they wanted it for much better reason - performance. > There is existing mechanism to pass data into firmware. Use it please.
Yes I can code it up using FW_CFG for now.
One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions.
Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted.
Why would it be corrupted?
Because ACPI tables are stored in the memory marked as "ACPI data" (IIRC seabios mark them as reserved). Guests do not save reserved memory during S4 (don't know about "ACPI data", but if guest does not copy tables from it to another location I doubt it saves the memory, anyway ACPI spec does not mandate it), so what happens on resume if the memory grows? Part of it, that was not marked as reserved before S4, is re-written with whatever data guest had there and rest contains now corrupted ACPI tables that BIOS put there during boot. We can hope that guest is smart enough to see that memory map changed and refuse to resume or we can put ACPI tables into NVS memory which has to be saved and restored during S4 by OSPM.
This easily solvable by asking for memory with a solid margin.
But what you write above sounds strange: would not it apply to any memory allocated with malloc_high? If so, a minor change in BIOS e.g. where we allocate a bit more, less, or in a different order, will break suspend.
In any case, FACS has a hardware signature value for just such a case. If we know VM can not be resumed on new QEMU, we can change the signature and it will cold-boot instead.
Nice, does Linux check it?
If not we can fix it.
Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea.
Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
-- Gleb.
I'm not arguing with that. Convince Anthony please.
Convince him in what? That fw_cfg is broken vrt migration and there are cases that will fail _today_ without any ACPI related changes? This is knows for ages.
-- Gleb.
That we should use fw_cfg to load acpi tables.
On Wed, May 08, 2013 at 03:35:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 02:35:44PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 02:07:24PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote: > On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote: > > On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: > > > On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: > > > > On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: > > > > > Untested yet, but I thought I'd share the > > > > > BIOS bits so we can agree on direction. > > > > > > > > > > In particular check out ROM sizes: > > > > > - Before patchset with DSDT enabled > > > > > Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) > > > > > - Before patchset with DSDT disabled > > > > > Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) > > > > > - After patchset: > > > > > Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) > > > > > - Legacy disabled at build time: > > > > > Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) > > > > > > > > > > As can be seen from this, most size savings come > > > > > from dropping DSDT, but we do save a bit by removing > > > > > other tables. Of course the real reason to move tables to QEMU > > > > > is so that ACPI can better match hardware. > > > > > > > > > > This patchset adds an option to move all code for formatting acpi tables > > > > > out of BIOS. With this, QEMU has full control over the table layout. > > > > > All tables are loaded from the new "/etc/acpi/" directory. > > > > > Any entries in this directory cause BIOS to disable > > > > > ACPI table generation completely. > > > > > A generic linker script, controlled by QEMU, is > > > > > loaded from "/etc/linker-script". It is used to > > > > > patch in table pointers and checksums. > > > > > > > > After some thought, there are two additional > > > > options worth considering, in that they simplify > > > > bios code somewhat: > > > > > > > > - bios could get size from qemu, allocate a buffer > > > > (e.g. could be one buffer for all tables) > > > > and pass the address to qemu. > > > > qemu does all the patching > > > > > > > > - further, qemu could do the copy of tables into > > > > that address directly > > > > > > This seems more complex than necessary to me. > > > > > > The important task is to get the tables generated in QEMU - I'd focus > > > on getting the tables generated in QEMU (one table per fw_cfg "file"). > > > Once that is done, the SeaBIOS side can be easily implemented, and we > > > can add any enhancements on top if we feel it is necessary. > > > > > +1. This "copy of tables into that address directly" is just an ad-hoc PV > > isa DMA device in disguise. Such device was refused when libguestfs > > asked for it, and they wanted it for much better reason - performance. > > There is existing mechanism to pass data into firmware. Use it please. > > Yes I can code it up using FW_CFG for now. > > One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, > unless we pass in very small bits of data which we > can guarantee never changes across qemu versions. > Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway?
That's not practical. They are too big to stay completely unchanged.
I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted.
Why would it be corrupted?
Because ACPI tables are stored in the memory marked as "ACPI data" (IIRC seabios mark them as reserved). Guests do not save reserved memory during S4 (don't know about "ACPI data", but if guest does not copy tables from it to another location I doubt it saves the memory, anyway ACPI spec does not mandate it), so what happens on resume if the memory grows? Part of it, that was not marked as reserved before S4, is re-written with whatever data guest had there and rest contains now corrupted ACPI tables that BIOS put there during boot. We can hope that guest is smart enough to see that memory map changed and refuse to resume or we can put ACPI tables into NVS memory which has to be saved and restored during S4 by OSPM.
This easily solvable by asking for memory with a solid margin.
But what you write above sounds strange: would not it apply to any memory allocated with malloc_high? If so, a minor change in BIOS e.g. where we allocate a bit more, less, or in a different order, will break suspend.
Good point. I really do not know how S4 resume handles memory map changes. The only sane way that I see is to refuse resume if memory map changes.
In any case, FACS has a hardware signature value for just such a case. If we know VM can not be resumed on new QEMU, we can change the signature and it will cold-boot instead.
Nice, does Linux check it?
If not we can fix it.
> Off-list, I suggested fixing it and migrating file > content, but Anthony thinks it's a bad idea. > Why is this a bad idea to fix device migration?
You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism.
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
-- Gleb.
I'm not arguing with that. Convince Anthony please.
Convince him in what? That fw_cfg is broken vrt migration and there are cases that will fail _today_ without any ACPI related changes? This is knows for ages.
-- Gleb.
That we should use fw_cfg to load acpi tables.
I haven't seen his arguments against it.
-- Gleb.
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
-Kevin
I have kind of done this, though only compile-tested for now - still need to update the bios with the new linker interface along the lines suggested by you.
If you want to see how the code looks like check out
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi the code is in hw/i386/acpi-build.c and hw/i386/bios-linker-loader.c
the history is all messed up now, I'll clean it up shortly.
That said, this uses fw_cfg so for this to be acceptable, we need to fix migration with big fw_cfg files.
On Wed, May 08, 2013 at 03:35:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 02:35:44PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 02:07:24PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
I'm not arguing with that. Convince Anthony please.
Convince him in what? That fw_cfg is broken vrt migration and there are cases that will fail _today_ without any ACPI related changes? This is knows for ages.
That we should use fw_cfg to load acpi tables.
I'm confused.
ACPI tables are not large. At most we're talking about 100K of data total.
I don't see what migration has to do with using fw_cfg to pass acpi tables - the content is only read at startup. There may be an issue for the corner case of VM restarts, but if so it's nothing new. If the content of a fw_cfg entry changes during a guest reboot it is going to have the same impact regardless of whether it's the "irq0-override" entry / "numa-nodes" entry - or if it's the "madt" entry / "srat" entry, etc. So, I don't see how fw_cfg would suddenly not be suitable.
Again, I recommend that ACPI (and mptable, smbios, pir) be generated in qemu and that the content be passed to SeaBIOS using one fw_cfg "file" per table.
-Kevin
On Wed, May 08, 2013 at 09:15:44PM +0300, Michael S. Tsirkin wrote:
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
Untested yet, but I thought I'd share the BIOS bits so we can agree on direction.
In particular check out ROM sizes:
- Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
- After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom)
As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware.
This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new "/etc/acpi/" directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from "/etc/linker-script". It is used to patch in table pointers and checksums.
After some thought, there are two additional options worth considering, in that they simplify bios code somewhat:
bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching
further, qemu could do the copy of tables into that address directly
This seems more complex than necessary to me.
The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg "file"). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary.
-Kevin
I have kind of done this, though only compile-tested for now - still need to update the bios with the new linker interface along the lines suggested by you.
If you want to see how the code looks like check out
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi the code is in hw/i386/acpi-build.c and hw/i386/bios-linker-loader.c
the history is all messed up now, I'll clean it up shortly.
That said, this uses fw_cfg so for this to be acceptable, we need to fix migration with big fw_cfg files.
We need to fix it anyway ;)
-- Gleb.
On Wed, May 08, 2013 at 06:55:22PM -0400, Kevin O'Connor wrote:
On Wed, May 08, 2013 at 03:35:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 02:35:44PM +0300, Gleb Natapov wrote:
On Wed, May 08, 2013 at 02:07:24PM +0300, Michael S. Tsirkin wrote:
On Wed, May 08, 2013 at 01:59:12PM +0300, Gleb Natapov wrote:
Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway.
I'm not arguing with that. Convince Anthony please.
Convince him in what? That fw_cfg is broken vrt migration and there are cases that will fail _today_ without any ACPI related changes? This is knows for ages.
That we should use fw_cfg to load acpi tables.
I'm confused.
ACPI tables are not large. At most we're talking about 100K of data total.
I don't see what migration has to do with using fw_cfg to pass acpi tables - the content is only read at startup. There may be an issue for the corner case of VM restarts, but if so it's nothing new. If the content of a fw_cfg entry changes during a guest reboot it is going to have the same impact regardless of whether it's the "irq0-override" entry / "numa-nodes" entry - or if it's the "madt" entry / "srat" entry, etc. So, I don't see how fw_cfg would suddenly not be suitable.
If the tables at the source of the migration and the destination differs and migration happens while seabios is reading them guest will have corrupted ACPI tables at the destination. The problem is not new. The same is true for reading option rom or splash screen or bootindex file, basically anything that we read via fw_cfg interface and it can be different between two qemu version. The window where the bug may happen is very small, so we never saw such problem in practice to my knowledge and the fix should be simple too: migrate fw_cfg that is been read during migration.
Again, I recommend that ACPI (and mptable, smbios, pir) be generated in qemu and that the content be passed to SeaBIOS using one fw_cfg "file" per table.
+1 again.
-- Gleb.
-
Fred . -
Gleb Natapov -
Kevin O'Connor -
Michael S. Tsirkin