On 23 January 2017 at 12:12, Igor Mammedov <imamm...@redhat.com> wrote: >> For reference, my approach to filling out the Xdsdt/Xfacs fields in >> build_fadt() is essentially the same as for the 32-bit variants from >> rev1: >> >> unsigned xfacs_offset = (char *)&fadt->Xfacs - table_data->data; >> bios_linker_loader_add_pointer(linker, >> ACPI_BUILD_TABLE_FILE, xfacs_offset, sizeof(fadt->Xfacs), >> ACPI_BUILD_TABLE_FILE, facs_tbl_offset); >> >> Suggestions welcome. > You are not supposed to fill in the same values in X_FOO as in FOO. > Spec says that only either one of above should be set.
Thanks for clarifying that. I'd been looking at the ACPI 2.0 spec, which isn't clear on this point - the 5.0 one unambiguously agrees with you. I've tested the FADT change through again. Leaving Xdsdt and Xfacs as 0 in the Rev3 does indeed allow macOS to boot, as previously reported. Rebooting also works, which is the whole point of the patch. I'm still no further with getting Windows 10 past the "ACPI BIOS ERROR" bluescreen, however, even after some more experimenting with the various fields. I've tried a Rev5 FADT too, the Win10 bootloader just hangs in this cace, I don't even get a bluescreen. macOS is still happy. I've dumped the ACPI tables that OVMF ultimately exports with an EFI shell utility, and it all looks ok to me there; I also added various debug output to OVMF's ACPI code, and again everything looks pretty sane there. I guess my problem is I don't know what I'm looking for as I haven't found a way for Windows to tell me exactly what it doesn't like. I'm basically stumped on the Windows 10 issue, but I'd still like to fix macOS guest rebooting. Would a patch be acceptable that introduces a Rev3 FADT option? (or Rev5 - this'll be much less code as the struct already exists) If so, what nature should the option take? For reference, my latest FADT Rev3 prototype patch follows, in case anyone can spot some obvious problem. (I've kept this as compact as possible as opposed to optimising code quality for upstreaming.) The Rev5 FADT is identical except it uses sizeof(*fadt) and of course 5 as the revision. --- hw/i386/acpi-build.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 1c928ab..19dde8b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -271,7 +271,7 @@ build_facs(GArray *table_data, BIOSLinker *linker) } /* Load chipset information in FADT */ -static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) +static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo *pm) { fadt->model = 1; fadt->reserved1 = 0; @@ -296,6 +296,11 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) (1 << ACPI_FADT_F_SLP_BUTTON) | (1 << ACPI_FADT_F_RTC_S4)); fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK); + fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP); + fadt->reset_value = 0xf; + fadt->reset_register.space_id = 1; + fadt->reset_register.bit_width = 8; + fadt->reset_register.address = ICH9_RST_CNT_IOPORT; /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs * For more than 8 CPUs, "Clustered Logical" mode has to be used */ @@ -303,6 +308,15 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL); } fadt->century = RTC_CENTURY; + + fadt->xpm1a_event_block.address = cpu_to_le64(pm->io_base); + fadt->xpm1a_event_block.space_id = 1; + fadt->xpm1a_control_block.address = cpu_to_le64(pm->io_base + 0x04); + fadt->xpm1a_control_block.space_id = 1; + fadt->xpm_timer_block.address = cpu_to_le64(pm->io_base + 0x08); + fadt->xpm_timer_block.space_id = 1; + fadt->xgpe0_block.address = cpu_to_le64(pm->gpe0_blk); + fadt->xgpe0_block.space_id = 1; } @@ -312,7 +326,7 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm, unsigned facs_tbl_offset, unsigned dsdt_tbl_offset, const char *oem_id, const char *oem_table_id) { - AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt)); + AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, 244); unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data; unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data; @@ -328,7 +342,7 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm, ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset); build_header(linker, table_data, - (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id); + (void *)fadt, "FACP", 244, 3, oem_id, oem_table_id); } void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, --