Thanks for this, Paolo. Very interesting idea. I couldn't get things working initially, but with a few fixups on the SeaBIOS side I can boot both legacy and modern OSes. See comments inline below for details on changes required.
Successfully booted (only a brief test): - Windows 2000 - Windows XP (32 bit) - Windows 7 (32 bit) - Windows 10 (64 bit, SeaBIOS) - Windows 10 (64 bit, OVMF) - macOS 10.12 (patched OVMF) On Tue, Jul 25, 2017 at 7:10 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > On 25/07/2017 18:23, Paolo Bonzini wrote: >> On 25/07/2017 18:14, Laszlo Ersek wrote: >>> "No regressions became apparent in tests with a range of Windows >>> (XP-10)" >>> >>> In theory, w2k falls within that range. >> >> Nope, Windows 2000 is like NT 5.0, XP is like NT 5.1. :( >> >> One possibility is to fix it in SeaBIOS instead: if you get a 2.0 FADT >> and an XSDT and no RSDT, it can build an RSDT and a 1.0 FADT itself, >> patching the RSDT to point to it. >> >> It's a hack, but it's the only place I see to make it "just work". And >> it could be extended nicely in the future. >> >> All QEMU would have to do is to provide an XSDT _instead_ of an RSDT. > > Completely untested code follows. Machine type compatibility code > should not be necessary because new QEMU always starts with a new BIOS. > > Not sure I'll have much time for this in the next few days, so help > is appreciated. > > Paolo > > QEMU: > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 36a6cc450e..ea750d54d9 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -1573,33 +1573,6 @@ void acpi_build_tables_cleanup(AcpiBuildTables > *tables, bool mfre) > g_array_free(tables->vmgenid, mfre); > } > > -/* Build rsdt table */ > -void > -build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > - const char *oem_id, const char *oem_table_id) > -{ > - int i; > - unsigned rsdt_entries_offset; > - AcpiRsdtDescriptorRev1 *rsdt; > - const unsigned table_data_len = (sizeof(uint32_t) * table_offsets->len); > - const unsigned rsdt_entry_size = sizeof(rsdt->table_offset_entry[0]); > - const size_t rsdt_len = sizeof(*rsdt) + table_data_len; > - > - rsdt = acpi_data_push(table_data, rsdt_len); > - rsdt_entries_offset = (char *)rsdt->table_offset_entry - > table_data->data; > - for (i = 0; i < table_offsets->len; ++i) { > - uint32_t ref_tbl_offset = g_array_index(table_offsets, uint32_t, i); > - uint32_t rsdt_entry_offset = rsdt_entries_offset + rsdt_entry_size * > i; > - > - /* rsdt->table_offset_entry to be filled by Guest linker */ > - bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_TABLE_FILE, rsdt_entry_offset, rsdt_entry_size, > - ACPI_BUILD_TABLE_FILE, ref_tbl_offset); > - } > - build_header(linker, table_data, > - (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id); > -} > - > /* Build xsdt table */ > void > build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 6b7bade183..ad00f6affd 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2557,12 +2557,12 @@ build_amd_iommu(GArray *table_data, BIOSLinker > *linker) > } > > static GArray * > -build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset) > +build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset) > { > AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp); > - unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address); > - unsigned rsdt_pa_offset = > - (char *)&rsdp->rsdt_physical_address - rsdp_table->data; > + unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address); > + unsigned xsdt_pa_offset = > + (char *)&rsdp->xsdt_physical_address - rsdp_table->data; > > bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16, > true /* fseg memory */); > @@ -2571,8 +2571,8 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, > unsigned rsdt_tbl_offset) > memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); > /* Address to be filled by Guest linker */ > bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size, > - ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset); > + ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size, > + ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset); > > /* Checksum to be filled by Guest linker */ > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > @@ -2621,7 +2621,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState > *machine) > PCMachineState *pcms = PC_MACHINE(machine); > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); > GArray *table_offsets; > - unsigned facs, dsdt, rsdt, fadt; > + unsigned facs, dsdt, xsdt, fadt; > AcpiPmInfo pm; > AcpiMiscInfo misc; > AcpiMcfgInfo mcfg; > @@ -2730,12 +2730,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState > *machine) > } > > /* RSDT is pointed to by RSDP */ > - rsdt = tables_blob->len; > - build_rsdt(tables_blob, tables->linker, table_offsets, > + xsdt = tables_blob->len; > + build_xsdt(tables_blob, tables->linker, table_offsets, > slic_oem.id, slic_oem.table_id); > > /* RSDP is in FSEG memory, so allocate it separately */ > - build_rsdp(tables->rsdp, tables->linker, rsdt); > + build_rsdp(tables->rsdp, tables->linker, xsdt); > > /* We'll expose it all to Guest so we want to reduce > * chance of size changes. > > > SeaBIOS: > > From 73b0facdb7349f5dbf24dc675647b68eeeec0ff4 Mon Sep 17 00:00:00 2001 > From: Paolo Bonzini <pbonz...@redhat.com> > Date: Tue, 25 Jul 2017 18:50:19 +0200 > Subject: [PATCH 1/2] seabios: build RSDT from XSDT > > Old operating systems would like to have a v1 FADT, but new > operating systems would like to have v3. > > Since old operating systems do not know about XSDTs, the > solution is to point the RSDT to a v1 FADT and the XSDT to a > v3 FADT. > > But, OVMF is not able to do that and barfs when it sees the > second FADT---plus really it's only BIOS operating systems > such as win2k that complain. So instead: 1) make QEMU provide > an XSDT only; 2) build the RSDT and v1 FADT in SeaBIOS. > > This patch makes SeaBIOS build an RSDT out of an existing XSDT. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > src/fw/paravirt.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- > src/std/acpi.h | 11 +++++++++++ > 2 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c > index 5b23d78..19a4abe 100644 > --- a/src/fw/paravirt.c > +++ b/src/fw/paravirt.c > @@ -25,6 +25,7 @@ > #include "x86.h" // cpuid > #include "xen.h" // xen_biostable_setup > #include "stacks.h" // yield > +#include "std/acpi.h" > > // Amount of continuous ram under 4Gig > u32 RamSize; > @@ -147,6 +148,46 @@ static void msr_feature_control_setup(void) > wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits); > } > > +static void > +build_compatibility_rsdt(void) > +{ > + if (RsdpAddr->rsdt_physical_address) > + return; > + > + u64 xsdt_addr = RsdpAddr->xsdt_physical_address; > + if (xsdt_addr & ~0xffffffffULL) > + return; > + > + struct xsdt_descriptor_rev1 *xsdt = (void*)(u32)xsdt_addr; > + void *end = (void*)xsdt + xsdt->length; > + struct rsdt_descriptor_rev1 *rsdt; > + int rsdt_size = offsetof(struct rsdt_descriptor_rev1, > table_offset_entry[0]); > + int i; > + for (i=0; (void*)&xsdt->table_offset_entry[i] < end; i++) { > + u64 tbl_addr = xsdt->table_offset_entry[i]; > + if (!tbl_addr || (tbl_addr & ~0xffffffffULL)) > + continue; > + rsdt_size += 4; > + } > + > + rsdt = malloc_high(rsdt_size); > + RsdpAddr->rsdt_physical_address = (u32)rsdt; Modifying the RSDP like this invalidates both its checksums, so they need to be adjusted. Something like this: RsdpAddr->checksum -= checksum(RsdpAddr, offsetof(struct rsdp_descriptor, length)); RsdpAddr->extended_checksum -= checksum(RsdpAddr, sizeof(struct rsdp_descriptor)); > + > + memcpy(rsdt, xsdt, sizeof(struct acpi_table_header)); > + rsdt->signature = RSDT_SIGNATURE; > + rsdt->length = rsdt_size; > + rsdt->revision = 1; > + int j; > + for (i=j=0; (void*)&xsdt->table_offset_entry[i] < end; i++) { > + u64 tbl_addr = xsdt->table_offset_entry[i]; > + if (!tbl_addr || (tbl_addr & ~0xffffffffULL)) > + continue; > + rsdt->table_offset_entry[j++] = (u32)tbl_addr; > + } > + > + rsdt->checksum -= checksum(rsdt, rsdt_size); > +} > + > void > qemu_platform_setup(void) > { > @@ -186,8 +227,10 @@ qemu_platform_setup(void) > > RsdpAddr = find_acpi_rsdp(); > > - if (RsdpAddr) > + if (RsdpAddr) { > + build_compatibility_rsdt(); > return; > + } > > /* If present, loader should have installed an RSDP. > * Not installed? We might still be able to continue > diff --git a/src/std/acpi.h b/src/std/acpi.h > index c2ea707..caf5e7e 100644 > --- a/src/std/acpi.h > +++ b/src/std/acpi.h > @@ -133,6 +133,17 @@ struct rsdt_descriptor_rev1 > } PACKED; > > /* > + * ACPI 2.0 Root System Description Table (RSDT) > + */ Comment should be "ACPI 2.0 Extended System Description Table (XSDT)" > +#define XSDT_SIGNATURE 0x54445358 // RSDT The signature is "XSDT", that works out to 0x58445358 > +struct xsdt_descriptor_rev1 > +{ > + ACPI_TABLE_HEADER_DEF /* ACPI common table header */ > + u64 table_offset_entry[0]; /* Array of pointers to other */ > + /* ACPI tables */ > +} PACKED; > + > +/* > * ACPI 1.0 Firmware ACPI Control Structure (FACS) > */ > #define FACS_SIGNATURE 0x53434146 // FACS > -- > 2.13.3 > > > From 46c7a296d27bd487cfd89a40973b1e61426dfbd0 Mon Sep 17 00:00:00 2001 > From: Paolo Bonzini <pbonz...@redhat.com> > Date: Tue, 25 Jul 2017 18:59:20 +0200 > Subject: [PATCH 2/2] seabios: create v1 FADT in compatibility RSDT > > This patch presents a v1 FADT inside the compatibility RSDT, so > that old operating systems are not broken. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > src/fw/paravirt.c | 18 +++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c > index 19a4abe..a9b9e80 100644 > --- a/src/fw/paravirt.c > +++ b/src/fw/paravirt.c > @@ -148,6 +148,18 @@ static void msr_feature_control_setup(void) > wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits); > } > > +static void* > +build_rev1_fadt(struct fadt_descriptor_rev1 *fadt_v3) > +{ > + struct fadt_descriptor_rev1 *fadt_v1 = malloc_high(sizeof *fadt_v1); > + > + memcpy(fadt_v1, fadt_v3, sizeof *fadt_v1); > + fadt_v1->length = sizeof *fadt_v1; > + fadt_v1->revision = 1; We should mask out the flags bits that are reserved in ACPI 1.0 as they refer to fields not present in this revision's FADT. e.g. // upper 23 bits reserved in rev1 FADT fadt_v1->flags &= 0x1ff; > + fadt_v1->checksum -= checksum(fadt_v1, fadt_v1->length); > + return fadt_v1; > +} > + > static void > build_compatibility_rsdt(void) > { > @@ -182,6 +194,12 @@ build_compatibility_rsdt(void) > u64 tbl_addr = xsdt->table_offset_entry[i]; > if (!tbl_addr || (tbl_addr & ~0xffffffffULL)) > continue; > + struct acpi_table_header *tbl = (void*)(u32)tbl_addr; > + /* The RSDT should only have an ACPI 1.0 FADT according to > + * the spec. > + */ > + if (tbl->signature == FACP_SIGNATURE && tbl->revision > 1) > + tbl_addr = (u32)build_rev1_fadt((void *)tbl); > rsdt->table_offset_entry[j++] = (u32)tbl_addr; > } > > -- > 2.13.3 > > With these changes: Reviewed-by: Phil Dennis-Jordan <p...@philjordan.eu>