public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
@ 2020-09-25  3:57 yun.lou
  2020-09-25  5:25 ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: yun.lou @ 2020-09-25  3:57 UTC (permalink / raw)
  To: devel; +Cc: jasonlouyun, Ray Ni, Eric Dong, Laszlo Ersek, Rahul Kumar

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.lou@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-01  8:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-25  3:57 [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib yun.lou
2020-09-25  5:25 ` Ni, Ray
2020-09-25  6:46   ` Laszlo Ersek
2020-09-25 14:14     ` [edk2-devel] " Zeng, Star
2021-03-25  7:58       ` Jason Lou
2021-04-01  8:41         ` Jason Lou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox