Paolo, I agree SetTime() is not called in very many places. But since the SetTime() service is added to Runtime Services Table when the RTC driver runs, the logic in SetTime() must be implemented to handle case where SetTime() is called before ACPI Tables are published.
Mike > -----Original Message----- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Wednesday, December 9, 2015 8:42 AM > To: Kinney, Michael D <michael.d.kin...@intel.com>; Ni, Ruiyu > <ruiyu...@intel.com>; Scott Duplichan <sc...@notabs.org> > Cc: 'edk2-devel@lists.01.org' <edk2-de...@ml01.01.org>; Zeng, Star > <star.z...@intel.com> > Subject: Re: [edk2] [Patch] PcAtChipsetPkg/Rtc: Fix a UEFI Win7 boot hang > issue > > > > On 09/12/2015 17:33, Kinney, Michael D wrote: > > Paolo, > > > > The RTC driver runs and produces the runtime services before the ACPI > > tables are available so what you suggest is not always > possible. > > The only place where Ruiyu's patch actually uses RTC_ADDRESS_CENTURY is > in SetTime, and that is not used at any place other than > SetupBrowserDxe. The ACPI tables should be available by then, shouldn't > they? > > Paolo > > > Mike > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > >> Paolo Bonzini > >> Sent: Wednesday, December 9, 2015 3:50 AM > >> To: Ni, Ruiyu <ruiyu...@intel.com>; Scott Duplichan <sc...@notabs.org> > >> Cc: 'edk2-devel@lists.01.org' <edk2-de...@ml01.01.org>; Zeng, Star > >> <star.z...@intel.com> > >> Subject: Re: [edk2] [Patch] PcAtChipsetPkg/Rtc: Fix a UEFI Win7 boot hang > >> issue > >> > >> > >> > >> On 09/12/2015 12:16, Ni, Ruiyu wrote: > >>> Scott, I debugged the issue further and had the below findings: > >>> According to the ACPI spec 6.0 5.2.9 Fixed ACPI Description Table > >>> (FADT), the FADT.Century can be set to 0 indicating the RTC doesn't > >>> support to store century value. But the Win7 boot loader > >>> hal!HalpInitializeCmos() will firstly read and save the FADT.Century > >>> to a 4-byte location pointed by hal!_HalpCmosCenturyOffset. When the > >>> FADT.Century is 0, it will save 0x32 (default value) to that > >>> location. Later hal!HalpReadCmosTime() skips to read the century > >>> value from CMOS when BIT 7 of the value poited by > >>> hal!_HalpCmosCenturyOffset is set. So in order to tell Windows that > >>> RTC doesn't support to store century, we need to set the FADT.Century > >>> to 0x80 other than 0. In summary, if FADT.Century is 0, Win7 boot > >>> loader reads century from 0x32; if FADT.Century & BIT7 != 0, it > >>> doesn't read century from CMOS; otherwise, it reads century from CMOS > >>> index FADT.Century. > >>> > >>> But that caused another issue. Linux code strictly follows the ACPI > >>> spec which reads the century value from FADT.Century if it doesn't > >>> equal to zero. If we leave 0x80 in FADT.Century, Linux kernel will > >>> reads century from 0x80 location (actually from location 0 because > >>> CMOS address is 7-bit). Location 0 stores the seconds of the time > >>> which means Linux will read random century value. > >>> > >>> So do you agree if a platform needs to support booting both Windows > >>> and Linux, it had better to set FADT.Century to 0x32 and save correct > >>> century value (0x20) to CMOS address 0x32? > >> > >> That's fair enough, but you should not use RTC_ADDRESS_CENTURY > >> unconditionally in PcRtcSetTime. Instead you should read the FADT > >> yourself and use the FADT.Century value if it is non-zero. If it is > >> zero, I suppose writing the century to 0x32 is the only thing you can do. > >> > >> Paolo > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
