[edk2-devel] [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
Ni, Ray
ray.ni at intel.com
Thu Apr 8 00:36:49 UTC 2021
Thanks for providing the detailed commit messages.
Reviewed-by: Ray Ni <ray.ni at intel.com>
> -----Original Message-----
> From: Lou, Yun <yun.lou at intel.com>
> Sent: Wednesday, April 7, 2021 4:16 PM
> To: devel at edk2.groups.io
> Cc: Lou, Yun <yun.lou at intel.com>; Ni, Ray <ray.ni at intel.com>; Dong, Eric <eric.dong at intel.com>; Laszlo Ersek
> <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>
> Subject: [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
>
> From: Jason Lou <yun.lou at 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.lou at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Rahul Kumar <rahul1.kumar at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list