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

* Re: [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  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
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2020-09-25  5:25 UTC (permalink / raw)
  To: Lou, Yun, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Lou, Yun <yun.lou@intel.com>
> Sent: Friday, September 25, 2020 11:58 AM
> To: devel@edk2.groups.io
> Cc: Lou, Yun <yun.lou@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@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.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	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  2020-09-25  5:25 ` Ni, Ray
@ 2020-09-25  6:46   ` Laszlo Ersek
  2020-09-25 14:14     ` [edk2-devel] " Zeng, Star
  0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2020-09-25  6:46 UTC (permalink / raw)
  To: Ni, Ray, Lou, Yun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul1

On 09/25/20 07:25, Ni, Ray wrote:
> Reviewed-by: Ray Ni <ray.ni@intel.com>

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
>> -----Original Message-----
>> From: Lou, Yun <yun.lou@intel.com>
>> Sent: Friday, September 25, 2020 11:58 AM
>> To: devel@edk2.groups.io
>> Cc: Lou, Yun <yun.lou@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@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.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	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  2020-09-25  6:46   ` Laszlo Ersek
@ 2020-09-25 14:14     ` Zeng, Star
  2021-03-25  7:58       ` Jason Lou
  0 siblings, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2020-09-25 14:14 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Lou, Yun
  Cc: Dong, Eric, Kumar, Rahul1, Zeng, Star

A little surprised by "Average time taken to find CpuCrystalFrequencyHob: about 2000 ns".
It depends on the hob is built early or late?
Seemingly, NanoSecondDelay() will always have some deviation.

Thanks,
Star
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Friday, September 25, 2020 2:46 PM
To: Ni, Ray <ray.ni@intel.com>; Lou, Yun <yun.lou@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.

On 09/25/20 07:25, Ni, Ray wrote:
> Reviewed-by: Ray Ni <ray.ni@intel.com>

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
>> -----Original Message-----
>> From: Lou, Yun <yun.lou@intel.com>
>> Sent: Friday, September 25, 2020 11:58 AM
>> To: devel@edk2.groups.io
>> Cc: Lou, Yun <yun.lou@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, 
>> Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, 
>> Rahul1 <rahul1.kumar@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.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	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  2020-09-25 14:14     ` [edk2-devel] " Zeng, Star
@ 2021-03-25  7:58       ` Jason Lou
  2021-04-01  8:41         ` Jason Lou
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Lou @ 2021-03-25  7:58 UTC (permalink / raw)
  To: Zeng, Star, devel

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Hi Star, sorry for my late response.

Yes, HOB search time is related to the sequence of HOB generation.

Some data collected on the test platform:
1. The time range required to search for the specified HOB: 20ns ~ 2700ns
Search for the 1st HOB(PHIT HOB): 20ns
Search for the last HOB: about 2700ns
2. The time required to search for CpuCrystalFrequencyHob HOB: about 2000ns.

The actual delay of NanoSecondDelay function is longer than the expected delay.

If any questions, please feel free let me know, thanks

Jason Lou

[-- Attachment #2: Type: text/html, Size: 688 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  2021-03-25  7:58       ` Jason Lou
@ 2021-04-01  8:41         ` Jason Lou
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Lou @ 2021-04-01  8:41 UTC (permalink / raw)
  To: Jason Lou, devel

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

1. Since the time it takes to find specified GUID Extension Hob (CpuCrystalFrequencyHob: about 2000ns)
is much longer than it takes to calculate TSC frequency with CPUID (about 450ns), I recommend using
BaseCpuTimerLib instead of PeiCpuTimerLib.

2. The time it takes to get TSC frequency from global variable is shorter than it takes to calculate TSC
frequency with CPUID, but 450ns is really a short time, the impact on the platform is very limited.
In addition, in order to simplify the code, recommend using BaseCpuTimerLib instead of DxeCpuTimerLib.

If anyone has questions or concerns, please feel free to let me know. If not, I will submit code patch later,
thanks.

Jason Lou

[-- Attachment #2: Type: text/html, Size: 773 bytes --]

^ permalink raw reply	[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