On 09/25/20 07:25, Ni, Ray wrote: > Reviewed-by: Ray Ni <ray...@intel.com>
Acked-by: Laszlo Ersek <ler...@redhat.com> Thanks Laszlo > >> -----Original Message----- >> From: Lou, Yun <yun....@intel.com> >> Sent: Friday, September 25, 2020 11:58 AM >> 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 v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib. >> >> 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 >> frequncy 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, the constructor function is called, it will take extra time to >> calculate TSC frequency. >> The time it takes to get TSC frequncy 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 Intel server platform and collected the >> following data: >> 1. Average time taken to find CpuCrystalFrequencyHob: about 2000 ns. >> 2. Average time taken to calculate TSC frequency: about 450 ns. >> >> Reference code: >> // >> // Calculate average time taken 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 taken to find >> Hob = %d ns\n", \ >> DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), >> 1000000000), *CpuCrystalCounterFrequency, NULL), >> 1000))); >> } >> >> // >> // Calculate average time taken 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 taken 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 | 4 +- >> 7 files changed, 1 insertion(+), 253 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c >> b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c >> deleted file mode 100644 >> index 269e5a3e83d7..000000000000 >> --- 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 91a721205653..000000000000 >> --- 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 6c83549c87da..000000000000 >> --- 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 f55b92abace7..000000000000 >> --- 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 7af0fc44a65d..000000000000 >> --- 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 49beb44908d6..000000000000 >> --- 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 b2b6d78a71b0..e915b5c81b66 100644 >> --- a/UefiCpuPkg/UefiCpuPkg.dsc >> +++ b/UefiCpuPkg/UefiCpuPkg.dsc >> @@ -1,7 +1,7 @@ >> ## @file >> >> # UefiCpuPkg Package >> >> # >> >> -# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR> >> >> +# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR> >> >> # >> >> # SPDX-License-Identifier: BSD-2-Clause-Patent >> >> # >> >> @@ -107,8 +107,6 @@ [Components] >> 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 >> >> >> >> [Components.IA32, Components.X64] >> >> UefiCpuPkg/CpuDxe/CpuDxe.inf >> >> -- >> 2.28.0.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65608): https://edk2.groups.io/g/devel/message/65608 Mute This Topic: https://groups.io/mt/77073830/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
