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 <liming....@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Ni, Ray > <ray...@intel.com> > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl > definition. > > Hi Liming > > 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. > > Thanks, > Marc > > > -----Original Message----- > > From: Chen, Marc W > > Sent: Thursday, August 1, 2019 4:48 PM > > To: Gao, Liming <liming....@intel.com>; devel@edk2.groups.io > > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Ni, Ray > > <ray...@intel.com> > > 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 <Ppi/SmmAccess.h>" 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 <marc.w.c...@intel.com> > > > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Ni, Ray > > > <ray...@intel.com> > > > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and > MmControl > > > definition. > > > > > > Marc: > > > The change is good. Reviewed-by: Liming Gao <liming....@intel.com> > > > > > > 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 <michael.d.kin...@intel.com>; Gao, Liming > > > ><liming....@intel.com>; Ni, Ray <ray...@intel.com> > > > >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 <michael.d.kin...@intel.com> > > > >Cc: Liming Gao <liming....@intel.com> > > > >Cc: Ray Ni <ray...@intel.com> > > > >Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2023 > > > >Signed-off-by: Marc W Chen <marc.w.c...@intel.com> > > > >--- > > > > 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.<BR> > > > >+ 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 Services > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instance. > > > >+ @param DescriptorIndex The region of MMRAM to Open. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support opening > > > 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 Services > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instance. > > > >+ @param DescriptorIndex The region of MMRAM to Close. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support opening > > > 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 Services > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instance. > > > >+ @param DescriptorIndex The region of MMRAM to Lock. > > > >+ > > > >+ @retval EFI_SUCCESS The operation was successful. > > > >+ @retval EFI_UNSUPPORTED The system does not support opening > > > 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 Services > Table > > > >published by the PEI Foundation. > > > >+ @param This The EFI_PEI_MM_ACCESS_PPI instance. > > > >+ @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 > > > >resource. > > > >+ @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.<BR> > > > >+ 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 Services > > > >Table > > > >published by the PEI Foundation. > > > >+ @param This The PEI_MM_CONTROL_PPI instance. > > > >+ @param ArgumentBuffer The value passed to the MMI handler. > > 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 DispatchFunction > > > >+ @param ArgumentBufferSize The size of the data passed in > > > >ArgumentBuffer or NULL if ArgumentBuffer is NULL. > > > >+ @param Periodic An optional mechanism to periodically > > > >repeat > > > >activation. > > > >+ @param ActivationInterval An optional parameter to repeat at this > > > period > > > >one > > > >+ time or, if the Periodic Boolean is > > > >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 *ArgumentBuffer > > > >OPTIONAL, > > > >+ IN OUT UINTN > > > >*ArgumentBufferSize OPTIONAL, > > > >+ IN BOOLEAN Periodic OPTIONAL, > > > >+ IN UINTN ActivationInterval > > > >OPTIONAL > > > >+ ); > > > >+ > > > >+/** > > > >+ Clears any system state that was created in response to the Trigger() > 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 this > > > >period > > one > > > >+ time or, if the Periodic Boolean is > > > >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 = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, > > > >0xee, > > 0xf5, > > > >0x99, 0x9a, 0xc1, 0xb7, 0x1f } } > > > > > > > >+ ## Include/Ppi/MmAccess.h > > > >+ gEfiPeiMmAccessPpiGuid = { 0x268f33a9, 0xcccd, 0x48be, { > > > >0x88, > > > 0x17, > > > >0x86, 0x5, 0x3a, 0xc3, 0x2e, 0xd6 }} > > > >+ > > > >+ ## Include/Ppi/MmControl.h > > > >+ gEfiPeiMmControlPpiGuid = { 0x61c68702, 0x4d7e, 0x4f43, { > > > >0x8d, > > > 0xef, > > > >0xa7, 0x43, 0x5, 0xce, 0x74, 0xc5 }} > > > >+ > > > > # > > > > # PPIs defined in PI 1.7. > > > > # > > > >-- > > > >2.16.2.windows.1 > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44788): https://edk2.groups.io/g/devel/message/44788 Mute This Topic: https://groups.io/mt/32639140/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
