On 25/07/2017 23:25, Phil Dennis-Jordan wrote:
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)
Thanks Phil! You unwittingly tested the compatibility path on all these OSes, since my QEMU patch forgot to setup rsdp->length, rsdp->revision and the extended checksum. However, I've now tested Windows XP, Linux w/SeaBIOS, Linux w/patched SeaBIOS and Linux w/OVMF.
I've now found out that edk2 contains similar logic. It uses a PCD (a compile-time flag essentially) to choose between ACPI >= 2.0 tables or ACPI 1.0-compatible tables. In the latter case, edk2 takes care of producing a v1 FADT if needed (similar to this patch) and linking the RSDT to it; otherwise it keeps whatever FADT was provided by platform code and produces an XSDT. So I'm promoting the SeaBIOS code from "hack" to "right thing to do". Though then I cannot brag about Kevin's nice words, too bad. :)
Paolo
On Tue, Jul 25, 2017 at 7:10 PM, Paolo Bonzini pbonzini@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 pbonzini@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 pbonzini@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 pbonzini@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 pbonzini@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 phil@philjordan.eu
Digressing:
On 07/26/17 10:53, Paolo Bonzini wrote:
On 25/07/2017 23:25, Phil Dennis-Jordan wrote:
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)
Thanks Phil! You unwittingly tested the compatibility path on all these OSes, since my QEMU patch forgot to setup rsdp->length, rsdp->revision and the extended checksum. However, I've now tested Windows XP, Linux w/SeaBIOS, Linux w/patched SeaBIOS and Linux w/OVMF.
I've now found out that edk2 contains similar logic. It uses a PCD (a compile-time flag essentially) to choose between ACPI >= 2.0 tables or ACPI 1.0-compatible tables. In the latter case, edk2 takes care of producing a v1 FADT if needed (similar to this patch) and linking the RSDT to it; otherwise it keeps whatever FADT was provided by platform code and produces an XSDT.
Not exactly; the PCD controls whether the EFI_ACPI_TABLE_PROTOCOL will expose an RSDT, an XSDT, or both (with matching contents). The FADT always comes from the specific edk2 platform (i.e., OVMF client code), and it is not translated in any way, regardless of the PCD value.
From "MdeModulePkg/MdeModulePkg.dec":
## Indicates which ACPI versions are targeted by the ACPI tables exposed to the OS # These values are aligned with the definitions in MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h # BIT 1 - EFI_ACPI_TABLE_VERSION_1_0B.<BR> # BIT 2 - EFI_ACPI_TABLE_VERSION_2_0.<BR> # BIT 3 - EFI_ACPI_TABLE_VERSION_3_0.<BR> # BIT 4 - EFI_ACPI_TABLE_VERSION_4_0.<BR> # BIT 5 - EFI_ACPI_TABLE_VERSION_5_0.<BR> # @Prompt Exposed ACPI table versions. gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x3E|UINT32|0x0001004c
The expectation is that the specific edk2 platform overrides this PCD at build time (if necessary), and then goes on (at boot time) to install ACPI tables -- using EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() -- that actually match the PCD setting.
From the "MdeModulePkg/Universal/Acpi/AcpiTableDxe/" driver's POV (that
is, from the EFI_ACPI_TABLE_PROTOCOL implementation's POV), the platform controls *both* the PCD and the actually installed tables like the FADT, so EFI_ACPI_TABLE_PROTOCOL expects the platform to make these consistent.
The tiny little problem is that the PCD is a build-time flag, but QEMU provides the FADT (and friends) at boot time, dynamically, in a format that is essentially opaque to OVMF. So OVMF is sticking with the default PCD (see above), resulting in both RSDT and XSDT root tables, regardless of the contents of the FADT and friends.
A somewhat (but not too much) similar situation is with the SMBIOS tables. The tables are composed / exported by QEMU over fw_cfg, and OVMF / AAVMF have to set some version-like PCDs that match the content: - PcdSmbiosDocRev - PcdSmbiosVersion
We do some ugly hacks in OVMF to ensure that these PCDs are set "in time", before the generic "MdeModulePkg/Universal/SmbiosDxe" -- providing EFI_SMBIOS_PROTOCOL -- starts up and consumes the PCDs. Namely, we have "OvmfPkg/Library/SmbiosVersionLib" which sets these PCDs based on fw_cfg, and we link this library via NULL class resolution into "MdeModulePkg/Universal/SmbiosDxe". So the PCDs will be set up just before EFI_SMBIOS_PROTOCOL is initialized and provided. In turn, "OvmfPkg/SmbiosPlatformDxe", which actually calls EFI_SMBIOS_PROTOCOL.Add() on the tables provided by QEMU, has a depex on EFI_SMBIOS_PROTOCOL -- first, this depex ensures that EFI_SMBIOS_PROTOCOL can be used by "OvmfPkg/SmbiosPlatformDxe", but second, the depex *also* ensures that the PCDs will have been set correctly by the time "OvmfPkg/SmbiosPlatformDxe" calls EFI_SMBIOS_PROTOCOL.Add() for the first time.
You might ask why we don't do the same in the ACPI case (i.e., for PcdAcpiExposedTableVersions). It's due to the following differences:
- (less importantly,) "MdeModulePkg.dec" allows platforms to pick "dynamic" for PcdSmbiosDocRev and PcdSmbiosVersion, not just "fixed at build". IOW, MdeModulePkg already expects platforms to set the SMBIOS version PCDs dynamically, if those platforms can ensure the setting occurs "early enough".
- (more importantly,) the information needed by OVMF, for setting the SMBIOS version PCDs in "OvmfPkg/Library/SmbiosVersionLib", is readily available for parsing from the separate, dedicated fw_cfg file called "etc/smbios/smbios-anchor". In fact, OVMF doesn't use this file for anything else than grabbing the versions for the PCDs. The actual "anchor" table (the smbios entry point) is produced by the EFI_SMBIOS_PROTOCOL implementation in "MdeModulePkg/Universal/SmbiosDxe", which consumes the version PCDs.
(The SMBIOS tables themselves are provided in another fw_cfg file, called "etc/smbios/smbios-tables". Incidentally, the structure of that file is also way simpler than the ACPI linker/loader blobs'; OVMF can pick it apart with a simple loop and pass the individual SMBIOS tables to EFI_SMBIOS_PROTOCOL.Add().)
Anyway, I'm calling this email a digression because it isn't really related to the FADT content that is exposed to the OS. I just wanted to clarify "PcdAcpiExposedTableVersions", and that there isn't any "deep" FADT translation in edk2, to my knowledge.
Thanks Laszlo
On 26/07/2017 13:42, Laszlo Ersek wrote:
Not exactly; the PCD controls whether the EFI_ACPI_TABLE_PROTOCOL will expose an RSDT, an XSDT, or both (with matching contents).
You're right that the code does not produce a v1 FADT, I mis-skimmed the awful code of AcpiTableDxe. Though the intentions seems to be there in the UEFI spec, because UEFI has different GUIDs for ACPI 1.0 and 2.0+ RSDPs---and they need not point to the same tables, even though the ACPI 1.0 RSDP is a subset of the 2.0+ one.
AcpiTableDxe's data structures have an "Rsdp1" field (pointing to "Rsdt1" and from there to "Fadt1") and an "Rsdp3" field (pointing to "Xsdt" and optionally "Rsdt3", and from both to "Fadt3"). However:
* Fadt1 and Fadt3 have exactly the same content.
* Rsdt3 doesn't point to Fadt1.
It should be easy to make "Fadt1" a v1 table instead of copying the same contents to Fadt1 and Fadt3, and to make Rsdt3 point to Fadt1. The CSM would just work if edk2 did this; until then Windows 2000 over SeaBIOS over OVMF remains broken, but I guess that's not that much of an issue.
Anyway, once you take CSM into account, IMHO a firmware solution becomes preferrable to Kevin's proposals to add a etc/table-loader-legacy (or similar) file in fw_cfg.
Paolo
The FADT always comes from the specific edk2 platform (i.e., OVMF client code), and it is not translated in any way, regardless of the PCD value.
From "MdeModulePkg/MdeModulePkg.dec":
## Indicates which ACPI versions are targeted by the ACPI tables exposed to the OS # These values are aligned with the definitions in MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h # BIT 1 - EFI_ACPI_TABLE_VERSION_1_0B.<BR> # BIT 2 - EFI_ACPI_TABLE_VERSION_2_0.<BR> # BIT 3 - EFI_ACPI_TABLE_VERSION_3_0.<BR> # BIT 4 - EFI_ACPI_TABLE_VERSION_4_0.<BR> # BIT 5 - EFI_ACPI_TABLE_VERSION_5_0.<BR> # @Prompt Exposed ACPI table versions. gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x3E|UINT32|0x0001004c
The expectation is that the specific edk2 platform overrides this PCD at build time (if necessary), and then goes on (at boot time) to install ACPI tables -- using EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() -- that actually match the PCD setting.