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
