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/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
