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

* Re: [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  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>
  1 sibling, 0 replies; 4+ messages in thread
From: Ni, Ray @ 2021-04-08  0:36 UTC (permalink / raw)
  To: Lou, Yun, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1

Thanks for providing the detailed commit messages.
Reviewed-by: Ray Ni <ray.ni@intel.com>


> -----Original Message-----
> From: Lou, Yun <yun.lou@intel.com>
> Sent: Wednesday, April 7, 2021 4:16 PM
> 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 v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
> 
> 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	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
       [not found] ` <1673BAF6BEAFB94F.26818@groups.io>
@ 2021-04-09  0:45   ` Ni, Ray
  2021-04-09 13:43     ` Laszlo Ersek
  0 siblings, 1 reply; 4+ messages in thread
From: Ni, Ray @ 2021-04-09  0:45 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Lou, Yun
  Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1

Laszlo,
OVMF isn't using this timerlib so I will assume you doesn't care about this change.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, April 8, 2021 8:37 AM
> To: Lou, Yun <yun.lou@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2] UefiCpuPkg: Remove PEI/DXE
> instances of CpuTimerLib.
> 
> Thanks for providing the detailed commit messages.
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> 
> > -----Original Message-----
> > From: Lou, Yun <yun.lou@intel.com>
> > Sent: Wednesday, April 7, 2021 4:16 PM
> > 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 v2] UefiCpuPkg: Remove PEI/DXE instances of
> CpuTimerLib.
> >
> > 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.i
> nf
> >
> >    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	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v2] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
  2021-04-09  0:45   ` [edk2-devel] " Ni, Ray
@ 2021-04-09 13:43     ` Laszlo Ersek
  0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2021-04-09 13:43 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Lou, Yun; +Cc: Dong, Eric, Kumar, Rahul1

Hi Ray,

On 04/09/21 02:45, Ni, Ray wrote:
> Laszlo,
> OVMF isn't using this timerlib so I will assume you doesn't care about
> this change.

correct; that's why I gave my ACK for v1 in last September [1] [2] [3]
-- I didn't want to block progress on this patch.

To be frank, I don't understand why version 2 has now been posted (the
v1 review session didn't seem to ask for changes).

The v2 posting does not include any of the v1 review tags (your R-b, and
my A-b). Neither does v2 spell out the changes relative to v1, which
would justify the absence of the v1 feedback tags in the v2 posting.
It's hard for me to remain responsive when the due process is ignored.

(In case I missed the v2 changelog: I'm sorry.)

Thanks
Laszlo

[1] https://edk2.groups.io/g/devel/message/65608
[2] http://mid.mail-archive.com/9ae864d9-7eb7-2e60-f043-299a7f7b74a7@redhat.com
[3] https://listman.redhat.com/archives/edk2-devel-archive/2020-September/msg00736.html


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