From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4A7FE211AEA43 for ; Wed, 9 Jan 2019 22:54:44 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jan 2019 22:54:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,460,1539673200"; d="scan'208";a="124898508" Received: from shzintpr01.sh.intel.com (HELO [10.7.209.21]) ([10.239.4.80]) by orsmga002.jf.intel.com with ESMTP; 09 Jan 2019 22:54:41 -0800 To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: Hao Wu , Michael D Kinney , Laszlo Ersek , Liming Gao , star.zeng@intel.com References: <20190103182825.32231-1-ard.biesheuvel@linaro.org> <20190103182825.32231-4-ard.biesheuvel@linaro.org> <9bfb4d7c-3d4e-c05c-49a1-1959ddc902e3@intel.com> From: "Zeng, Star" Message-ID: <02657425-ab44-07e2-f35a-81cd12379a2b@intel.com> Date: Thu, 10 Jan 2019 14:54:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <9bfb4d7c-3d4e-c05c-49a1-1959ddc902e3@intel.com> Subject: Re: [PATCH 2/6] MdePkg: implement MmServicesTableLib based on traditional SMM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Jan 2019 06:54:44 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Hi Ard, Another minor feedback. On 2019/1/10 14:14, Zeng, Star wrote: > Hi Ard, > > Some minor feedback added inline. > > On 2019/1/4 2:28, Ard Biesheuvel wrote: >> The definitions of the rebranded MM protocol stack were chosen such >> that the existing SMM based core drivers can be reused. So let's >> implement MmServicesTableLib based on gEfiMmBaseProtocolGuid, which >> is simply gEfiSmmBase2ProtocolGuid under the hood. > > Good commit log. > >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >>   MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c   | 63 >> ++++++++++++++++++++ >>   MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf | 45 >> ++++++++++++++ >>   MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni | 22 +++++++ >>   MdePkg/MdePkg.dsc                                        |  1 + >>   4 files changed, 131 insertions(+) >> >> diff --git a/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c >> b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c >> new file mode 100644 >> index 000000000000..f41d25e59805 >> --- /dev/null >> +++ b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c >> @@ -0,0 +1,63 @@ >> +/** @file >> +  MM Services Table Library. >> + >> +  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>> +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.
>> +  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. >> + >> +**/ >> + >> +#include >> +#include >> +#include >> +#include >> + >> +EFI_MM_SYSTEM_TABLE   *gMmst             = NULL; >> + >> +/** >> +  The constructor function caches the pointer of SMM Services Table. Use "MM" instead of "SMM" here? Thanks, Star >> + >> +  @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 EFI_SUCCESS. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +MmServicesTableLibConstructor ( >> +  IN EFI_HANDLE        ImageHandle, >> +  IN EFI_SYSTEM_TABLE  *SystemTable >> +  ) >> +{ >> +  EFI_STATUS              Status; >> +  EFI_MM_BASE_PROTOCOL    *InternalMmBase; >> + >> +  InternalMmBase = NULL; >> +  // >> +  // Retrieve MM Base Protocol,  Do not use gBS from >> UefiBootServicesTableLib on purpose >> +  // to prevent inclusion of gBS, gST, and gImageHandle from SMM >> Drivers unless the >> +  // MM driver explicity declares that dependency. >> +  // >> +  Status = SystemTable->BootServices->LocateProtocol ( >> +                                        &gEfiMmBaseProtocolGuid, >> +                                        NULL, >> +                                        (VOID **)&InternalMmBase >> +                                        ); >> +  ASSERT_EFI_ERROR (Status); >> +  ASSERT (InternalMmBase != NULL); >> + >> +  // >> +  // We are in MM, retrieve the pointer to MM System Table >> +  // >> +  InternalMmBase->GetMmstLocation (InternalMmBase, &gMmst); >> +  ASSERT (gMmst != NULL); >> + >> +  return EFI_SUCCESS; >> +} >> diff --git a/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf >> b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf >> new file mode 100644 >> index 000000000000..4418cc2f1464 >> --- /dev/null >> +++ b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf >> @@ -0,0 +1,45 @@ >> +## @file >> +# MM Services Table Library. >> +# >> +# Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2018, Linaro, Ltd. All rights reserved.
>> +# >> +#  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. >> +# >> +# >> +## >> + >> +[Defines] >> +  INF_VERSION                    = 0x0001001B >> +  BASE_NAME                      = MmServicesTableLib >> +  MODULE_UNI_FILE                = MmServicesTableLib.uni >> +  FILE_GUID                      = 9508ECFD-66D1-4B4C-9415-F25F0FFF9E93 >> +  MODULE_TYPE                    = DXE_SMM_DRIVER >> +  VERSION_STRING                 = 1.0 >> +  LIBRARY_CLASS                  = MmServicesTableLib|DXE_SMM_DRIVER >> +  PI_SPECIFICATION_VERSION       = 0x00010032 >> +  CONSTRUCTOR                    = MmServicesTableLibConstructor >> + >> +# >> +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64 >> +# >> + >> +[Sources] >> +  MmServicesTableLib.c >> + >> +[Packages] >> +  MdePkg/MdePkg.dec >> + >> +[LibraryClasses] >> +  DebugLib >> + >> +[Protocols] >> +  gEfiMmBaseProtocolGuid                      ## CONSUMES >> + >> +[Depex] >> +  gEfiMmBaseProtocolGuid >> diff --git a/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni >> b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni >> new file mode 100644 >> index 000000000000..2b2c7e14024c >> --- /dev/null >> +++ b/MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni >> @@ -0,0 +1,22 @@ >> +// /** @file >> +// MM Services Table Library. >> +// >> +// MM Services Table Library. > > How about adding "It implements MmServicesTableLib based on > gEfiMmBaseProtocolGuid, which is simply gEfiSmmBase2ProtocolGuid under > the hood." here to make the implementation more clear? > > With Jian's feedback and this handled, Reviewed-by: Star Zeng > . > > Thanks, > Star > >> +// >> +// Copyright (c) 2009 - 2014, Intel Corporation. All rights >> reserved.
>> +// Copyright (c) 2018, Linaro, Ltd. All rights reserved.
>> +// >> +// 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. >> +// >> +// **/ >> + >> + >> +#string STR_MODULE_ABSTRACT             #language en-US "MM Services >> Table Library" >> + >> +#string STR_MODULE_DESCRIPTION          #language en-US "MM Services >> Table Library." >> + >> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc >> index d43ffe4deb49..428b14e6a4e0 100644 >> --- a/MdePkg/MdePkg.dsc >> +++ b/MdePkg/MdePkg.dsc >> @@ -141,6 +141,7 @@ [Components.IA32, Components.X64] >>     MdePkg/Library/BaseRngLib/BaseRngLib.inf >>     MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf >>     MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf >> +  MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf >>   [Components.EBC] >>     MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf >>