Re: [PATCH v4 1/3] MdeModulePkg: Add performance property configuration table

["Zeng, Star" <star.zeng@intel.com>]

Agree, I will send patch for it ASAP.

Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Friday, February 24, 2017 1:30 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Carsey, Jaben <jaben.carsey@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [PATCH v4 1/3] MdeModulePkg: Add performance property configuration table

On 3 February 2017 at 05:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thank you Mike and Star.
>
> It is good idea to remove TimerLib dependency.
> Series Reviewed-by: Jiewen.yao@intel.com
>

This patch breaks the GCC build:

<https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c>:
In function 'DxeCorePerformanceLibConstructor':
<https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c>:538:3:
error: passing argument 2 of 'EfiGetSystemConfigurationTable' from incompatible pointer type [-Werror]
   Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid, &PerformanceProperty);
   ^
In file included from
<https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInternal.h>:35:0,
                 from
<https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c>:26:
<https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdePkg/Include/Library/UefiLib.h>:135:1:
note: expected 'void **' but argument is of type 'struct PERFORMANCE_PROPERTY **'
 EfiGetSystemConfigurationTable (
 ^

This fixes it for me:

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 1564514518d3..5438bd086144 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -535,7 +535,7 @@ DxeCorePerformanceLibConstructor (

   InternalGetPeiPerformance ();

-  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid, &PerformanceProperty);
+  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
(VOID **)&PerformanceProperty);
   if (EFI_ERROR (Status)) {
     //
     // Install configuration table for performance property.


Thanks,
Ard,


>
>> -----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 1/3] MdeModulePkg: Add performance property 
>> configuration table
>>
>> From: Star Zeng <star.zeng@intel.com>
>>
>> Define PERFORMANCE_PROPERTY, and install performance property 
>> configuration table in DxeCorePerformanceLib and 
>> SmmCorePerformanceLib.
>>
>> 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>
>> ---
>>  MdeModulePkg/Include/Guid/Performance.h            | 12 ++++++++++-
>>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 23
>> ++++++++++++++++++++--
>>  .../DxeCorePerformanceLib.inf                      |  2 ++
>>  .../DxeCorePerformanceLibInternal.h                |  3 ++-
>>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 18
>> +++++++++++++++++
>>  .../SmmCorePerformanceLib.inf                      |  5 ++++-
>>  6 files changed, 58 insertions(+), 5 deletions(-)
>>
>> diff --git a/MdeModulePkg/Include/Guid/Performance.h
>> b/MdeModulePkg/Include/Guid/Performance.h
>> index c40046c..df40c6c 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 - 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 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                Frequency;
>> +  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 4739bb8..1564514 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> @@ -10,7 +10,7 @@
>>    This library is mainly used by DxeCore to start performance 
>> logging to ensure that
>>    Performance Protocol is installed at the very beginning of DXE phase.
>>
>> -Copyright (c) 2006 - 2016, Intel Corporation. All rights 
>> reserved.<BR>
>> +Copyright (c) 2006 - 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 @@ -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,8 @@ DxeCorePerformanceLibConstructor (
>>    )
>>  {
>>    EFI_STATUS                Status;
>> +  PERFORMANCE_PROPERTY      *PerformanceProperty;
>> +
>>
>>    if (!PerformanceMeasurementEnabled ()) {
>>      //
>> @@ -531,7 +535,22 @@ DxeCorePerformanceLibConstructor (
>>
>>    InternalGetPeiPerformance ();
>>
>> -  return Status;
>> +  Status = EfiGetSystemConfigurationTable 
>> + (&gPerformanceProtocolGuid,
>> &PerformanceProperty);
>> +  if (EFI_ERROR (Status)) {
>> +    //
>> +    // Install configuration table for performance property.
>> +    //
>> +    mPerformanceProperty.Revision  =
>> PERFORMANCE_PROPERTY_REVISION;
>> +    mPerformanceProperty.Reserved  = 0;
>> +    mPerformanceProperty.Frequency = GetPerformanceCounterProperties 
>> + (
>> +
>> &mPerformanceProperty.TimerStartValue,
>> +
>> &mPerformanceProperty.TimerEndValue
>> +                                       );
>> +    Status = gBS->InstallConfigurationTable 
>> + (&gPerformanceProtocolGuid,
>> &mPerformanceProperty);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>> +
>> +  return EFI_SUCCESS;
>>  }
>>
>>  /**
>> diff --git
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.in
>> f 
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.in
>> f
>> index e091c62..5b89ce2 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.in
>> f
>> +++
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.in
>> f
>> @@ -56,11 +56,13 @@
>>    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
>> ernal.h
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInt
>> ernal.h
>> index 2b9ccd2..f1540d8 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInt
>> ernal.h
>> +++
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLibInt
>> ernal.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 - 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 @@ -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 6f8d2dd..a03a8c8 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,7 @@ InitializeSmmCorePerformanceLib (  {
>>    EFI_STATUS                Status;
>>    EFI_HANDLE                Handle;
>> +  PERFORMANCE_PROPERTY      *PerformanceProperty;
>>
>>    //
>>    // Initialize spin lock
>> @@ -725,6 +728,21 @@ InitializeSmmCorePerformanceLib (
>>    ASSERT_EFI_ERROR (Status);
>>    Status = gSmst->SmiHandlerRegister (SmmPerformanceHandlerEx,
>> &gSmmPerformanceExProtocolGuid, &Handle);
>>    ASSERT_EFI_ERROR (Status);
>> +
>> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
>> &PerformanceProperty);
>> +  if (EFI_ERROR (Status)) {
>> +    //
>> +    // Install configuration table for performance property.
>> +    //
>> +    mPerformanceProperty.Revision  =
>> PERFORMANCE_PROPERTY_REVISION;
>> +    mPerformanceProperty.Reserved  = 0;
>> +    mPerformanceProperty.Frequency = GetPerformanceCounterProperties (
>> +
>> &mPerformanceProperty.TimerStartValue,
>> +
>> &mPerformanceProperty.TimerEndValue
>> +                                       );
>> +    Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid,
>> &mPerformanceProperty);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>>  }
>>
>>  /**
>> diff --git
>> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.i
>> nf
>> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.i
>> nf
>> index 160a749..1b2fbd3 100644
>> ---
>> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.i
>> nf
>> +++
>> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.i
>> nf
>> @@ -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 - 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
>> @@ -57,6 +57,7 @@
>>    SynchronizationLib
>>    SmmServicesTableLib
>>    SmmMemLib
>> +  UefiLib
>>
>>  [Protocols]
>>    gEfiSmmBase2ProtocolGuid                  ## CONSUMES
>> @@ -68,6 +69,8 @@
>>    ## PRODUCES ## UNDEFINED # Install protocol
>>    ## CONSUMES ## UNDEFINED # SmiHandlerRegister
>>    gSmmPerformanceExProtocolGuid
>> +  ## PRODUCES ## SystemTable
>> +  gPerformanceProtocolGuid
>>
>>  [Pcd]
>>    gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask    ##
>> CONSUMES
>> --
>> 2.6.3.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel