Star: Thanks for you feedback. I will update the patch and push it.
>-----Original Message----- >From: Zeng, Star >Sent: Friday, January 26, 2018 1:57 PM >To: Gao, Liming <liming....@intel.com>; edk2-devel@lists.01.org >Cc: Zeng, Star <star.z...@intel.com> >Subject: RE: [edk2] [PATCH v2] PcAtChipsetPkg: Add PeiAcpiTimerLib to save >PerformanceCounterFrequency in HOB > >Hi Liming, > >Reviewed-by: Star Zeng <star.z...@intel.com> > >Two minor comments. Just for your information. >1. Notice the title, it may be too long. >2. In PeiAcpiTimerLib.c, you may need to add ASSERT >(PerformanceCounterFrequency != NULL) after BuildGuidHob. >And you can move " PerformanceCounterFrequency = >(UINT64*)GET_GUID_HOB_DATA (GuidHob); " to the else condition, then >only one "return" is needed. > >+ PerformanceCounterFrequency = NULL; >+ GuidHob = GetFirstGuidHob (&mFrequencyHobGuid); >+ if (GuidHob == NULL) { >+ PerformanceCounterFrequency = >(UINT64*)BuildGuidHob(&mFrequencyHobGuid, sizeof >(*PerformanceCounterFrequency)); >+ *PerformanceCounterFrequency = InternalCalculateTscFrequency (); >+ return *PerformanceCounterFrequency; >+ } >+ PerformanceCounterFrequency = (UINT64*)GET_GUID_HOB_DATA >(GuidHob); >+ >+ return *PerformanceCounterFrequency; >+} > > >Thanks, >Star >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Liming Gao >Sent: Tuesday, January 23, 2018 10:24 AM >To: edk2-devel@lists.01.org >Cc: Zeng, Star <star.z...@intel.com> >Subject: [edk2] [PATCH v2] PcAtChipsetPkg: Add PeiAcpiTimerLib to save >PerformanceCounterFrequency in HOB > >In V2: >1) Update PeiAcpiTimerLib base name to PeiAcpiTimerLib >2) Update PeiAcpiTimerLib to add the missing constructor to enable ACPI IO >space >3) Update DxeAcpiTimerLib to cache frequency in constructor. > >PeiAcpiTimerLib caches PerformanceCounterFrequency in HOB, then Pei and >Dxe >AcpiTimerLib can share the same PerformanceCounterFrequency. > >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Liming Gao <liming....@intel.com> >Cc: Star Zeng <star.z...@intel.com> >--- > PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 4 +- > .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 57 +++++++++++++++++- > .../Library/AcpiTimerLib/DxeAcpiTimerLib.inf | 7 ++- > .../Library/AcpiTimerLib/PeiAcpiTimerLib.c | 68 >++++++++++++++++++++++ > .../Library/AcpiTimerLib/PeiAcpiTimerLib.inf | 56 ++++++++++++++++++ > .../Library/AcpiTimerLib/PeiAcpiTimerLib.uni | 23 ++++++++ > PcAtChipsetPkg/PcAtChipsetPkg.dsc | 4 +- > 7 files changed, 211 insertions(+), 8 deletions(-) > create mode 100644 PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.c > create mode 100644 >PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf > create mode 100644 >PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.uni > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c >b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c >index 792781a33f..2f3cb4bca4 100644 >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c >@@ -1,7 +1,7 @@ > /** @file > ACPI Timer implements one instance of Timer Library. > >- Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR> >+ Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD >License > which accompanies this distribution. The full text of the license may be >found at >@@ -21,6 +21,8 @@ > #include <Library/DebugLib.h> > #include <IndustryStandard/Acpi.h> > >+GUID mFrequencyHobGuid = { 0x3fca54f6, 0xe1a2, 0x4b20, { 0xbe, 0x76, >0x92, 0x6b, 0x4b, 0x48, 0xbf, 0xaa }}; >+ > /** > Internal function to retrieves the 64-bit frequency in Hz. > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c >index b141c680fb..67e18a1360 100644 >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c >@@ -12,9 +12,27 @@ > > **/ > >-#include <Base.h> >+#include <PiDxe.h> > #include <Library/TimerLib.h> > #include <Library/BaseLib.h> >+#include <Library/HobLib.h> >+ >+extern GUID mFrequencyHobGuid; >+ >+/** >+ The constructor function enables ACPI IO space. >+ >+ If ACPI I/O space not enabled, this function will enable it. >+ It will always return RETURN_SUCCESS. >+ >+ @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS. >+ >+**/ >+RETURN_STATUS >+EFIAPI >+AcpiTimerLibConstructor ( >+ VOID >+ ); > > /** > Calculate TSC frequency. >@@ -54,8 +72,41 @@ InternalGetPerformanceCounterFrequency ( > VOID > ) > { >- if (mPerformanceCounterFrequency == 0) { >+ return mPerformanceCounterFrequency; >+} >+ >+/** >+ The constructor function enables ACPI IO space, and caches >PerformanceCounterFrequency. >+ >+ @param ImageHandle The firmware allocated handle for the EFI image. >+ @param SystemTable A pointer to the EFI System Table. >+ >+ @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS. >+ >+**/ >+EFI_STATUS >+EFIAPI >+DxeAcpiTimerLibConstructor ( >+ IN EFI_HANDLE ImageHandle, >+ IN EFI_SYSTEM_TABLE *SystemTable >+ ) >+{ >+ EFI_HOB_GUID_TYPE *GuidHob; >+ >+ // >+ // Enable ACPI IO space. >+ // >+ AcpiTimerLibConstructor (); >+ >+ // >+ // Initialize PerformanceCounterFrequency >+ // >+ GuidHob = GetFirstGuidHob (&mFrequencyHobGuid); >+ if (GuidHob != NULL) { >+ mPerformanceCounterFrequency = *(UINT64*)GET_GUID_HOB_DATA >(GuidHob); >+ } else { > mPerformanceCounterFrequency = InternalCalculateTscFrequency (); > } >- return mPerformanceCounterFrequency; >+ >+ return EFI_SUCCESS; > } >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf >index 2c1cc7d33c..601041c801 100644 >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf >@@ -7,7 +7,7 @@ > # Note: The implementation uses the lower 24-bits of the ACPI timer and > # is compatible with both 24-bit and 32-bit ACPI timers. > # >-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR> >+# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD >License > # which accompanies this distribution. The full text of the license may be >found at >@@ -22,10 +22,10 @@ > INF_VERSION = 0x00010005 > BASE_NAME = DxeAcpiTimerLib > FILE_GUID = E624B98C-845A-4b94-9B50-B20475D552B9 >- MODULE_TYPE = BASE >+ MODULE_TYPE = DXE_DRIVER > VERSION_STRING = 1.0 > LIBRARY_CLASS = TimerLib|DXE_CORE DXE_DRIVER >DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER >SMM_CORE >- CONSTRUCTOR = AcpiTimerLibConstructor >+ CONSTRUCTOR = DxeAcpiTimerLibConstructor > MODULE_UNI_FILE = DxeAcpiTimerLib.uni > > [Sources] >@@ -42,6 +42,7 @@ > PciLib > IoLib > DebugLib >+ HobLib > > [Pcd] > gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBusNumber ## >CONSUMES >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.c >b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.c >new file mode 100644 >index 0000000000..a4ed6a4434 >--- /dev/null >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.c >@@ -0,0 +1,68 @@ >+/** @file >+ ACPI Timer implements one instance of Timer Library. >+ >+ Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> >+ This program and the accompanying materials >+ are licensed and made available under the terms and conditions of the BSD >License >+ which accompanies this distribution. The full text of the license may be >found at >+ http://opensource.org/licenses/bsd-license.php >+ >+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >BASIS, >+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >EXPRESS OR IMPLIED. >+ >+**/ >+ >+#include <PiPei.h> >+#include <Library/TimerLib.h> >+#include <Library/BaseLib.h> >+#include <Library/HobLib.h> >+ >+extern GUID mFrequencyHobGuid; >+ >+/** >+ Calculate TSC frequency. >+ >+ The TSC counting frequency is determined by comparing how far it counts >+ during a 101.4 us period as determined by the ACPI timer. >+ The ACPI timer is used because it counts at a known frequency. >+ The TSC is sampled, followed by waiting 363 counts of the ACPI timer, >+ or 101.4 us. The TSC is then sampled again. The difference multiplied by >+ 9861 is the TSC frequency. There will be a small error because of the >+ overhead of reading the ACPI timer. An attempt is made to determine and >+ compensate for this error. >+ >+ @return The number of TSC counts per second. >+ >+**/ >+UINT64 >+InternalCalculateTscFrequency ( >+ VOID >+ ); >+ >+/** >+ Internal function to retrieves the 64-bit frequency in Hz. >+ >+ Internal function to retrieves the 64-bit frequency in Hz. >+ >+ @return The frequency in Hz. >+ >+**/ >+UINT64 >+InternalGetPerformanceCounterFrequency ( >+ VOID >+ ) >+{ >+ UINT64 *PerformanceCounterFrequency; >+ EFI_HOB_GUID_TYPE *GuidHob; >+ >+ PerformanceCounterFrequency = NULL; >+ GuidHob = GetFirstGuidHob (&mFrequencyHobGuid); >+ if (GuidHob == NULL) { >+ PerformanceCounterFrequency = >(UINT64*)BuildGuidHob(&mFrequencyHobGuid, sizeof >(*PerformanceCounterFrequency)); >+ *PerformanceCounterFrequency = InternalCalculateTscFrequency (); >+ return *PerformanceCounterFrequency; >+ } >+ PerformanceCounterFrequency = (UINT64*)GET_GUID_HOB_DATA >(GuidHob); >+ >+ return *PerformanceCounterFrequency; >+} >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf >b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf >new file mode 100644 >index 0000000000..1d8c95a3ec >--- /dev/null >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf >@@ -0,0 +1,56 @@ >+## @file >+# PEI ACPI Timer Library >+# >+# Provides basic timer support using the ACPI timer hardware. The >performance >+# counter features are provided by the processors time stamp counter. >+# >+# Note: The implementation uses the lower 24-bits of the ACPI timer and >+# is compatible with both 24-bit and 32-bit ACPI timers. >+# >+# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR> >+# This program and the accompanying materials >+# are licensed and made available under the terms and conditions of the >BSD License >+# which accompanies this distribution. The full text of the license may be >found at >+# http://opensource.org/licenses/bsd-license.php >+# >+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >BASIS, >+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >EXPRESS OR IMPLIED. >+# >+## >+ >+[Defines] >+ INF_VERSION = 0x00010005 >+ BASE_NAME = PeiAcpiTimerLib >+ FILE_GUID = 3FCA54F6-E1A2-4B20-BE76-926B4B48BFAA >+ MODULE_TYPE = BASE >+ VERSION_STRING = 1.0 >+ LIBRARY_CLASS = TimerLib|PEI_CORE PEIM >+ CONSTRUCTOR = AcpiTimerLibConstructor >+ MODULE_UNI_FILE = PeiAcpiTimerLib.uni >+ >+[Sources] >+ AcpiTimerLib.c >+ PeiAcpiTimerLib.c >+ >+[Packages] >+ MdePkg/MdePkg.dec >+ PcAtChipsetPkg/PcAtChipsetPkg.dec >+ >+[LibraryClasses] >+ BaseLib >+ PcdLib >+ PciLib >+ IoLib >+ DebugLib >+ HobLib >+ >+[Pcd] >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBusNumber ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciDeviceNumber ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciFunctionNumber ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciEnableRegisterOffset ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoBarEnableMask ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBarRegisterOffset ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddress ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiPm1TmrOffset ## >CONSUMES >+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask ## >CONSUMES >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.uni >b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.uni >new file mode 100644 >index 0000000000..668161b14a >--- /dev/null >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.uni >@@ -0,0 +1,23 @@ >+// /** @file >+// PEI ACPI Timer Library >+// >+// Provides basic timer support using the ACPI timer hardware. The >performance >+// counter features are provided by the processors time stamp counter. >+// >+// Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> >+// >+// This program and the accompanying materials >+// are licensed and made available under the terms and conditions of the >BSD License >+// which accompanies this distribution. The full text of the license may be >found at >+// http://opensource.org/licenses/bsd-license.php >+// >+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >BASIS, >+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >EXPRESS OR IMPLIED. >+// >+// **/ >+ >+ >+#string STR_MODULE_ABSTRACT #language en-US "ACPI Timer >Library" >+ >+#string STR_MODULE_DESCRIPTION #language en-US "Provides basic >timer support using the ACPI timer hardware." >+ >diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dsc >b/PcAtChipsetPkg/PcAtChipsetPkg.dsc >index b740f00e63..2395e9cf26 100644 >--- a/PcAtChipsetPkg/PcAtChipsetPkg.dsc >+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dsc >@@ -1,7 +1,7 @@ > ## @file > # PC/AT Chipset Package > # >-# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR> >+# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR> > # > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD >License >@@ -46,6 +46,7 @@ > IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf > LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf > >ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseRe >portStatusCodeLibNull.inf >+ HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > [Components] > PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >@@ -57,6 +58,7 @@ > PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf > PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf > PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf >+ PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf > >PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeD >xe.inf > > [BuildOptions] >-- >2.11.0.windows.1 > >_______________________________________________ >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