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
