Thanks for providing the detailed commit messages. Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message----- > From: Lou, Yun <yun....@intel.com> > Sent: Wednesday, April 7, 2021 4:16 PM > To: devel@edk2.groups.io > Cc: Lou, Yun <yun....@intel.com>; Ni, Ray <ray...@intel.com>; Dong, Eric > <eric.d...@intel.com>; Laszlo Ersek > <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com> > Subject: [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib. > > From: Jason Lou <yun....@intel.com> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2832 > > 1. Remove PEI instance(PeiCpuTimerLib). > PeiCpuTimerLib is currently designed to save time by getting CPU TSC > frequency from Hob. BaseCpuTimerLib is designed to calculate TSC frequency > by using CPUID[15h] each time. > The time it takes to find CpuCrystalFrequencyHob (about 2000ns) is much > longer than it takes to calculate TSC frequency with CPUID[15h] (about > 450ns), which means using BaseCpuTimerLib to trigger a delay is more > accurate than using PeiCpuTimerLib, recommend to use BaseCpuTimerLib > instead of PeiCpuTimerLib. > > 2. Remove DXE instance(DxeCpuTimerLib). > DxeCpuTimerLib is designed to calculate TSC frequency with CPUID[15h] in > its constructor function, then save it in a global variable. For this > design, once the driver containing this instance is running, this > constructor function is called, it will take extra time to calculate TSC > frequency. > The time it takes to get TSC frequency from global variable is shorter > than it takes to calculate TSC frequency with CPUID[15h], but 450ns is a > short time, the impact on the platform is very limited. > In addition, in order to simplify the code, recommend to use > BaseCpuTimerLib instead of DxeCpuTimerLib. > > I did some experiments on one server platform and collected following data: > 1. Average time required to find CpuCrystalFrequencyHob: about 2000 ns. > 2. Average time required to find the last Hob: about 2700 ns. > 2. Average time required to calculate TSC frequency: about 450 ns. > > Reference code: > // > // Calculate average time required to find Hob. > // > DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - > GetFirstGuidHob (1000 cycles)\n")); > Ticks1 = AsmReadTsc(); > for (i = 0; i < 1000; i++) { > GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid); > } > Ticks2 = AsmReadTsc(); > > if (GuidHob == NULL) { > DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - CpuCrystalFrequencyHob can not > be found!\n")); > } else { > DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - Average time required to find > Hob = %d ns\n", \ > DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), > 1000000000), *CpuCrystalCounterFrequency, NULL), > 1000))); > } > > // > // Calculate average time required to calculate CPU frequency. > // > DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - > CpuidCoreClockCalculateTscFrequency (1000 > cycles)\n")); > Ticks1 = AsmReadTsc(); > for (i = 0; i < 1000; i++) { > Freq = CpuidCoreClockCalculateTscFrequency (); > } > Ticks2 = AsmReadTsc(); > DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - Average time required to > calculate TSC frequency = %d ns\n", \ > DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), > 1000000000), *CpuCrystalCounterFrequency, NULL), 1000))); > > Signed-off-by: Jason Lou <yun....@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > --- > UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c | 85 -------------------- > UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c | 58 ------------- > UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf | 37 --------- > UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni | 17 ---- > UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf | 36 --------- > UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni | 17 ---- > UefiCpuPkg/UefiCpuPkg.dsc | 2 - > 7 files changed, 252 deletions(-) > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c > b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c > deleted file mode 100644 > index 269e5a3e83..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c > +++ /dev/null > @@ -1,85 +0,0 @@ > -/** @file > > - CPUID Leaf 0x15 for Core Crystal Clock frequency instance of Timer Library. > > - > > - Copyright (c) 2019 Intel Corporation. All rights reserved.<BR> > > - SPDX-License-Identifier: BSD-2-Clause-Patent > > - > > -**/ > > - > > -#include <PiDxe.h> > > -#include <Library/TimerLib.h> > > -#include <Library/BaseLib.h> > > -#include <Library/HobLib.h> > > - > > -extern GUID mCpuCrystalFrequencyHobGuid; > > - > > -/** > > - CPUID Leaf 0x15 for Core Crystal Clock Frequency. > > - > > - The TSC counting frequency is determined by using CPUID leaf 0x15. > Frequency in MHz = Core XTAL frequency * EBX/EAX. > > - In newer flavors of the CPU, core xtal frequency is returned in ECX or 0 > if not supported. > > - @return The number of TSC counts per second. > > - > > -**/ > > -UINT64 > > -CpuidCoreClockCalculateTscFrequency ( > > - VOID > > - ); > > - > > -// > > -// Cached CPU Crystal counter frequency > > -// > > -UINT64 mCpuCrystalCounterFrequency = 0; > > - > > - > > -/** > > - 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 > > - ) > > -{ > > - return mCpuCrystalCounterFrequency; > > -} > > - > > -/** > > - The constructor function is to initialize CpuCrystalCounterFrequency. > > - > > - @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 > > -DxeCpuTimerLibConstructor ( > > - IN EFI_HANDLE ImageHandle, > > - IN EFI_SYSTEM_TABLE *SystemTable > > - ) > > -{ > > - EFI_HOB_GUID_TYPE *GuidHob; > > - > > - // > > - // Initialize CpuCrystalCounterFrequency > > - // > > - GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid); > > - if (GuidHob != NULL) { > > - mCpuCrystalCounterFrequency = *(UINT64*)GET_GUID_HOB_DATA (GuidHob); > > - } else { > > - mCpuCrystalCounterFrequency = CpuidCoreClockCalculateTscFrequency (); > > - } > > - > > - if (mCpuCrystalCounterFrequency == 0) { > > - return EFI_UNSUPPORTED; > > - } > > - > > - return EFI_SUCCESS; > > -} > > - > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c > b/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c > deleted file mode 100644 > index 91a7212056..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c > +++ /dev/null > @@ -1,58 +0,0 @@ > -/** @file > > - CPUID Leaf 0x15 for Core Crystal Clock frequency instance as PEI Timer > Library. > > - > > - Copyright (c) 2019 Intel Corporation. All rights reserved.<BR> > > - SPDX-License-Identifier: BSD-2-Clause-Patent > > - > > -**/ > > - > > -#include <PiPei.h> > > -#include <Library/TimerLib.h> > > -#include <Library/BaseLib.h> > > -#include <Library/HobLib.h> > > -#include <Library/DebugLib.h> > > - > > -extern GUID mCpuCrystalFrequencyHobGuid; > > - > > -/** > > - CPUID Leaf 0x15 for Core Crystal Clock Frequency. > > - > > - The TSC counting frequency is determined by using CPUID leaf 0x15. > Frequency in MHz = Core XTAL frequency * EBX/EAX. > > - In newer flavors of the CPU, core xtal frequency is returned in ECX or 0 > if not supported. > > - @return The number of TSC counts per second. > > - > > -**/ > > -UINT64 > > -CpuidCoreClockCalculateTscFrequency ( > > - 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 *CpuCrystalCounterFrequency; > > - EFI_HOB_GUID_TYPE *GuidHob; > > - > > - CpuCrystalCounterFrequency = NULL; > > - GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid); > > - if (GuidHob == NULL) { > > - CpuCrystalCounterFrequency = > (UINT64*)BuildGuidHob(&mCpuCrystalFrequencyHobGuid, sizeof > (*CpuCrystalCounterFrequency)); > > - ASSERT (CpuCrystalCounterFrequency != NULL); > > - *CpuCrystalCounterFrequency = CpuidCoreClockCalculateTscFrequency (); > > - } else { > > - CpuCrystalCounterFrequency = (UINT64*)GET_GUID_HOB_DATA (GuidHob); > > - } > > - > > - return *CpuCrystalCounterFrequency; > > -} > > - > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf > b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf > deleted file mode 100644 > index 6c83549c87..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf > +++ /dev/null > @@ -1,37 +0,0 @@ > -## @file > > -# DXE CPU Timer Library > > -# > > -# Provides basic timer support using CPUID Leaf 0x15 XTAL frequency. The > performance > > -# counter features are provided by the processors time stamp counter. > > -# > > -# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > -# SPDX-License-Identifier: BSD-2-Clause-Patent > > -# > > -## > > - > > -[Defines] > > - INF_VERSION = 0x00010005 > > - BASE_NAME = DxeCpuTimerLib > > - FILE_GUID = F22CC0DA-E7DB-4E4D-ABE2-A608188233A2 > > - 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 = DxeCpuTimerLibConstructor > > - MODULE_UNI_FILE = DxeCpuTimerLib.uni > > - > > -[Sources] > > - CpuTimerLib.c > > - DxeCpuTimerLib.c > > - > > -[Packages] > > - MdePkg/MdePkg.dec > > - UefiCpuPkg/UefiCpuPkg.dec > > - > > -[LibraryClasses] > > - BaseLib > > - PcdLib > > - DebugLib > > - HobLib > > - > > -[Pcd] > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency ## CONSUMES > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni > b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni > deleted file mode 100644 > index f55b92abac..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni > +++ /dev/null > @@ -1,17 +0,0 @@ > -// /** @file > > -// DXE CPU Timer Library > > -// > > -// Provides basic timer support using CPUID Leaf 0x15 XTAL frequency. The > performance > > -// counter features are provided by the processors time stamp counter. > > -// > > -// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > -// > > -// SPDX-License-Identifier: BSD-2-Clause-Patent > > -// > > -// **/ > > - > > - > > -#string STR_MODULE_ABSTRACT #language en-US "CPU Timer Library" > > - > > -#string STR_MODULE_DESCRIPTION #language en-US "Provides basic > timer support using CPUID Leaf 0x15 XTAL > frequency." > > - > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf > b/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf > deleted file mode 100644 > index 7af0fc44a6..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf > +++ /dev/null > @@ -1,36 +0,0 @@ > -## @file > > -# PEI CPU Timer Library > > -# > > -# Provides basic timer support using CPUID Leaf 0x15 XTAL frequency. The > performance > > -# counter features are provided by the processors time stamp counter. > > -# > > -# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > -# SPDX-License-Identifier: BSD-2-Clause-Patent > > -# > > -## > > - > > -[Defines] > > - INF_VERSION = 0x00010005 > > - BASE_NAME = PeiCpuTimerLib > > - FILE_GUID = 2B13DE00-1A5F-4DD7-A298-01B08AF1015A > > - MODULE_TYPE = BASE > > - VERSION_STRING = 1.0 > > - LIBRARY_CLASS = TimerLib|PEI_CORE PEIM > > - MODULE_UNI_FILE = PeiCpuTimerLib.uni > > - > > -[Sources] > > - CpuTimerLib.c > > - PeiCpuTimerLib.c > > - > > -[Packages] > > - MdePkg/MdePkg.dec > > - UefiCpuPkg/UefiCpuPkg.dec > > - > > -[LibraryClasses] > > - BaseLib > > - PcdLib > > - DebugLib > > - HobLib > > - > > -[Pcd] > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency ## CONSUMES > > diff --git a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni > b/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni > deleted file mode 100644 > index 49beb44908..0000000000 > --- a/UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni > +++ /dev/null > @@ -1,17 +0,0 @@ > -// /** @file > > -// PEI CPU Timer Library > > -// > > -// Provides basic timer support using CPUID Leaf 0x15 XTAL frequency. The > performance > > -// counter features are provided by the processors time stamp counter. > > -// > > -// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > -// > > -// SPDX-License-Identifier: BSD-2-Clause-Patent > > -// > > -// **/ > > - > > - > > -#string STR_MODULE_ABSTRACT #language en-US "CPU Timer Library" > > - > > -#string STR_MODULE_DESCRIPTION #language en-US "Provides basic > timer support using CPUID Leaf 0x15 XTAL > frequency." > > - > > diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc > index 98c4c53465..c16cf8d1b9 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dsc > +++ b/UefiCpuPkg/UefiCpuPkg.dsc > @@ -116,8 +116,6 @@ > UefiCpuPkg/Library/SecPeiDxeTimerLibUefiCpu/SecPeiDxeTimerLibUefiCpu.inf > > UefiCpuPkg/Application/Cpuid/Cpuid.inf > > UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf > > - UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf > > - UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf > > UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf > > UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf > > > > -- > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73807): https://edk2.groups.io/g/devel/message/73807 Mute This Topic: https://groups.io/mt/81910860/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
