From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58542) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHDxv-0002Rg-FJ for qemu-devel@nongnu.org; Thu, 07 Jan 2016 12:03:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHDxu-0004Q8-Cu for qemu-devel@nongnu.org; Thu, 07 Jan 2016 12:03:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45111) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHDxu-0004Pr-6J for qemu-devel@nongnu.org; Thu, 07 Jan 2016 12:03:10 -0500 References: <1449768334-12782-1-git-send-email-lersek@redhat.com> <20151210195336.459c0c64@igors-macbook-pro.local> <568AA993.2090006@redhat.com> <568E38F2.1010507@redhat.com> <20160107132159-mutt-send-email-mst@redhat.com> <568E4ADF.4020808@redhat.com> From: Laszlo Ersek Message-ID: <568E9A4A.7040602@redhat.com> Date: Thu, 7 Jan 2016 18:03:06 +0100 MIME-Version: 1.0 In-Reply-To: <568E4ADF.4020808@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 , "Michael S. Tsirkin" Cc: Ruiyu Ni , Igor Mammedov , Richard Henderson , Eduardo Habkost , qemu-devel@nongnu.org On 01/07/16 12:24, Paolo Bonzini wrote: > > > On 07/01/2016 12:22, Michael S. Tsirkin wrote: >> 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? > > Sure! > > Reviewed-by: Paolo Bonzini > > Paolo > Thanks guys. Laszlo