Andrew, Sorry, I am still confused about that. Extending PEI_PERFORMANCE_LOG_HEADER can only record the timer source of PeiCore. Except we are going to record time instead of tick, I am not sure how to know the caller may be using different instance of TimerLib by current interfaces PERF_START() ->StartPerformanceMeasurement() and PERF_END()->EndPerformanceMeasurement()
#define PERF_START(Handle, Token, Module, TimeStamp) RETURN_STATUS EFIAPI StartPerformanceMeasurement ( IN CONST VOID *Handle, OPTIONAL IN CONST CHAR8 *Token, OPTIONAL IN CONST CHAR8 *Module, OPTIONAL IN UINT64 TimeStamp ); Thanks, Star -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Andrew Fish Sent: Wednesday, June 15, 2016 10:25 AM To: Zeng, Star <[email protected]> Cc: Kinney, Michael D <[email protected]>; Carsey, Jaben <[email protected]>; [email protected]; Yao, Jiewen <[email protected]>; Gao, Liming <[email protected]> Subject: Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table > On Jun 14, 2016, at 6:55 PM, Zeng, Star <[email protected]> wrote: > > Mike & Andrew, > > Could you have more detail about the idea of a record type for a time source? > I am not so clear about that. > Star, For example the PEI_PERFORMANCE_LOG_HEADER could be updated to included the frequency and/or and/or an enum that identifies the source like TSC. https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Guid/Performance.h#L38 Then each time source that was unique could be a different instance of the MdeModulePkg/Include/Guid/Performance.h data. Thanks, Andrew Fish > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Tuesday, June 14, 2016 9:27 AM > To: [email protected]; Kinney, Michael D <[email protected]> > Cc: Zeng, Star <[email protected]>; [email protected]; Carsey, > Jaben <[email protected]>; Yao, Jiewen <[email protected]>; > Gao, Liming <[email protected]> > Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install > performance property configuration table > > Andrew, > > I like the idea of a record type for a time source. A PerformanceLib > constructor could add that record, and performance log entries could be > tagged with a time source record indicator. > > I agree that any timer source that requires an algorithm to measure the > frequency could get slightly different frequency values which would increase > the number of time source records for the same timer. > > The other even more complicated issue is a performance record whose start > value is set in one module and stop value is set in a different module. If > those two modules use different TimerLib instances, then it won't work. > Which is one of the reasons the single TimerLib instance solution is used. > > Mike > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> Sent: Monday, June 13, 2016 4:34 PM >> To: Kinney, Michael D <[email protected]> >> Cc: Zeng, Star <[email protected]>; [email protected]; >> Carsey, Jaben <[email protected]>; Yao, Jiewen >> <[email protected]>; Gao, Liming <[email protected]> >> Subject: Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install >> performance property configuration table >> >> >>> 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/DxeCorePerformance >>>> +++ Lib.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/DxeCorePerformance >>>> +++ Lib.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/DxeCorePerformanceLibInt >> e >> rnal.h >>>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibI >>>> nternal.h index 2b9ccd2fee0c..fb5c6c2395ea 100644 >>>> --- >>>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibI >>>> nternal.h >>>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformance >>>> +++ LibInternal.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/SmmCorePerformance >>>> +++ Lib.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/SmmCorePerformance >>>> +++ Lib.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 _______________________________________________ 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
