Re: [PATCH v4 2/3] PerformancePkg Dp_App: Remove TimerLib dependency

["Yao, Jiewen" <jiewen.yao@intel.com>]

I have one more suggestion.
Can we use MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf as default one in DSC?

So that the binary built with default DSC can be used on any platform.

PerformancePkg.dsc:
   PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Michael Kinney
> Sent: Friday, February 3, 2017 12:56 PM
> To: edk2-devel@lists.01.org
> Cc: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH v4 2/3] PerformancePkg Dp_App: Remove TimerLib
> dependency
> 
> From: Star Zeng <star.zeng@intel.com>
> 
> Current Dp_App implementation depends on TimerLib,
> as different platforms may implement and use their
> own TimerLib, it makes the dp application needs to
> be built by platform. The TimerLib dependency can
> be removed by using performance property configuration
> table to make Dp_App to be generic.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Cinnamon Shia <cinnamon.shia@hpe.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  PerformancePkg/Dp_App/Dp.c          | 35
> +++++++++++++++++------------------
>  PerformancePkg/Dp_App/Dp.inf        |  6 ++++--
>  PerformancePkg/Dp_App/DpInternal.h  |  6 ++----
>  PerformancePkg/Dp_App/DpProfile.c   |  3 +--
>  PerformancePkg/Dp_App/DpStrings.uni | 10 ++++++----
>  PerformancePkg/Dp_App/DpTrace.c     | 25 ++-----------------------
>  PerformancePkg/Dp_App/DpUtilities.c |  3 +--
>  PerformancePkg/Dp_App/Literals.c    |  3 +--
>  8 files changed, 34 insertions(+), 57 deletions(-)
> 
> diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c
> index 26a3ebc..389f368 100644
> --- a/PerformancePkg/Dp_App/Dp.c
> +++ b/PerformancePkg/Dp_App/Dp.c
> @@ -13,7 +13,7 @@
>    Dp uses this information to group records in different ways.  It also uses
>    timer information to calculate elapsed time for each measurement.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2015-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
> @@ -30,7 +30,6 @@
>  #include <Library/BaseLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/DebugLib.h>
> -#include <Library/TimerLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/UefiHiiServicesLib.h>
>  #include <Library/HiiLib.h>
> @@ -201,14 +200,13 @@ InitializeDp (
>    IN EFI_SYSTEM_TABLE         *SystemTable
>    )
>  {
> -  UINT64                    Freq;
> -  UINT64                    Ticker;
> -  UINT32                    ListIndex;
> -
> -  LIST_ENTRY                *ParamPackage;
> -  CONST CHAR16              *CmdLineArg;
> -  EFI_STRING                StringPtr;
> -  UINTN                     Number2Display;
> +  PERFORMANCE_PROPERTY          *PerformanceProperty;
> +  UINT32                        ListIndex;
> +
> +  LIST_ENTRY                    *ParamPackage;
> +  CONST CHAR16                  *CmdLineArg;
> +  EFI_STRING                    StringPtr;
> +  UINTN                         Number2Display;
> 
>    EFI_STATUS                    Status;
>    BOOLEAN                       SummaryMode;
> @@ -267,11 +265,6 @@ InitializeDp (
>    StringDpOptionLc = NULL;
>    StringPtr        = NULL;
> 
> -  // Get DP's entry time as soon as possible.
> -  // This is used as the Shell-Phase end time.
> -  //
> -  Ticker  = GetPerformanceCounter ();
> -
>    //
>    // Retrieve HII package list from ImageHandle
>    //
> @@ -406,10 +399,16 @@ InitializeDp (
>        //    StartCount = Value loaded into the counter when it starts counting
>        //      EndCount = Value counter counts to before it needs to be reset
>        //
> -      Freq = GetPerformanceCounterProperties (&TimerInfo.StartCount,
> &TimerInfo.EndCount);
> +      Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
> &PerformanceProperty);
> +      if (EFI_ERROR (Status)) {
> +        PrintToken (STRING_TOKEN (STR_PERF_PROPERTY_NOT_FOUND));
> +        goto Done;
> +      }
> 
>        // Convert the Frequency from Hz to KHz
> -      TimerInfo.Frequency = (UINT32)DivU64x32 (Freq, 1000);
> +      TimerInfo.Frequency  = (UINT32)DivU64x32
> (PerformanceProperty->Frequency, 1000);
> +      TimerInfo.StartCount = PerformanceProperty->TimerStartValue;
> +      TimerInfo.EndCount   = PerformanceProperty->TimerEndValue;
> 
>        // Determine in which direction the performance counter counts.
>        TimerInfo.CountUp = (BOOLEAN) (TimerInfo.EndCount >=
> TimerInfo.StartCount);
> @@ -487,7 +486,7 @@ InitializeDp (
>        else {
>          //------------- Begin Cooked Mode Processing
>          if (TraceMode) {
> -          ProcessPhases ( Ticker );
> +          ProcessPhases ();
>            if ( ! SummaryMode) {
>              Status = ProcessHandles ( ExcludeMode);
>              if (Status == EFI_ABORTED) {
> diff --git a/PerformancePkg/Dp_App/Dp.inf b/PerformancePkg/Dp_App/Dp.inf
> index 8abc20f..1204d8e 100644
> --- a/PerformancePkg/Dp_App/Dp.inf
> +++ b/PerformancePkg/Dp_App/Dp.inf
> @@ -1,7 +1,7 @@
>  ##  @file
>  #  Display Performance Application, Module information file.
>  #
> -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2017, 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
> @@ -56,7 +56,6 @@
>    MemoryAllocationLib
>    DebugLib
>    UefiBootServicesTableLib
> -  TimerLib
>    PeCoffGetEntryPointLib
>    PerformanceLib
>    PrintLib
> @@ -67,6 +66,9 @@
>    DevicePathLib
>    DxeServicesLib
> 
> +[Guids]
> +  gPerformanceProtocolGuid                                ##
> CONSUMES ## SystemTable
> +
>  [Protocols]
>    gEfiLoadedImageProtocolGuid                             ##
> CONSUMES
>    gEfiHiiPackageListProtocolGuid                          ## CONSUMES
> diff --git a/PerformancePkg/Dp_App/DpInternal.h
> b/PerformancePkg/Dp_App/DpInternal.h
> index 53c5fb2..1ab36ba 100644
> --- a/PerformancePkg/Dp_App/DpInternal.h
> +++ b/PerformancePkg/Dp_App/DpInternal.h
> @@ -6,7 +6,7 @@
>    Dp application.  In addition to global data, function declarations for
>    DpUtilities.c, DpTrace.c, and DpProfile.c are included here.
> 
> -  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2015-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
> @@ -254,12 +254,10 @@ DumpRawTrace(
>  /**
>    Gather and print Major Phase metrics.
> 
> -  @param[in]    Ticker      The timer value for the END of Shell phase
> -
>  **/
>  VOID
>  ProcessPhases(
> -  IN UINT64 Ticker
> +  VOID
>    );
> 
> 
> diff --git a/PerformancePkg/Dp_App/DpProfile.c
> b/PerformancePkg/Dp_App/DpProfile.c
> index eb5b3dd..15c2ef1 100644
> --- a/PerformancePkg/Dp_App/DpProfile.c
> +++ b/PerformancePkg/Dp_App/DpProfile.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Measured Profiling reporting for the Dp utility.
> 
> -  Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, 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
> @@ -16,7 +16,6 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> -#include <Library/TimerLib.h>
>  #include <Library/PeCoffGetEntryPointLib.h>
>  #include <Library/PerformanceLib.h>
>  #include <Library/PrintLib.h>
> diff --git a/PerformancePkg/Dp_App/DpStrings.uni
> b/PerformancePkg/Dp_App/DpStrings.uni
> index daaa242..131d16b 100644
> --- a/PerformancePkg/Dp_App/DpStrings.uni
> +++ b/PerformancePkg/Dp_App/DpStrings.uni
> @@ -1,12 +1,12 @@
>  // *++
>  //
> -// Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +// Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>  // (C) Copyright 2015 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
>  // which accompanies this distribution.  The full text of the license may be
> found at
>  // http://opensource.org/licenses/bsd-license.php
> -//
> +//
>  // THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>  // WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
>  //
> @@ -114,7 +114,7 @@
>                                         #language fr-FR  "   -n
> COUNT\n"
>  #string STR_DP_HELP_ID                 #language en-US  "   -i  display
> identifier\n"
>                                         #language fr-FR  "   -i\n"
> -#string STR_DP_HELP_CUM_DATA           #language en-US  "   -c TOKEN
> - Display pre-defined and custom cumulative data\r\n"
> +#string STR_DP_HELP_CUM_DATA           #language en-US  "   -c TOKEN
> - Display pre-defined and custom cumulative data\r\n"
>                                                          "
> Pre-defined cumulative token are:\r\n"
>                                                          "
> 1. LoadImage:\r\n"
>                                                          "
> 2. StartImage:\r\n"
> @@ -137,6 +137,8 @@
>                                         #language fr-FR  "Localiser Toute
> erreur de Poignées - %r\n"
>  #string STR_DP_ERROR_NAME              #language en-US  "Unknown
> Driver Name"
>                                         #language fr-FR  "Unknown
> Driver Name"
> +#string STR_PERF_PROPERTY_NOT_FOUND    #language en-US
> "Performance property not found\n"
> +                                       #language fr-FR  "Performance
> property not found\n"
>  #string STR_DP_BUILD_REVISION          #language en-US  "\nDP Build
> Version:       %d.%d\n"
>                                         #language fr-FR  "\nDP Construit
> la Version:         %d,%d\n"
>  #string STR_DP_KHZ                     #language en-US  "System
> Performance Timer Frequency:   %,8d (KHz)\n"
> @@ -238,7 +240,7 @@
>  #string STR_DP_RAW_VARS2               #language en-US
> "%5d: %16LX %16LX %16LX  %31a  %31a %5d\n"
>                                         #language fr-FR
> "%5d: %16LX %16LX %16LX  %31a  %31a %5d\n"
>  #string STR_DP_RAW_HEADR2              #language en-US  "\nIndex
> Handle        Start Count       End Count                  Token
> Module                   ID\n"
> -                                       #language fr-FR  "\nIndex
> Handle        Start Count       End Count                  Token
> Module                   ID\n"
> +                                       #language fr-FR  "\nIndex
> Handle        Start Count       End Count                  Token
> Module                   ID\n"
>  #string STR_DP_OPTION_UA               #language en-US  "-A"
>                                         #language fr-FR  "-A"
>  #string STR_DP_OPTION_LA               #language en-US  "-a"
> diff --git a/PerformancePkg/Dp_App/DpTrace.c
> b/PerformancePkg/Dp_App/DpTrace.c
> index 6a62bba..9b4e0ed 100644
> --- a/PerformancePkg/Dp_App/DpTrace.c
> +++ b/PerformancePkg/Dp_App/DpTrace.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Trace reporting for the Dp utility.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2015-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
> @@ -17,7 +17,6 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> -#include <Library/TimerLib.h>
>  #include <Library/PeCoffGetEntryPointLib.h>
>  #include <Library/PerformanceLib.h>
>  #include <Library/PrintLib.h>
> @@ -385,12 +384,10 @@ DumpRawTrace(
>  /**
>    Gather and print Major Phase metrics.
> 
> -  @param[in]    Ticker      The timer value for the END of Shell phase
> -
>  **/
>  VOID
>  ProcessPhases(
> -  IN UINT64            Ticker
> +  VOID
>    )
>  {
>    MEASUREMENT_RECORD        Measurement;
> @@ -399,7 +396,6 @@ ProcessPhases(
>    UINT64                    PeiTime;
>    UINT64                    DxeTime;
>    UINT64                    BdsTime;
> -  UINT64                    ShellTime;
>    UINT64                    ElapsedTime;
>    UINT64                    Duration;
>    UINT64                    Total;
> @@ -412,7 +408,6 @@ ProcessPhases(
>    PeiTime         = 0;
>    DxeTime         = 0;
>    BdsTime         = 0;
> -  ShellTime       = 0;
>    //
>    // Get Execution Phase Statistics
>    //
> @@ -433,9 +428,6 @@ ProcessPhases(
>                            &Measurement.EndTimeStamp,
>                            &Measurement.Identifier)) != 0)
>    {
> -    if (AsciiStrnCmp (Measurement.Token, ALit_SHELL, PERF_TOKEN_LENGTH)
> == 0) {
> -      Measurement.EndTimeStamp = Ticker;
> -    }
>      if (Measurement.EndTimeStamp == 0) { // Skip "incomplete" records
>        continue;
>      }
> @@ -453,8 +445,6 @@ ProcessPhases(
>        DxeTime      = Duration;
>      } else if (AsciiStrnCmp (Measurement.Token, ALit_BDS,
> PERF_TOKEN_LENGTH) == 0) {
>        BdsTime      = Duration;
> -    } else if (AsciiStrnCmp (Measurement.Token, ALit_SHELL,
> PERF_TOKEN_LENGTH) == 0) {
> -      ShellTime    = Duration;
>      }
>    }
> 
> @@ -509,17 +499,6 @@ ProcessPhases(
>      PrintToken (STRING_TOKEN (STR_DP_PHASE_BDSTO), ALit_BdsTO,
> ElapsedTime);
>    }
> 
> -  // print SHELL phase duration time
> -  //
> -  if (ShellTime > 0) {
> -    ElapsedTime = DivU64x32 (
> -                    ShellTime,
> -                    (UINT32)TimerInfo.Frequency
> -                    );
> -    Total += ElapsedTime;
> -    PrintToken (STRING_TOKEN (STR_DP_PHASE_DURATION), ALit_SHELL,
> ElapsedTime);
> -  }
> -
>    PrintToken (STRING_TOKEN (STR_DP_TOTAL_DURATION), Total);
>  }
> 
> diff --git a/PerformancePkg/Dp_App/DpUtilities.c
> b/PerformancePkg/Dp_App/DpUtilities.c
> index 13d3897..d5840e8 100644
> --- a/PerformancePkg/Dp_App/DpUtilities.c
> +++ b/PerformancePkg/Dp_App/DpUtilities.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Utility functions used by the Dp application.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, 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
> @@ -17,7 +17,6 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> -#include <Library/TimerLib.h>
>  #include <Library/PeCoffGetEntryPointLib.h>
>  #include <Library/PrintLib.h>
>  #include <Library/HiiLib.h>
> diff --git a/PerformancePkg/Dp_App/Literals.c
> b/PerformancePkg/Dp_App/Literals.c
> index 68de0fb..c1cddfb 100644
> --- a/PerformancePkg/Dp_App/Literals.c
> +++ b/PerformancePkg/Dp_App/Literals.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Definitions of ASCII string literals used by DP.
> 
> -  Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, 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
> @@ -16,7 +16,6 @@
>  CHAR8 const ALit_TimerLibError[] = "Timer library instance error!\n";
>  CHAR8 const ALit_SEC[]    = SEC_TOK;
>  CHAR8 const ALit_DXE[]    = DXE_TOK;
> -CHAR8 const ALit_SHELL[]  = SHELL_TOK;
>  CHAR8 const ALit_PEI[]    = PEI_TOK;
>  CHAR8 const ALit_BDS[]    = BDS_TOK;
>  CHAR8 const ALit_BdsTO[]  = "BdsTimeOut";
> --
> 2.6.3.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel