public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
@ 2021-04-07  8:16 Jason Lou
  2021-04-08  0:36 ` Ni, Ray
       [not found] ` <1673BAF6BEAFB94F.26818@groups.io>
  0 siblings, 2 replies; 4+ messages in thread
From: Jason Lou @ 2021-04-07  8:16 UTC (permalink / raw)
  To: devel; +Cc: Jason Lou, Ray Ni, Eric Dong, Laszlo Ersek, Rahul Kumar

From: Jason Lou <yun.lou@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@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                         |  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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-09 13:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-07  8:16 [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib Jason Lou
2021-04-08  0:36 ` Ni, Ray
     [not found] ` <1673BAF6BEAFB94F.26818@groups.io>
2021-04-09  0:45   ` [edk2-devel] " Ni, Ray
2021-04-09 13:43     ` Laszlo Ersek

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