From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: ray.ni@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Thu, 01 Aug 2019 03:04:14 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Aug 2019 03:04:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,333,1559545200"; d="scan'208";a="172869099" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga008.fm.intel.com with ESMTP; 01 Aug 2019 03:04:14 -0700 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 1 Aug 2019 03:04:13 -0700 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 1 Aug 2019 03:04:13 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.112]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.215]) with mapi id 14.03.0439.000; Thu, 1 Aug 2019 18:04:11 +0800 From: "Ni, Ray" To: "Chen, Marc W" , "Gao, Liming" , "devel@edk2.groups.io" CC: "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl definition. Thread-Topic: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl definition. Thread-Index: AQHVRcW8y5xmct2vzUm4pgTkW8tdYqbl/g2wgAABaiCAAAEfIIAAE/uw Date: Thu, 1 Aug 2019 10:04:11 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C267BB4@SHSMSX104.ccr.corp.intel.com> References: <20190729042556.17692-1-marc.w.chen@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E4C7400@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Marc, Is the purpose to avoid platform code update with the wrapper header file = in MdeModulePkg? Certain platforms they may require to not depend on MdeModulePkg but just = depend on MdePkg. Having SmmAccess.h in MdeModulePkg doesn't help. It also bring confusing. Thanks, Ray > -----Original Message----- > From: Chen, Marc W > Sent: Thursday, August 1, 2019 4:53 PM > To: Gao, Liming ; devel@edk2.groups.io > Cc: Kinney, Michael D ; Ni, Ray > > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl > definition. >=20 > Hi Liming >=20 > Another thought, do you think it is ok to keep SmmAccess.h in > MdeModulePkg? We can use #define and typedef to convert MmAccess to > SmmAccess, just like current SmmAccess Protocol in MdePkg. >=20 > Thanks, > Marc >=20 > > -----Original Message----- > > From: Chen, Marc W > > Sent: Thursday, August 1, 2019 4:48 PM > > To: Gao, Liming ; devel@edk2.groups.io > > Cc: Kinney, Michael D ; Ni, Ray > > > > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and > MmControl > > definition. > > > > Hi Liming > > > > Since there are multiple packages are still using SmmAccess.h, we need > > to change all packages to use MmAccess.h instead of SmmAccess.h first, > > then clean up SmmAccess.h from MdeModulePkg will be the last step. > > Below are the packages that has "include " for your > > reference. > > * Edk2 repo: > > o MdeModulePkg > > o OvmfPkg > > o UefiCpuPkg > > * Edk2Platform repo: > > o MinPlatformPkg > > o QuarkSocPkg > > > > Thanks, > > Marc > > > > > -----Original Message----- > > > From: Gao, Liming > > > Sent: Thursday, August 1, 2019 4:42 PM > > > To: devel@edk2.groups.io; Chen, Marc W > > > Cc: Kinney, Michael D ; Ni, Ray > > > > > > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and > MmControl > > > definition. > > > > > > Marc: > > > The change is good. Reviewed-by: Liming Gao > > > > > > Besides, I see BZ also mention to remove the one in MdeModulePkg. > > > Have you the following patches for the change in MdeModulePkg? > > > > > > Thanks > > > Liming > > > >-----Original Message----- > > > >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > > >Of Marc W Chen > > > >Sent: Monday, July 29, 2019 12:26 PM > > > >To: devel@edk2.groups.io > > > >Cc: Kinney, Michael D ; Gao, Liming > > > >; Ni, Ray > > > >Subject: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl > > > >definition. > > > > > > > >EFI MmAccess and MmControl PPIs are defined in the PI 1.5 > specification. > > > > > > > >Cc: Michael D Kinney > > > >Cc: Liming Gao > > > >Cc: Ray Ni > > > >Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2023 > > > >Signed-off-by: Marc W Chen > > > >--- > > > > MdePkg/Include/Ppi/MmAccess.h | 155 > > > >+++++++++++++++++++++++++++++++++++++++++ > > > > MdePkg/Include/Ppi/MmControl.h | 90 > ++++++++++++++++++++++++ > > > > MdePkg/MdePkg.dec | 6 ++ > > > > 3 files changed, 251 insertions(+) create mode 100644 > > > > MdePkg/Include/Ppi/MmAccess.h create mode 100644 > > > > MdePkg/Include/Ppi/MmControl.h > > > > > > > >diff --git a/MdePkg/Include/Ppi/MmAccess.h > > > >b/MdePkg/Include/Ppi/MmAccess.h new file mode 100644 index > > > >0000000000..636e7288a0 > > > >--- /dev/null > > > >+++ b/MdePkg/Include/Ppi/MmAccess.h > > > >@@ -0,0 +1,155 @@ > > > >+/** @file > > > >+ EFI MM Access PPI definition. > > > >+ > > > >+ This PPI is used to control the visibility of the MMRAM on the > platform. > > > >+ The EFI_PEI_MM_ACCESS_PPI abstracts the location and > > > >+ characteristics > > of > > > >MMRAM. The > > > >+ principal functionality found in the memory controller includes > > > >+ the > > > following: > > > >+ - Exposing the MMRAM to all non-MM agents, or the "open" state > > > >+ - Shrouding the MMRAM to all but the MM agents, or the "closed" > > > >+ state > > > >+ - Preserving the system integrity, or "locking" the MMRAM, such > > > >+ that > > the > > > >settings cannot be > > > >+ perturbed by either boot service or runtime agents > > > >+ > > > >+ Copyright (c) 2019, Intel Corporation. All rights reserved.
> > > >+ SPDX-License-Identifier: BSD-2-Clause-Patent > > > >+ > > > >+ @par Revision Reference: > > > >+ This PPI is introduced in PI Version 1.5. > > > >+ > > > >+**/ > > > >+ > > > >+#ifndef _MM_ACCESS_PPI_H_ > > > >+#define _MM_ACCESS_PPI_H_ > > > >+ > > > >+#define EFI_PEI_MM_ACCESS_PPI_GUID \ > > > >+ { 0x268f33a9, 0xcccd, 0x48be, { 0x88, 0x17, 0x86, 0x5, 0x3a, > > > >+0xc3, 0x2e, > > > >0xd6 }} > > > >+ > > > >+typedef struct _EFI_PEI_MM_ACCESS_PPI EFI_PEI_MM_ACCESS_PPI; > > > >+ > > > >+/** > > > >+ Opens the MMRAM area to be accessible by a PEIM. > > > >+ > > > >+ This function "opens" MMRAM so that it is visible while not > > > >+ inside of > > MM. > > > >The function should > > > >+ return EFI_UNSUPPORTED if the hardware does not support hiding > > > >+ of > > > >MMRAM. The function > > > >+ should return EFI_DEVICE_ERROR if the MMRAM configuration is > locked. > > > >+ > > > >+ @param PeiServices An indirect pointer to the PEI Se= rvices > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instanc= e. > > > >+ @param DescriptorIndex The region of MMRAM to Open. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support openi= ng > > > and > > > >closing of MMRAM. > > > >+ @retval EFI_DEVICE_ERROR MMRAM cannot be opened, perhaps > > > >because it is locked. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *EFI_PEI_MM_OPEN)( > > > >+ IN EFI_PEI_SERVICES **PeiServices, > > > >+ IN EFI_PEI_MM_ACCESS_PPI *This, > > > >+ IN UINTN DescriptorIndex > > > >+ ); > > > >+ > > > >+/** > > > >+ Inhibits access to the MMRAM. > > > >+ > > > >+ This function "closes" MMRAM so that it is not visible while > > > >+ outside of > > > MM. > > > >The function should > > > >+ return EFI_UNSUPPORTED if the hardware does not support hiding > > > >+ of > > > >MMRAM. > > > >+ > > > >+ @param PeiServices An indirect pointer to the PEI Se= rvices > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instanc= e. > > > >+ @param DescriptorIndex The region of MMRAM to Close. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support openi= ng > > > and > > > >closing of MMRAM. > > > >+ @retval EFI_DEVICE_ERROR MMRAM cannot be closed. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *EFI_PEI_MM_CLOSE)( > > > >+ IN EFI_PEI_SERVICES **PeiServices, > > > >+ IN EFI_PEI_MM_ACCESS_PPI *This, > > > >+ IN UINTN DescriptorIndex > > > >+ ); > > > >+ > > > >+/** > > > >+ This function prohibits access to the MMRAM region. This > > > >+function is > > > usually > > > >implemented such > > > >+ that it is a write-once operation. > > > >+ > > > >+ @param PeiServices An indirect pointer to the PEI Se= rvices > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instanc= e. > > > >+ @param DescriptorIndex The region of MMRAM to Lock. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support openi= ng > > > and > > > >closing of MMRAM. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *EFI_PEI_MM_LOCK)( > > > >+ IN EFI_PEI_SERVICES **PeiServices, > > > >+ IN EFI_PEI_MM_ACCESS_PPI *This, > > > >+ IN UINTN DescriptorIndex > > > >+ ); > > > >+ > > > >+/** > > > >+ Queries the memory controller for the possible regions that will > > > >+support > > > >MMRAM. > > > >+ > > > >+ This function describes the MMRAM regions. > > > >+ This data structure forms the contract between the MM_ACCESS and > > > >MM_IPL drivers. There is an > > > >+ ambiguity when any MMRAM region is remapped. For example, on > > some > > > >chipsets, some MMRAM > > > >+ regions can be initialized at one physical address but is later > > > >+ accessed at > > > >another processor address. > > > >+ There is currently no way for the MM IPL driver to know that it > > > >+ must use > > > >two different addresses > > > >+ depending on what it is trying to do. As a result, initial > > > >+ configuration and > > > >loading can use the > > > >+ physical address PhysicalStart while MMRAM is open. However, > > > >+ once > > the > > > >region has been > > > >+ closed and needs to be accessed by agents in MM, the CpuStart > > > >+ address > > > >must be used. > > > >+ This PPI publishes the available memory that the chipset can > > > >+ shroud for > > > the > > > >use of installing code. > > > >+ These regions serve the dual purpose of describing which regions > > > >+ have > > > >been open, closed, or locked. > > > >+ In addition, these regions may include overlapping memory > > > >+ ranges, > > > >depending on the chipset > > > >+ implementation. The latter might include a chipset that supports > > > >+ T-SEG, > > > >where memory near the top > > > >+ of the physical DRAM can be allocated for MMRAM too. > > > >+ The key thing to note is that the regions that are described by > > > >+ the PPI > > are > > > a > > > >subset of the capabilities > > > >+ of the hardware. > > > >+ > > > >+ @param PeiServices An indirect pointer to the PEI Se= rvices > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instanc= e. > > > >+ @param MmramMapSize A pointer to the size, in bytes, = of the > > > >MmramMemoryMap buffer. On input, this value is > > > >+ the size of the buffer that is > > > >+ allocated by the caller. On > > > >output, it is the size of the > > > >+ buffer that was returned by the > > > >+ firmware if the buffer > > > was > > > >large enough, or, if the > > > >+ buffer was too small, the size of > > > >+ the buffer that is > > > needed to > > > >contain the map. > > > >+ @param MmramMap A pointer to the buffer in which > firmware > > > >places the current memory map. The map is > > > >+ an array of EFI_MMRAM_DESCRIPTORs > > > >+ > > > >+ @retval EFI_SUCCESS The chipset supported the given r= esource. > > > >+ @retval EFI_BUFFER_TOO_SMALL The MmramMap parameter was > too > > > >small. The current > > > >+ buffer size needed to hold the > > > >+ memory map is > > returned > > > in > > > >+ MmramMapSize. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *EFI_PEI_MM_CAPABILITIES)( > > > >+ IN EFI_PEI_SERVICES **PeiServices, > > > >+ IN EFI_PEI_MM_ACCESS_PPI *This, > > > >+ IN OUT UINTN *MmramMapSize, > > > >+ IN OUT EFI_MMRAM_DESCRIPTOR *MmramMap > > > >+ ); > > > >+ > > > >+/// > > > >+/// EFI MM Access PPI is used to control the visibility of the > > > >+MMRAM on > > > the > > > >platform. > > > >+/// It abstracts the location and characteristics of MMRAM. The > > > >+platform > > > >should report > > > >+/// all MMRAM via EFI_PEI_MM_ACCESS_PPI. The expectation is that > > > >+the > > > >north bridge or > > > >+/// memory controller would publish this PPI. > > > >+/// > > > >+struct _EFI_PEI_MM_ACCESS_PPI { > > > >+ EFI_PEI_MM_OPEN Open; > > > >+ EFI_PEI_MM_CLOSE Close; > > > >+ EFI_PEI_MM_LOCK Lock; > > > >+ EFI_PEI_MM_CAPABILITIES GetCapabilities; > > > >+ BOOLEAN LockState; > > > >+ BOOLEAN OpenState; > > > >+}; > > > >+ > > > >+extern EFI_GUID gEfiPeiMmAccessPpiGuid; > > > >+ > > > >+#endif > > > >diff --git a/MdePkg/Include/Ppi/MmControl.h > > > >b/MdePkg/Include/Ppi/MmControl.h new file mode 100644 index > > > >0000000000..983ed95cd5 > > > >--- /dev/null > > > >+++ b/MdePkg/Include/Ppi/MmControl.h > > > >@@ -0,0 +1,90 @@ > > > >+/** @file > > > >+ EFI MM Control PPI definition. > > > >+ > > > >+ This PPI is used initiate synchronous MMI activations. This PPI > > > >+ could be > > > >published by a processor > > > >+ driver to abstract the MMI IPI or a driver which abstracts the > > > >+ ASIC that > > is > > > >supporting the APM port. > > > >+ Because of the possibility of performing MMI IPI transactions, > > > >+ the ability > > > to > > > >generate this event > > > >+ from a platform chipset agent is an optional capability for both > > > >+ IA-32 > > and > > > >x64-based systems. > > > >+ > > > >+ Copyright (c) 2019, Intel Corporation. All rights reserved.
> > > >+ SPDX-License-Identifier: BSD-2-Clause-Patent > > > >+ > > > >+ @par Revision Reference: > > > >+ This PPI is introduced in PI Version 1.5. > > > >+ > > > >+**/ > > > >+ > > > >+ > > > >+#ifndef _MM_CONTROL_PPI_H_ > > > >+#define _MM_CONTROL_PPI_H_ > > > >+ > > > >+#define EFI_PEI_MM_CONTROL_PPI_GUID \ > > > >+ { 0x61c68702, 0x4d7e, 0x4f43, 0x8d, 0xef, 0xa7, 0x43, 0x5, 0xce, > > > >+0x74, > > > 0xc5 } > > > >+ > > > >+typedef struct _EFI_PEI_MM_CONTROL_PPI > EFI_PEI_MM_CONTROL_PPI; > > > >+ > > > >+/** > > > >+ Invokes PPI activation from the PI PEI environment. > > > >+ > > > >+ @param PeiServices An indirect pointer to the PEI Ser= vices Table > > > >published by the PEI Foundation. > > > >+ @param This The PEI_MM_CONTROL_PPI instance. > > > >+ @param ArgumentBuffer The value passed to the MMI handle= r. > > This > > > >value corresponds to the > > > >+ SwMmiInputValue in the > > > >+ RegisterContext parameter > > for > > > the > > > >Register() > > > >+ function in the > > > >+ EFI_MM_SW_DISPATCH_PROTOCOL > > and > > > in > > > >the Context parameter > > > >+ in the call to the DispatchFunctio= n > > > >+ @param ArgumentBufferSize The size of the data passed in > > > >ArgumentBuffer or NULL if ArgumentBuffer is NULL. > > > >+ @param Periodic An optional mechanism to periodica= lly repeat > > > >activation. > > > >+ @param ActivationInterval An optional parameter to repeat at= this > > > period > > > >one > > > >+ time or, if the Periodic Boolean i= s set, periodically. > > > >+ > > > >+ @retval EFI_SUCCESS The MMI has been engendered. > > > >+ @retval EFI_DEVICE_ERROR The timing is unsupported. > > > >+ @retval EFI_INVALID_PARAMETER The activation period is > unsupported. > > > >+ @retval EFI_NOT_STARTED The MM base service has not been > > > initialized. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *EFI_PEI_MM_ACTIVATE) ( > > > >+ IN EFI_PEI_SERVICES **PeiServices= , > > > >+ IN EFI_PEI_MM_CONTROL_PPI * This, > > > >+ IN OUT INT8 *ArgumentBuff= er OPTIONAL, > > > >+ IN OUT UINTN *ArgumentBuff= erSize OPTIONAL, > > > >+ IN BOOLEAN Periodic OPTI= ONAL, > > > >+ IN UINTN ActivationInt= erval OPTIONAL > > > >+ ); > > > >+ > > > >+/** > > > >+ Clears any system state that was created in response to the Trig= ger() > call. > > > >+ > > > >+ @param PeiServices General purpose services available= to every > > > PEIM. > > > >+ @param This The PEI_MM_CONTROL_PPI instance. > > > >+ @param Periodic Optional parameter to repeat at th= is period > > one > > > >+ time or, if the Periodic Boolean i= s set, periodically. > > > >+ > > > >+ @retval EFI_SUCCESS The MMI has been engendered. > > > >+ @retval EFI_DEVICE_ERROR The source could not be cleared. > > > >+ @retval EFI_INVALID_PARAMETER The service did not support the > > > Periodic > > > >input argument. > > > >+ > > > >+**/ > > > >+typedef > > > >+EFI_STATUS > > > >+(EFIAPI *PEI_MM_DEACTIVATE) ( > > > >+ IN EFI_PEI_SERVICES **PeiServices, > > > >+ IN EFI_PEI_MM_CONTROL_PPI * This, > > > >+ IN BOOLEAN Periodic OPTIONAL > > > >+ ); > > > >+ > > > >+/// > > > >+/// The EFI_PEI_MM_CONTROL_PPI is produced by a PEIM. It > provides > > an > > > >abstraction of the > > > >+/// platform hardware that generates an MMI. There are often I/O > > > >+ports > > > >that, when accessed, will > > > >+/// generate the MMI. Also, the hardware optionally supports the > > periodic > > > >generation of these signals. > > > >+/// > > > >+struct _PEI_MM_CONTROL_PPI { > > > >+ PEI_MM_ACTIVATE Trigger; > > > >+ PEI_MM_DEACTIVATE Clear; > > > >+}; > > > >+ > > > >+extern EFI_GUID gEfiPeiMmControlPpiGuid; > > > >+ > > > >+#endif > > > >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > >b382efd578..34e0f39395 100644 > > > >--- a/MdePkg/MdePkg.dec > > > >+++ b/MdePkg/MdePkg.dec > > > >@@ -925,6 +925,12 @@ > > > > ## Include/Ppi/SecHobData.h > > > > gEfiSecHobDataPpiGuid =3D { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, > > > >0xee, > > 0xf5, > > > >0x99, 0x9a, 0xc1, 0xb7, 0x1f } } > > > > > > > >+ ## Include/Ppi/MmAccess.h > > > >+ gEfiPeiMmAccessPpiGuid =3D { 0x268f33a9, 0xcccd, 0x48b= e, { 0x88, > > > 0x17, > > > >0x86, 0x5, 0x3a, 0xc3, 0x2e, 0xd6 }} > > > >+ > > > >+ ## Include/Ppi/MmControl.h > > > >+ gEfiPeiMmControlPpiGuid =3D { 0x61c68702, 0x4d7e, 0x4f4= 3, { 0x8d, > > > 0xef, > > > >0xa7, 0x43, 0x5, 0xce, 0x74, 0xc5 }} > > > >+ > > > > # > > > > # PPIs defined in PI 1.7. > > > > # > > > >-- > > > >2.16.2.windows.1 > > > > > > > > > > > >