From patchwork Thu Jan 26 13:43:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Dennis-Jordan X-Patchwork-Id: 720175 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3v8NSC0gvgz9t8j for ; Fri, 27 Jan 2017 00:44:11 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=philjordan-eu.20150623.gappssmtp.com header.i=@philjordan-eu.20150623.gappssmtp.com header.b="miv9qgqR"; dkim-atps=neutral Received: from localhost ([::1]:38706 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cWkLQ-0004Ld-PF for incoming@patchwork.ozlabs.org; Thu, 26 Jan 2017 08:44:08 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36627) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cWkKl-0003xB-By for qemu-devel@nongnu.org; Thu, 26 Jan 2017 08:43:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cWkKk-0003zQ-7D for qemu-devel@nongnu.org; Thu, 26 Jan 2017 08:43:27 -0500 Received: from mail-ot0-x243.google.com ([2607:f8b0:4003:c0f::243]:36707) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cWkKj-0003zM-UN for qemu-devel@nongnu.org; Thu, 26 Jan 2017 08:43:26 -0500 Received: by mail-ot0-x243.google.com with SMTP id 36so27399319otx.3 for ; Thu, 26 Jan 2017 05:43:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philjordan-eu.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=MLHKl/nukhNHe7P/5C41ST75kBxQ350WLchQSqs5bbo=; b=miv9qgqRnIgdKoKPWFFIohBTsaayGLV+a9jfKJDF0ZETTXHpy7D0X78336FHn0QkBF CT3ksSzDlyJ8odJ/tty5A0+bNlZdvHSXlRyTA1WgM/6iXsa6gln6D1WRI4+P+Nkjr7LD 1AOA3oIZ0HARTSigYdMFE3DGadXoMH7ecn7xIy4KsWWVRowfGU6gnyCri5iDH5CX8ye8 0fluebmLfUIHss1ejd8sor3gWDN0GkOQW9cj13uCoqOGMapYEMmNzXuGhTwtVf8VjAOV YmzeMu9S/xMpl6U9+MWP0KQ2Dk7B8JkncRrpww4E9gZGOvWdHwcj/vHLLBRTWQ8S/rJR BSBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=MLHKl/nukhNHe7P/5C41ST75kBxQ350WLchQSqs5bbo=; b=VDcUa/P1TGIqbpK+NNdR6mYeJ4EbID1Pq/zF/h66hFihI5kGpDyQSjzQnnL6QhsQ7d Es/pjWZqlZKpgPpy6O5/byxYlYwZdbJNT0koQQdrHJEh6Lm9ngLCnBV343uh1paaPCl4 +ryKmmICC9lS6IZq2QZ/EVMxgGTkO9gG05sJQJBM8U39XGdWosbfTOAp2Sqr4jEZb45M LmKo5IryO4dF1PKedEXTqlP+IDCnWRKk6GT+Jsb6tTAnI3HAoZp3l2ZRVX8LErKuTKXO h+ZdII+TTD4s+tNpZ5EDjX/6UYptMcN6dCwbEvmZQBBgHJq00xOFCxZCFBim401CORdM gVlw== X-Gm-Message-State: AIkVDXJaZiSHsCUaq7ogpmw+fnONJAjT/OsA7z2w8DRwNjlDODC3i9bVBpmazAgEMSMBV4YoZXpsp6/JKU4yGg== X-Received: by 10.157.15.144 with SMTP id d16mr1063880otd.169.1485438204963; Thu, 26 Jan 2017 05:43:24 -0800 (PST) MIME-Version: 1.0 Received: by 10.157.19.53 with HTTP; Thu, 26 Jan 2017 05:43:04 -0800 (PST) In-Reply-To: <20170123121204.547e6415@nial.brq.redhat.com> References: <1484739954-86833-1-git-send-email-phil@philjordan.eu> <20170118175335-mutt-send-email-mst@kernel.org> <20170123121204.547e6415@nial.brq.redhat.com> From: Phil Dennis-Jordan Date: Thu, 26 Jan 2017 14:43:04 +0100 Message-ID: To: Igor Mammedov X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:4003:c0f::243 Subject: Re: [Qemu-devel] [PATCH RFC] acpi: add reset register to fadt X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Richard Henderson , qemu-devel@nongnu.org, Eduardo Habkost , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On 23 January 2017 at 12:12, Igor Mammedov 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(-) */ @@ -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, -- 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