Yes. Regards, Ray
-----Original Message----- From: Kinney, Michael D Sent: Thursday, December 10, 2015 9:33 AM To: Ni, Ruiyu <ruiyu...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Kinney, Michael D <michael.d.kin...@intel.com> Cc: Paolo Bonzini <pbonz...@redhat.com>; Scott Duplichan <sc...@notabs.org>; 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 Ray, Do you prefer PCD solution? Mike > -----Original Message----- > From: Ni, Ruiyu > Sent: Wednesday, December 9, 2015 4:45 PM > To: Kinney, Michael D <michael.d.kin...@intel.com>; Laszlo Ersek > <ler...@redhat.com> > Cc: Paolo Bonzini <pbonz...@redhat.com>; Scott Duplichan <sc...@notabs.org>; > 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 > > Mike, > The UEFI Spec doesn't require implementation to call > InstallConfigurationTable() every time the ACPI table is updated through > EFI_ACPI_PROTOCOL.InstallAcpiTable(). > I checked the implementation: > AcpiSupportDxe in IntelFrameworkModulePkg only calls > IntallConfigurationTable() in the first time InstallAcpiTable() is called. > AcpiTableDxe in MdeModulePkg calls InstallConfigurationTable() every time > InstallAcpiTable() is called. > > So we cannot depend on the event notification for EFI_ACPI_TABLE_GUID. > > PI Spec defines a ACPI_SDT protocol (produced by AcpiTableDxe in > MdeModulePkg) which supports table installation notification. But > it's an optional protocol and > not every platform includes AcpiTableDxe to produce this protocol. > Per my understanding, only EFI_ACPI_PROTOCOL is mandatory. > > Regards, > Ray > > > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, December 10, 2015 3:49 AM > To: Ni, Ruiyu <ruiyu...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Kinney, > Michael D <michael.d.kin...@intel.com> > Cc: Paolo Bonzini <pbonz...@redhat.com>; Scott Duplichan <sc...@notabs.org>; > 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 > > Ray, > > The UEFI Specification 2.5 p. 136 - CreateEventEx() requires an event group > to be signaled every time a configuration table is installed > or updated. > > The RTC driver could create an event notification for the EFI_ACPI_TABLE_GUID > and cache FADT field in notification function. > > Mike > > > -----Original Message----- > > From: Ni, Ruiyu > > Sent: Wednesday, December 9, 2015 10:48 AM > > To: Laszlo Ersek <ler...@redhat.com> > > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Paolo Bonzini > > <pbonz...@redhat.com>; Scott Duplichan <sc...@notabs.org>; > > 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 > > > > Laszlo, > > I agree with your concerns that the ACPI table can be modified at anytime > > before firmware hands off control to OS, after all it's a > > buffer reported by system configuration table. So in extreme cases, a > > platform may firstly set FADT.century to 0x80 but change it to > > 0x32 later. If the SetTime isn't called later, century won't be saved to > > CMOS. Even RTC driver hooks the ExitBootServices event to > > update CMOS, the FADT.century may be changed after RTC's callback. > > > > So how about use a PCD named PcdCmosCenturyLocation type UINT8, and the PCD > > value should less then 0x80 otherwise assertion > in > > RTC driver happens? > > It's platform 's responsibility to convert 0 to 0x80 and save in ACPI table > > if it's a win-only platform. For a OS neutral platform, non- > zero > > PCD should be always chosen. > > > > > > > > Thanks, > > Ray > > > > > 在 2015年12月10日，02:07，Laszlo Ersek <ler...@redhat.com> 写道： > > > > > >> On 12/09/15 18:55, Kinney, Michael D wrote: > > >> I think we need to evaluate both methods discussed here: > > >> > > >> 1) RTC driver use info from FADT in SetTime() when FADT is available. > > >> Add event notification to RTC driver to capture FADT info > > before ExitBootServices(). > > >> 2) Use PCD in RTC driver and in ACPI FADT table. > > >> > > >> Advantage of (1) is that it is compatible with existing ACPI Table > > >> implementations. > > > > > > I agree that (1) is compatible with existing *valid* ACPI table > > > implementations. > > > > > > I can't resist mentioning though that I've seen platform firmware "in > > > the wild" that messed with ACPI tables in the exit-boot-services > > > callback. Obviously this is completely broken: installing new tables > > > involves memory allocation and therefore messes with the memory map. > > > > > > Still, a technically valid driver that is nonetheless too clever for its > > > own good might poke data into existent ACPI tables (without memory > > > allocation or release) at exit-boot-services, derived from other data in > > > the system that *it* expects to become available asynchronously, > > > sometime before exit-boot-services. (Yes, I've seen this happen.) > > > > > > Given that the order of callbacks is unspecified, I generally try to > > > steer clear from inter-module dependencies in any exit-boot-services > > > callback; I believe that any driver should use that callback only for > > > re-setting its internal (software or hardware) state. (Without touching > > > the memory allocation services, of course). > > > > > > I think that the PCD is more robust, and in case a platform's ACPI > > > driver is *not* updated to consider it, then the situation is still no > > > worse than with Ray's current patch, in this thread. As I understand, > > > the PCD can be given a default value that would trigger the behavior > > > proposed in this patch. > > > > > > Thanks > > > Laszlo > > > > > >> > > >> Mike > > >> > > >> > > >>> -----Original Message----- > > >>> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > > >>> Sent: Wednesday, December 9, 2015 9:40 AM > > >>> To: Laszlo Ersek <ler...@redhat.com>; 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 18:37, Laszlo Ersek wrote: > > >>>> - A DXE driver that runs before *both* the ACPI platform DXE driver, > > >>>> and > > >>>> this runtime DXE driver -- to be ordered by any means necessary --, > > >>>> *or* > > >>>> a PEIM, sets a dynamic PCD that keys off *both* the ACPI platform DXE > > >>>> driver and this runtime DXE driver. I think this is the best approach. > > >>> > > >>> Yes, replacing RTC_ADDRESS_CENTURY with a PCD sounds like a very good > > >>> idea. > > > > > > _______________________________________________ > > > 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
