> On Jun 13, 2016, at 4:14 PM, Kinney, Michael D <[email protected]> 
> wrote:
> 
> Star,
> 
> The PERFORMANCE_PROPERTY structure has a field called CpuFreq.
> 
> Since there are many different types of timer sources, some not 
> related to  the CPU frequencies, should this field name just be 
> Frequency?
> 
> Also, one of the long standing issues with the DP command is that
> all the modules that record performance records must use the same
> instance of the TimerLib so this same frequency counter is used 
> for all performance records.
> 
> If we are going to make the types of changes you are proposing
> here, can we also evaluate if we can update the performance 
> records to include the counter frequency information so the DP
> command can support evaluation of log entries that are generated
> using different TimerLib instances?
> 

Mike,

Maybe thee should just be multiple instances of the performance logs, one per 
time source vs tagging every entry? 

It can get really complicated as I seem to remember updating a timer lib to use 
TSC + timer XYZ. Depending on how the Freq is calculated you could get 
different frequencies for the TSC. For example a PCD value vs. calibration with 
APIC could make the time sources look not the same for the TSC. Thus maybe some 
kind of indication of the the TSC was used could be useful? 

Thanks,

Andrew Fish

> Thanks,
> 
> Mike
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:[email protected]] On Behalf Of Star 
>> Zeng
>> Sent: Sunday, June 12, 2016 12:27 AM
>> To: [email protected]
>> Cc: Carsey, Jaben <[email protected]>; Yao, Jiewen 
>> <[email protected]>; Gao,
>> Liming <[email protected]>
>> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
>> property
>> configuration table
>> 
>> Define PERFORMANCE_PROPERTY, and install performance property configuration
>> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
>> 
>> Cc: Liming Gao <[email protected]>
>> Cc: Jiewen Yao <[email protected]>
>> Cc: Cinnamon Shia <[email protected]>
>> Cc: Jaben Carsey <[email protected]>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng <[email protected]>
>> ---
>> MdeModulePkg/Include/Guid/Performance.h            | 12 ++++++++++-
>> .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
>> +++++++++++++++++++++-
>> .../DxeCorePerformanceLib.inf                      |  4 +++-
>> .../DxeCorePerformanceLibInternal.h                |  3 ++-
>> .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 ++++++++++++++++++++
>> .../SmmCorePerformanceLib.inf                      |  5 ++++-
>> 6 files changed, 65 insertions(+), 5 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Include/Guid/Performance.h
>> b/MdeModulePkg/Include/Guid/Performance.h
>> index c40046c87811..fe6972d9bf6d 100644
>> --- a/MdeModulePkg/Include/Guid/Performance.h
>> +++ b/MdeModulePkg/Include/Guid/Performance.h
>> @@ -4,7 +4,7 @@
>>   * performance protocol interfaces.
>>   * performance variables.
>> 
>> -Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials are licensed and made available 
>> under
>> the terms and conditions of the BSD License that accompanies this 
>> distribution.
>> The full text of the license may be found at
>> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR
>> IMPLIED.
>> #ifndef __PERFORMANCE_DATA_H__
>> #define __PERFORMANCE_DATA_H__
>> 
>> +#define PERFORMANCE_PROPERTY_REVISION 0x1
>> +
>> +typedef struct {
>> +  UINT32                Revision;
>> +  UINT32                Reserved;
>> +  UINT64                CpuFreq;
>> +  UINT64                TimerStartValue;
>> +  UINT64                TimerEndValue;
>> +} PERFORMANCE_PROPERTY;
>> +
>> //
>> // PEI_PERFORMANCE_STRING_SIZE must be a multiple of 8.
>> //
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 4739bb842661..de7f7cbff9b1 100644
>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> @@ -61,6 +61,8 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
>>   GetGaugeEx
>>   };
>> 
>> +PERFORMANCE_PROPERTY mPerformanceProperty;
>> +
>> /**
>>   Searches in the gauge array with keyword Handle, Token, Module and 
>> Identifier.
>> 
>> @@ -502,6 +504,10 @@ DxeCorePerformanceLibConstructor (
>>   )
>> {
>>   EFI_STATUS                Status;
>> +  UINT64                    Freq;
>> +  UINT64                    StartValue;
>> +  UINT64                    EndValue;
>> +  PERFORMANCE_PROPERTY      *PerformanceProperty;
>> 
>>   if (!PerformanceMeasurementEnabled ()) {
>>     //
>> @@ -531,7 +537,23 @@ DxeCorePerformanceLibConstructor (
>> 
>>   InternalGetPeiPerformance ();
>> 
>> -  return Status;
>> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
>> &PerformanceProperty);
>> +  if (EFI_ERROR (Status)) {
>> +    PerformanceProperty = &mPerformanceProperty;
>> +    //
>> +    // Install configuration table for performance property.
>> +    //
>> +    PerformanceProperty->Revision = PERFORMANCE_PROPERTY_REVISION;
>> +    PerformanceProperty->Reserved = 0;
>> +    Freq = GetPerformanceCounterProperties (&StartValue, &EndValue);
>> +    PerformanceProperty->CpuFreq = Freq;
>> +    PerformanceProperty->TimerStartValue = StartValue;
>> +    PerformanceProperty->TimerEndValue = EndValue;
>> +    Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid,
>> PerformanceProperty);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> }
>> 
>> /**
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> index f73d0a4386ad..40a5ddc1e53e 100644
>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> @@ -9,7 +9,7 @@
>> #  This library is mainly used by DxeCore to start performance logging to 
>> ensure that
>> #  Performance and PerformanceEx Protocol are installed at the very 
>> beginning of DXE
>> phase.
>> #
>> -#  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
>> +#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>> #  This program and the accompanying materials
>> #  are licensed and made available under the terms and conditions of the BSD 
>> License
>> @@ -56,11 +56,13 @@ [LibraryClasses]
>>   BaseLib
>>   HobLib
>>   DebugLib
>> +  UefiLib
>> 
>> 
>> [Guids]
>>   ## SOMETIMES_CONSUMES   ## HOB
>>   ## PRODUCES             ## UNDEFINED # Install protocol
>> +  ## PRODUCES             ## SystemTable
>>   gPerformanceProtocolGuid
>>   ## SOMETIMES_CONSUMES   ## HOB
>>   ## PRODUCES             ## UNDEFINED # Install protocol
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInternal.h
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInternal.h
>> index 2b9ccd2fee0c..fb5c6c2395ea 100644
>> --- 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInternal.h
>> +++ 
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInternal.h
>> @@ -4,7 +4,7 @@
>>   This header file holds the prototypes of the Performance and PerformanceEx 
>> Protocol
>> published by this
>>   library instance at its constructor.
>> 
>> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD 
>> License
>> which accompanies this distribution.  The full text of the license may be 
>> found at
>> @@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR
>> IMPLIED.
>> #include <Library/PcdLib.h>
>> #include <Library/UefiBootServicesTableLib.h>
>> #include <Library/MemoryAllocationLib.h>
>> +#include <Library/UefiLib.h>
>> 
>> //
>> // Interface declarations for PerformanceEx Protocol.
>> diff --git 
>> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
>> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
>> index 6f8d2dd064a3..93f7a83437a4 100644
>> --- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
>> @@ -69,6 +69,8 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
>>   GetGaugeEx
>> };
>> 
>> +PERFORMANCE_PROPERTY mPerformanceProperty;
>> +
>> /**
>>   Searches in the gauge array with keyword Handle, Token, Module and 
>> Identfier.
>> 
>> @@ -687,6 +689,10 @@ InitializeSmmCorePerformanceLib (
>> {
>>   EFI_STATUS                Status;
>>   EFI_HANDLE                Handle;
>> +  UINT64                    Freq;
>> +  UINT64                    StartValue;
>> +  UINT64                    EndValue;
>> +  PERFORMANCE_PROPERTY      *PerformanceProperty;
>> 
>>   //
>>   // Initialize spin lock
>> @@ -725,6 +731,22 @@ InitializeSmmCorePerformanceLib (
>>   ASSERT_EFI_ERROR (Status);
>>   Status = gSmst->SmiHandlerRegister (SmmPerformanceHandlerEx,
>> &gSmmPerformanceExProtocolGuid, &Handle);
>>   ASSERT_EFI_ERROR (Status);
>> +
>> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
>> &PerformanceProperty);
>> +  if (EFI_ERROR (Status)) {
>> +    PerformanceProperty = &mPerformanceProperty;
>> +    //
>> +    // Install configuration table for performance property.
>> +    //
>> +    PerformanceProperty->Revision = PERFORMANCE_PROPERTY_REVISION;
>> +    PerformanceProperty->Reserved = 0;
>> +    Freq = GetPerformanceCounterProperties (&StartValue, &EndValue);
>> +    PerformanceProperty->CpuFreq = Freq;
>> +    PerformanceProperty->TimerStartValue = StartValue;
>> +    PerformanceProperty->TimerEndValue = EndValue;
>> +    Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid,
>> PerformanceProperty);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>> }
>> 
>> /**
>> diff --git 
>> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
>> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
>> index 160a749390e1..5b5924d41848 100644
>> --- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
>> +++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
>> @@ -8,7 +8,7 @@
>> #  This library is mainly used by SMM Core to start performance logging to 
>> ensure that
>> #  SMM Performance and PerformanceEx Protocol are installed at the very 
>> beginning of
>> SMM phase.
>> #
>> -#  Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR>
>> +#  Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
>> #  This program and the accompanying materials
>> #  are licensed and made available under the terms and conditions of the BSD 
>> License
>> #  which accompanies this distribution.  The full text of the license may be 
>> found at
>> @@ -57,6 +57,7 @@ [LibraryClasses]
>>   SynchronizationLib
>>   SmmServicesTableLib
>>   SmmMemLib
>> +  UefiLib
>> 
>> [Protocols]
>>   gEfiSmmBase2ProtocolGuid                  ## CONSUMES
>> @@ -68,6 +69,8 @@ [Guids]
>>   ## PRODUCES ## UNDEFINED # Install protocol
>>   ## CONSUMES ## UNDEFINED # SmiHandlerRegister
>>   gSmmPerformanceExProtocolGuid
>> +  ## PRODUCES ## SystemTable
>> +  gPerformanceProtocolGuid
>> 
>> [Pcd]
>>   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask    ## CONSUMES
>> --
>> 2.7.0.windows.1
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> [email protected]
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to