public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	 "edk2-devel@lists.01.org" <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: [PATCH v4 1/3] MdeModulePkg: Add performance property configuration table
Date: Thu, 23 Feb 2017 17:30:05 +0000	[thread overview]
Message-ID: <CAKv+Gu_52b5YotWnfwP=qqE28d7Lw0yTDCMj1uf41ThaeUTDqg@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8E8BED@shsmsx102.ccr.corp.intel.com>

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.inf
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> index e091c62..5b89ce2 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> +++
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> @@ -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


  reply	other threads:[~2017-02-23 17:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  4:55 [PATCH v4 0/3] Remove TimerLib dependency from DP Michael Kinney
2017-02-03  4:55 ` [PATCH v4 1/3] MdeModulePkg: Add performance property configuration table Michael Kinney
2017-02-03  5:32   ` Yao, Jiewen
2017-02-23 17:30     ` Ard Biesheuvel [this message]
2017-02-24  1:32       ` Zeng, Star
2017-02-03  4:55 ` [PATCH v4 2/3] PerformancePkg Dp_App: Remove TimerLib dependency Michael Kinney
2017-02-03  5:34   ` Yao, Jiewen
2017-02-03  4:55 ` [PATCH v4 3/3] ShellPkg UefiDpLib: " Michael Kinney
2017-02-03  5:35   ` Yao, Jiewen
2017-02-03 17:36     ` Kinney, Michael D
2017-02-04  0:32       ` Yao, Jiewen
2017-02-03 17:47 ` [PATCH v4 0/3] Remove TimerLib dependency from DP Carsey, Jaben
2017-02-08 18:19   ` Andrew Fish

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu_52b5YotWnfwP=qqE28d7Lw0yTDCMj1uf41ThaeUTDqg@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox