From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aH8eJ-0001J0-Fi for qemu-devel@nongnu.org; Thu, 07 Jan 2016 06:22:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aH8eG-0004oc-8V for qemu-devel@nongnu.org; Thu, 07 Jan 2016 06:22:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47765) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aH8eG-0004oY-0z for qemu-devel@nongnu.org; Thu, 07 Jan 2016 06:22:32 -0500 Date: Thu, 7 Jan 2016 13:22:27 +0200 From: "Michael S. Tsirkin" Message-ID: <20160107132159-mutt-send-email-mst@redhat.com> References: <1449768334-12782-1-git-send-email-lersek@redhat.com> <20151210195336.459c0c64@igors-macbook-pro.local> <568AA993.2090006@redhat.com> <568E38F2.1010507@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <568E38F2.1010507@redhat.com> Subject: Re: [Qemu-devel] [PATCH] hw/i386: fill in the CENTURY field of the FADT (FACP) ACPI table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Ruiyu Ni , Eduardo Habkost , qemu-devel@nongnu.org, Igor Mammedov , Laszlo Ersek , Richard Henderson On Thu, Jan 07, 2016 at 11:07:46AM +0100, Paolo Bonzini wrote: > > > On 04/01/2016 18:19, Laszlo Ersek wrote: > > On 12/10/15 19:53, Igor Mammedov wrote: > >> On Thu, 10 Dec 2015 18:25:34 +0100 > >> Laszlo Ersek wrote: > >> > >>> The ACPI specification (minimally versions 1.0b through 6.0) define > >>> the FADT.CENTURY field as: > >>> > >>> The RTC CMOS RAM index to the century of data value (hundred and > >>> thousand year decimals). If this field contains a zero, then the RTC > >>> centenary feature is not supported. If this field has a non-zero > >>> value, then this field contains an index into RTC RAM space that OSPM > >>> can use to program the centenary field. > >>> > >>> The x86 targets generate ACPI payload, emulate an RTC > >>> (CONFIG_MC146818RTC), and that RTC supports the "centenary > >>> feature" (see occurrences of RTC_CENTURY in cmos_ioport_write() and > >>> cmos_ioport_read() in "hw/timer/mc146818rtc.c".) > >>> > >>> However, FADT.CENTURY is left at zero currently: > >>> > >>> [06Ch 0108 1] RTC Century Index : 00 > >>> > >>> which -- according to analysis done by Ruiyu Ni at Intel -- should > >>> cause Linux and Windows 8+ to think the RTC centenary feature is > >>> unavailable, and cause Windows 7 to (incorrectly) assume that the > >>> offset to use is constant 0x32. (0x32 happens to be the right value > >>> on QEMU, but Windows 7 is wrong to assume anything at all). > >>> > >>> Exposing the right nonzero offset in FADT.CENTURY informs Linux and > >>> Windows 8+ about the right capabilities of the hardware, plus it > >>> retrofits our FADT to Windows 7's behavior. > >>> > >>> Regression tested with the following guests (all UEFI installs): > >>> - i386 Q35: Fedora 21 ("Fedlet" edition) > >>> - x86_64: > >>> - i440fx: > >>> - Fedora 21 > >>> - RHEL 6 and 7 > >>> - Windows 7 and 10 > >>> - Windows Server 2008 R2 and 2012 R2 > >>> - Q35: > >>> - Fedora 22 > >>> - Windows 8.1 > >>> > >>> Cc: "Michael S. Tsirkin" (supporter:ACPI/SMBIOS) > >>> Cc: Igor Mammedov (supporter:ACPI/SMBIOS) > >>> Cc: Paolo Bonzini (maintainer:X86) > >>> Cc: Richard Henderson (maintainer:X86) > >>> Cc: Eduardo Habkost (maintainer:X86) > >>> Cc: Ruiyu Ni > >>> Signed-off-by: Laszlo Ersek > >>> --- > >>> hw/i386/acpi-build.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >>> index 95e0c65..c5e6c4b 100644 > >>> --- a/hw/i386/acpi-build.c > >>> +++ b/hw/i386/acpi-build.c > >>> @@ -42,6 +42,7 @@ > >>> #include "sysemu/tpm.h" > >>> #include "hw/acpi/tpm.h" > >>> #include "sysemu/tpm_backend.h" > >>> +#include "hw/timer/mc146818rtc_regs.h" > >>> > >>> /* Supported chipsets: */ > >>> #include "hw/acpi/piix4.h" > >>> @@ -334,6 +335,7 @@ static void fadt_setup(AcpiFadtDescriptorRev1 > >>> *fadt, AcpiPmInfo *pm) if (max_cpus > 8) { > >>> fadt->flags |= cpu_to_le32(1 << > >>> ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL); } > >>> + fadt->century = RTC_CENTURY; > >>> } > >>> > >>> > >> > >> Reviewed-by: Igor Mammedov > >> > > > > Thanks. > > > > Can someone please pick up this patch? > > It should probably go in through Michael's tree, but I've queued it too > so that it isn't forgotten. > > Paolo Yes - thanks! I picked this up - should I add your Reviewed-by tag?