Hi Donald,

Thanks for your explanation.  In this case, Reviewed-by: Eric Dong 
<eric.d...@intel.com>

Thanks,
Eric
> -----Original Message-----
> From: Kuo, Donald
> Sent: Tuesday, August 13, 2019 11:26 AM
> To: Dong, Eric <eric.d...@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Chan,
> Amy <amy.c...@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaga...@intel.com>
> Subject: RE: [PATCH] UefiCpuPkg: Adding a new TSC library by using
> CPUID(0x15) TSC leaf
> 
> Hi Eric,
> 
> The CPUID Leaf 0x1:EDX.TSC[bit 4] is to check capability for
> IA32_TIME_STAMP_COUNTER MSR and RDTSC instruction which defined in
> IA32 SDM chapter 17.17
> 
> And what we implement is based on IA32 SDM Chapter 18.7 for CPU core
> XTAL clock frequency which is from CPUID Leaf 0x15 and new TSC frequency
> = (ECX, Core XTAL Frequency) * EBX/EAX
> 
> So no need to check CPUID(0x01).
> 
> Thanks,
> Donald
> 
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Tuesday, August 13, 2019 10:27 AM
> > To: Kuo, Donald <donald....@intel.com>; devel@edk2.groups.io
> > Cc: Ni, Ray <ray...@intel.com>; Zeng, Star <star.z...@intel.com>;
> > Chan, Amy <amy.c...@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaga...@intel.com>
> > Subject: RE: [PATCH] UefiCpuPkg: Adding a new TSC library by using
> > CPUID(0x15) TSC leaf
> >
> > Hi Donald,
> >
> > Do you think it's necessary to check the TIME_STAMP_COUNTER capability
> > before using it? I see SDM has CPUID(0x01) which return the capability
> > of TIME_STAMP_COUNTER.
> >
> > Thanks,
> > Eric
> >
> > > -----Original Message-----
> > > From: Kuo, Donald
> > > Sent: Monday, August 12, 2019 7:23 PM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray <ray...@intel.com>; Zeng, Star <star.z...@intel.com>;
> > > Dong, Eric <eric.d...@intel.com>; Chan, Amy <amy.c...@intel.com>;
> > > Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>
> > > Subject: [PATCH] UefiCpuPkg: Adding a new TSC library by using
> > > CPUID(0x15) TSC leaf
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1909
> > >
> > > Cc: Ray Ni <ray...@intel.com>
> > > Cc: Star Zeng <star.z...@intel.com>
> > > Cc: Eric Dong <eric.d...@intel.com>
> > > Cc: Amy Chan <amy.c...@intel.com>
> > > Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com>
> > > Signed-off-by: Donald Kuo <donald....@intel.com>
> > > ---
> > >  .../Library/BaseCpuTimerLib/BaseCpuTimerLib.c      |  40 +++
> > >  .../Library/BaseCpuTimerLib/BaseCpuTimerLib.inf    |  35 +++
> > >  .../Library/BaseCpuTimerLib/BaseCpuTimerLib.uni    |  16 ++
> > >  UefiCpuPkg/Library/BaseCpuTimerLib/CpuTimerLib.c   | 272
> > > +++++++++++++++++++++
> > >  UefiCpuPkg/UefiCpuPkg.dec                          |   8 +
> > >  UefiCpuPkg/UefiCpuPkg.dsc                          |   1 +
> > >  6 files changed, 372 insertions(+)
> > >  create mode 100644
> > > UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.c
> > >  create mode 100644
> > > UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.inf
> > >  create mode 100644
> > > UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.uni
> > >  create mode 100644
> UefiCpuPkg/Library/BaseCpuTimerLib/CpuTimerLib.c
> > >
> > > diff --git a/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.c
> > > b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.c
> > > new file mode 100644
> > > index 0000000000..ccb92a95d3
> > > --- /dev/null
> > > +++ b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.c
> > > @@ -0,0 +1,40 @@
> > > +/** @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 <Base.h>
> > > +#include <Library/TimerLib.h>
> > > +#include <Library/BaseLib.h>
> > > +
> > > +/**
> > > +  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
> > > +  )
> > > +{
> > > +  return CpuidCoreClockCalculateTscFrequency (); }
> > > diff --git a/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.inf
> > > b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.inf
> > > new file mode 100644
> > > index 0000000000..7e27a55c90
> > > --- /dev/null
> > > +++ b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.inf
> > > @@ -0,0 +1,35 @@
> > > +## @file
> > > +#  Base 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                      = BaseCpuTimerLib
> > > +  FILE_GUID                      = F10B5B91-D15A-496C-B044-B5235721AA08
> > > +  MODULE_TYPE                    = BASE
> > > +  VERSION_STRING                 = 1.0
> > > +  LIBRARY_CLASS                  = TimerLib
> > > +  MODULE_UNI_FILE                = BaseCpuTimerLib.uni
> > > +
> > > +[Sources]
> > > +  CpuTimerLib.c
> > > +  BaseCpuTimerLib.c
> > > +
> > > +[Packages]
> > > +  MdePkg/MdePkg.dec
> > > +  UefiCpuPkg/UefiCpuPkg.dec
> > > +
> > > +[LibraryClasses]
> > > +  BaseLib
> > > +  PcdLib
> > > +  DebugLib
> > > +
> > > +[Pcd]
> > > +  gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency  ##
> > > +CONSUMES
> > > diff --git a/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.uni
> > > b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.uni
> > > new file mode 100644
> > > index 0000000000..6e5c3ef70e
> > > --- /dev/null
> > > +++ b/UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.uni
> > > @@ -0,0 +1,16 @@
> > > +// /** @file
> > > +// Base 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/BaseCpuTimerLib/CpuTimerLib.c
> > > b/UefiCpuPkg/Library/BaseCpuTimerLib/CpuTimerLib.c
> > > new file mode 100644
> > > index 0000000000..39492acd8e
> > > --- /dev/null
> > > +++ b/UefiCpuPkg/Library/BaseCpuTimerLib/CpuTimerLib.c
> > > @@ -0,0 +1,272 @@
> > > +/** @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 <Base.h>
> > > +#include <Library/TimerLib.h>
> > > +#include <Library/BaseLib.h>
> > > +#include <Library/PcdLib.h>
> > > +#include <Library/DebugLib.h>
> > > +#include <Register/Cpuid.h>
> > > +
> > > +/**
> > > +  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
> > > +  );
> > > +
> > > +/**
> > > +  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
> > > +  )
> > > +{
> > > +  CPUID_VERSION_INFO_EAX Eax;
> > > +  UINT64                 TscFrequency;
> > > +  UINT64                 CoreXtalFrequency;
> > > +  UINT32                 RegEax;
> > > +  UINT32                 RegEbx;
> > > +  UINT32                 RegEcx;
> > > +
> > > +  //
> > > +  // Use CPUID leaf 0x15 Time Stamp Counter and Nominal Core
> > > + Crystal Clock Information  // EBX returns 0 if not supported. ECX,
> > > + if non zero,
> > > provides Core Xtal Frequency in hertz.
> > > +  // TSC frequency = (ECX, Core Xtal Frequency) * EBX/EAX.
> > > +  //
> > > +  AsmCpuid (CPUID_TIME_STAMP_COUNTER, &RegEax, &RegEbx,
> > &RegEcx,
> > > NULL);
> > > +
> > > +  //
> > > +  // If EBX returns 0, the XTAL ratio is not enumerated.
> > > +  //
> > > +  ASSERT (RegEbx != 0);
> > > +  //
> > > +  // If ECX returns 0, the XTAL frequency is not enumerated.
> > > +  //
> > > +  if (RegEcx == 0) {
> > > +    CoreXtalFrequency = PcdGet64 (PcdCpuCoreCrystalClockFrequency);
> > > +  } else {
> > > +    CoreXtalFrequency = (UINT64) RegEcx;  }
> > > +
> > > +  //
> > > +  // Calculate TSC frequency = (ECX, Core Xtal Frequency) * EBX/EAX
> > > + // TscFrequency = DivU64x32 (MultU64x32 (CoreXtalFrequency,
> > > + RegEbx)
> > > + + (UINT64)(RegEax >> 1), RegEax);
> > > +
> > > +  return TscFrequency;
> > > +}
> > > +
> > > +/**
> > > +  Stalls the CPU for at least the given number of ticks.
> > > +
> > > +  Stalls the CPU for at least the given number of ticks. It's
> > > + invoked by
> > > +  MicroSecondDelay() and NanoSecondDelay().
> > > +
> > > +  @param  Delay     A period of time to delay in ticks.
> > > +
> > > +**/
> > > +VOID
> > > +InternalCpuDelay (
> > > +  IN UINT64  Delay
> > > +  )
> > > +{
> > > +  UINT64  Ticks;
> > > +
> > > +  //
> > > +  // The target timer count is calculated here  //  Ticks =
> > > + AsmReadTsc() + Delay;
> > > +
> > > +  //
> > > +  // Wait until time out
> > > +  // Timer wrap-arounds are NOT handled correctly by this function.
> > > +  // Thus, this function must be called within 10 years of reset
> > > +since
> > > +  // Intel guarantees a minimum of 10 years before the TSC wraps.
> > > +  //
> > > +  while (AsmReadTsc() <= Ticks) {
> > > +    CpuPause();
> > > +  }
> > > +}
> > > +
> > > +/**
> > > +  Stalls the CPU for at least the given number of microseconds.
> > > +
> > > +  Stalls the CPU for the number of microseconds specified by
> > MicroSeconds.
> > > +
> > > +  @param[in]  MicroSeconds  The minimum number of microseconds to
> > > delay.
> > > +
> > > +  @return MicroSeconds
> > > +
> > > +**/
> > > +UINTN
> > > +EFIAPI
> > > +MicroSecondDelay (
> > > +  IN UINTN  MicroSeconds
> > > +  )
> > > +{
> > > +
> > > +  InternalCpuDelay (
> > > +    DivU64x32 (
> > > +      MultU64x64 (
> > > +        MicroSeconds,
> > > +        CpuidCoreClockCalculateTscFrequency ()
> > > +        ),
> > > +      1000000u
> > > +    )
> > > +  );
> > > +
> > > +  return MicroSeconds;
> > > +}
> > > +
> > > +/**
> > > +  Stalls the CPU for at least the given number of nanoseconds.
> > > +
> > > +  Stalls the CPU for the number of nanoseconds specified by
> NanoSeconds.
> > > +
> > > +  @param  NanoSeconds The minimum number of nanoseconds to delay.
> > > +
> > > +  @return NanoSeconds
> > > +
> > > +**/
> > > +UINTN
> > > +EFIAPI
> > > +NanoSecondDelay (
> > > +  IN UINTN  NanoSeconds
> > > +  )
> > > +{
> > > +
> > > +  InternalCpuDelay (
> > > +    DivU64x32 (
> > > +      MultU64x64 (
> > > +        NanoSeconds,
> > > +        CpuidCoreClockCalculateTscFrequency ()
> > > +        ),
> > > +      1000000000u
> > > +    )
> > > +  );
> > > +
> > > +  return NanoSeconds;
> > > +}
> > > +
> > > +/**
> > > +  Retrieves the current value of a 64-bit free running performance
> counter.
> > > +
> > > +  Retrieves the current value of a 64-bit free running performance
> > > + counter. The  counter can either count up by 1 or count down by 1.
> > > + If the physical  performance counter counts by a larger increment,
> > > + then the counter values  must be translated. The properties of the
> > > + counter can be retrieved from  GetPerformanceCounterProperties().
> > > +
> > > +  @return The current value of the free running performance counter.
> > > +
> > > +**/
> > > +UINT64
> > > +EFIAPI
> > > +GetPerformanceCounter (
> > > +  VOID
> > > +  )
> > > +{
> > > +  return AsmReadTsc ();
> > > +}
> > > +
> > > +/**
> > > +  Retrieves the 64-bit frequency in Hz and the range of performance
> > > +counter
> > > +  values.
> > > +
> > > +  If StartValue is not NULL, then the value that the performance
> > > + counter starts  with immediately after is it rolls over is
> > > + returned in StartValue. If  EndValue is not NULL, then the value
> > > + that the performance counter end with  immediately before it rolls
> > > + over is returned in EndValue. The 64-bit  frequency of the
> > > + performance counter in Hz is always returned. If StartValue  is
> > > + less than EndValue, then the performance counter counts up. If
> > > + StartValue  is greater than EndValue, then the performance counter
> > > + counts down. For example, a 64-bit free running counter that
> > > + counts up would have a StartValue  of
> > > + 0 and an EndValue of 0xFFFFFFFFFFFFFFFF. A 24-bit free running
> > > + counter
> > > that counts down would have a StartValue of 0xFFFFFF and an EndValue
> > > of
> > 0.
> > > +
> > > +  @param  StartValue  The value the performance counter starts with
> > > + when
> > > it
> > > +                      rolls over.
> > > +  @param  EndValue    The value that the performance counter ends
> with
> > > before
> > > +                      it rolls over.
> > > +
> > > +  @return The frequency in Hz.
> > > +
> > > +**/
> > > +UINT64
> > > +EFIAPI
> > > +GetPerformanceCounterProperties (
> > > +  OUT UINT64  *StartValue,  OPTIONAL
> > > +  OUT UINT64  *EndValue     OPTIONAL
> > > +  )
> > > +{
> > > +  if (StartValue != NULL) {
> > > +    *StartValue = 0;
> > > +  }
> > > +
> > > +  if (EndValue != NULL) {
> > > +    *EndValue = 0xffffffffffffffffULL;  }  return
> > > + InternalGetPerformanceCounterFrequency (); }
> > > +
> > > +/**
> > > +  Converts elapsed ticks of performance counter to time in nanoseconds.
> > > +
> > > +  This function converts the elapsed ticks of running performance
> > > + counter to  time value in unit of nanoseconds.
> > > +
> > > +  @param  Ticks     The number of elapsed ticks of running performance
> > > counter.
> > > +
> > > +  @return The elapsed time in nanoseconds.
> > > +
> > > +**/
> > > +UINT64
> > > +EFIAPI
> > > +GetTimeInNanoSecond (
> > > +  IN UINT64  Ticks
> > > +  )
> > > +{
> > > +  UINT64  Frequency;
> > > +  UINT64  NanoSeconds;
> > > +  UINT64  Remainder;
> > > +  INTN    Shift;
> > > +
> > > +  Frequency = GetPerformanceCounterProperties (NULL, NULL);
> > > +
> > > +  //
> > > +  //          Ticks
> > > +  // Time = --------- x 1,000,000,000
> > > +  //        Frequency
> > > +  //
> > > +  NanoSeconds = MultU64x32 (DivU64x64Remainder (Ticks, Frequency,
> > > + &Remainder), 1000000000u);
> > > +
> > > +  //
> > > +  // Ensure (Remainder * 1,000,000,000) will not overflow 64-bit.
> > > +  // Since 2^29 < 1,000,000,000 = 0x3B9ACA00 < 2^30, Remainder
> > > + should <
> > > + 2^(64-30) = 2^34,  // i.e. highest bit set in Remainder should <= 33.
> > > +  //
> > > +  Shift = MAX (0, HighBitSet64 (Remainder) - 33);  Remainder =
> > > + RShiftU64 (Remainder, (UINTN) Shift);  Frequency = RShiftU64
> > > + (Frequency, (UINTN) Shift);  NanoSeconds += DivU64x64Remainder
> > > + (MultU64x32 (Remainder, 1000000000u), Frequency, NULL);
> > > +
> > > +  return NanoSeconds;
> > > +}
> > > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> > > index 14ddaa8633..a94bd2ea30 100644
> > > --- a/UefiCpuPkg/UefiCpuPkg.dec
> > > +++ b/UefiCpuPkg/UefiCpuPkg.dec
> > > @@ -211,6 +211,14 @@
> > >    # @Prompt If CPU features will be initialized during S3 resume.
> > >
> > >
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|FALSE|BOO
> > > LEAN|0x0000001D
> > >
> > > +  ## Specifies CPUID Leaf 0x15 Time Stamp Counter and Nominal Core
> > > Crystal Clock Frequency.
> > > +  # TSC Frequency = ECX (core crystal clock frequency) * EBX/EAX.
> > > +  #   Intel Xeon Processor Scalable Family with CPUID signature 06_55H =
> > > 25000000 (25MHz)
> > > +  #   6th and 7th generation Intel Core processors and Intel Xeon W
> > > Processor Family = 24000000 (24MHz)
> > > +  #   Intel Atom processors based on Goldmont Microarchitecture with
> > > CPUID signature 06_5CH = 19200000 (19.2MHz)
> > > +  # @Prompt Core Crystal Clock Frequency is for CPUID Leaf 0x15.ECX
> > > +
> > > +
> > >
> >
> gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency|24000000|
> > > UIN
> > > + T64|0x32132113
> > > +
> > >  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> PcdsDynamicEx]
> > >    ## Specifies max supported number of Logical Processors.
> > >    # @Prompt Configure max supported number of Logical Processors
> > > diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
> > > index bf690d3978..e1337c741b 100644
> > > --- a/UefiCpuPkg/UefiCpuPkg.dsc
> > > +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> > > @@ -143,6 +143,7 @@
> > >
> > >
> SmmCpuFeaturesLib|UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFea
> > > turesLibStm.inf
> > >    }
> > >    UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> > > +  UefiCpuPkg/Library/BaseCpuTimerLib/BaseCpuTimerLib.inf
> > >
> > >  [BuildOptions]
> > >    *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> > > --
> > > 2.14.2.windows.3


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#45501): https://edk2.groups.io/g/devel/message/45501
Mute This Topic: https://groups.io/mt/32839184/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to