public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marc W Chen" <marc.w.chen@intel.com>
To: "Gao, Liming" <liming.gao@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl definition.
Date: Thu, 1 Aug 2019 08:52:55 +0000	[thread overview]
Message-ID: <AF60760DA41C634EBADDE512AA3D4E537E306646@PGSMSX108.gar.corp.intel.com> (raw)
In-Reply-To: 4A89E2EF3DFEDB4C8BFDE51014F606A14E4C7400@SHSMSX104.ccr.corp.intel.com

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.gao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray
> <ray.ni@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.chen@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray
> > <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl
> > definition.
> >
> > Marc:
> >   The change is good. Reviewed-by: Liming Gao <liming.gao@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.kinney@intel.com>; Gao, Liming
> > ><liming.gao@intel.com>; Ni, Ray <ray.ni@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.kinney@intel.com>
> > >Cc: Liming Gao <liming.gao@intel.com>
> > >Cc: Ray Ni <ray.ni@intel.com>
> > >Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2023
> > >Signed-off-by: Marc W Chen <marc.w.chen@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
> > >
> > >
> > >


  parent reply	other threads:[~2019-08-01  8:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-29  4:25 [PATCH] MdePkg: Add MmAccess and MmControl definition Marc W Chen
2019-08-01  8:42 ` [edk2-devel] " Liming Gao
2019-08-01  8:48   ` Marc W Chen
2019-08-02  2:12     ` Laszlo Ersek
2019-08-01  8:52   ` Marc W Chen [this message]
2019-08-01 10:04     ` Ni, Ray
2019-08-01 10:15       ` Marc W Chen
2019-08-02  2:14         ` Laszlo Ersek
2019-08-06  8:56           ` Marc W Chen
2019-08-06 10:08             ` Ni, Ray
2019-08-07  4:52               ` Liming Gao
2019-08-07 16:10               ` Laszlo Ersek
2019-08-10  3:32                 ` Marc W Chen
2019-08-10  3:34                   ` Liming Gao
     [not found]                   ` <15B9725390DC0000.29320@groups.io>
2019-08-13  9:19                     ` Liming Gao
2019-08-14  9:53                       ` Marc W Chen
2019-08-18  5:20                         ` Ni, Ray
2019-08-18  5:21                         ` Ni, Ray

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=AF60760DA41C634EBADDE512AA3D4E537E306646@PGSMSX108.gar.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