public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: Marvin H?user <Marvin.Haeuser@outlook.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>
Subject: Re: [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.
Date: Thu, 2 Aug 2018 15:20:53 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E2C4B82@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <VI1PR0801MB1790279ADB8B89DF460F8E6980560@VI1PR0801MB1790.eurprd08.prod.outlook.com>

Marvin:
  I suggest to verify this library functionality on Standalone MM driver first, then propose how to change it to support more module type. 

Thanks
Liming
> -----Original Message-----
> From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
> Sent: Monday, July 23, 2018 9:02 PM
> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.
> 
> Best regards,
> Marvin.
> 
> > -----Original Message-----
> > From: Gao, Liming <liming.gao@intel.com>
> > Sent: Monday, July 23, 2018 8:46 AM
> > To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.
> >
> > Marvin:
> >   If this library supports standalone MM module only, I agree its source file
> > includes Standalone MM
> >   But, this library instance also supports DXE, SMM. I don't think
> > StandaloneMm name is good for them.
> 
> I called all files "DxeStandaloneMm", the same way as other libraries append environment types ("PeiDxePostCodeLib", etc).
> Did I overlook something?
> 
> >
> >   Besides, I want to know why you changes this library instance. Are there the
> > standalone MM module to depend on this AcpiTimerLib?
> 
> I wanted to try out porting a private module which happened to depend on AcpiTimerLib and thus HobLib.
> HobLib will follow once the MmServicesTableLib patch has been decided on and I will adapt others too once I run into needing them.
> 
> >
> > Thanks
> > Liming
> > >-----Original Message-----
> > >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > >Marvin H?user
> > >Sent: Monday, July 23, 2018 7:05 AM
> > >To: edk2-devel@lists.01.org
> > >Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > >Subject: [edk2] [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone
> > >MM.
> > >
> > >To support Standalone MM, the current DXE implementation, which is also
> > >used to support DXE SMM Drivers, has been modified. Its type was
> > >changed to BASE to make the constructor function generic,
> > MM_STANDALONE
> > >modules types have been added to the support list and the internal
> > >files were adapted to show support.
> > >
> > >"DxeAcpiTimerLib.inf" has not been renamed to not break packages.
> > >This might be addressed with a separate patch.
> > >
> > >Contributed-under: TianoCore Contribution Agreement 1.1
> > >Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> > >---
> > > PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.c =>
> > >DxeStandaloneMmAcpiTimerLib.c}     |  9 +++------
> > > PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >| 14 +++++++-------
> > > PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.uni =>
> > >StandaloneMmDxeAcpiTimerLib.uni} |  2 +-
> > > 3 files changed, 11 insertions(+), 14 deletions(-)
> > >
> > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >similarity index 88%
> > >rename from PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >rename to
> > >PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >index 9ed10ef2e297..784f33871d75 100644
> > >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >+++
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >@@ -13,6 +13,7 @@
> > > **/
> > >
> > > #include <PiDxe.h>
> > >+#include <PiMm.h>
> > > #include <Library/TimerLib.h>
> > > #include <Library/BaseLib.h>
> > > #include <Library/HobLib.h>
> > >@@ -78,17 +79,13 @@ InternalGetPerformanceCounterFrequency (
> > > /**
> > >   The constructor function enables ACPI IO space, and caches
> > >PerformanceCounterFrequency.
> > >
> > >-  @param  ImageHandle   The firmware allocated handle for the EFI image.
> > >-  @param  SystemTable   A pointer to the EFI System Table.
> > >-
> > >   @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
> > >
> > > **/
> > > EFI_STATUS
> > > EFIAPI
> > >-DxeAcpiTimerLibConstructor (
> > >-  IN EFI_HANDLE        ImageHandle,
> > >-  IN EFI_SYSTEM_TABLE  *SystemTable
> > >+DxeStandaloneMmAcpiTimerLibConstructor (
> > >+  VOID
> > >   )
> > > {
> > >   EFI_HOB_GUID_TYPE   *GuidHob;
> > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >index 601041c80137..f1f62247649e 100644
> > >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >@@ -1,5 +1,5 @@
> > > ## @file
> > >-#  DXE ACPI Timer Library
> > >+#  DXE and Standalone MM ACPI Timer Library
> > > #
> > > #  Provides basic timer support using the ACPI timer hardware.  The
> > >performance  #  counter features are provided by the processors time
> > >stamp counter.
> > >@@ -20,17 +20,17 @@
> > >
> > > [Defines]
> > >   INF_VERSION                    = 0x00010005
> > >-  BASE_NAME                      = DxeAcpiTimerLib
> > >+  BASE_NAME                      = DxeStandaloneMmAcpiTimerLib
> > >   FILE_GUID                      = E624B98C-845A-4b94-9B50-B20475D552B9
> > >-  MODULE_TYPE                    = DXE_DRIVER
> > >+  MODULE_TYPE                    = BASE
> > >   VERSION_STRING                 = 1.0
> > >-  LIBRARY_CLASS                  = TimerLib|DXE_CORE DXE_DRIVER
> > >DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION
> > UEFI_DRIVER SMM_CORE
> > >-  CONSTRUCTOR                    = DxeAcpiTimerLibConstructor
> > >-  MODULE_UNI_FILE                = DxeAcpiTimerLib.uni
> > >+  LIBRARY_CLASS                  = TimerLib|DXE_CORE DXE_DRIVER
> > >DXE_RUNTIME_DRIVER DXE_SMM_DRIVER MM_STANDALONE
> > UEFI_APPLICATION
> > >UEFI_DRIVER SMM_CORE MM_CORE_STANDALONE
> > >+  CONSTRUCTOR                    = DxeStandaloneMmAcpiTimerLibConstructor
> > >+  MODULE_UNI_FILE                = DxeStandaloneMmAcpiTimerLib.uni
> > >
> > > [Sources]
> > >   AcpiTimerLib.c
> > >-  DxeAcpiTimerLib.c
> > >+  DxeStandaloneMmAcpiTimerLib.c
> > >
> > > [Packages]
> > >   MdePkg/MdePkg.dec
> > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.uni
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmDxeAcpiTimerLib.u
> > ni
> > >similarity index 92%
> > >rename from PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.uni
> > >rename to
> > >PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmDxeAcpiTimerLib.uni
> > >index bf31abf81d68..6b3abb85ebc8 100644
> > >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.uni
> > >+++
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmDxeAcpiTimerLib.u
> > ni
> > >@@ -1,5 +1,5 @@
> > > // /** @file
> > >-// DXE ACPI Timer Library
> > >+// DXE and Standalone MM ACPI Timer Library
> > > //
> > > // Provides basic timer support using the ACPI timer hardware.  The
> > >performance  // counter features are provided by the processors time
> > >stamp counter.
> > >--
> > >2.18.0.windows.1
> > >
> > >_______________________________________________
> > >edk2-devel mailing list
> > >edk2-devel@lists.01.org
> > >https://lists.01.org/mailman/listinfo/edk2-devel


      reply	other threads:[~2018-08-02 15:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <3e63e0c06e38886af03bfc7a7e90ce6608f44ce8.1532300587.git.Marvin.Haeuser@outlook.com>
2018-07-22 23:05 ` [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM Marvin Häuser
2018-07-23  6:46   ` Gao, Liming
2018-07-23 13:01     ` Marvin H?user
2018-08-02 15:20       ` Gao, Liming [this message]

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=4A89E2EF3DFEDB4C8BFDE51014F606A14E2C4B82@SHSMSX104.ccr.corp.intel.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