Re: [PATCH] MdePkg: Add MmAccess and MmControl definition.


Laszlo Ersek
 

On 08/06/19 12:08, Ni, Ray wrote:
Laszlo,
Do you want to avoid touching the OvmfPkg due the file removal in MdeModulePkg?
My main problem with this *specific* update proposal is that the
identifiers (type names and such) that would be subject to removal were
publicly specified in earlier PI spec releases.

I don't suggest that we maintain two parallel sets of typedefs and such
in edk2. It's fine to keep the new "MM" nomenclature the main one. But
we should keep the "SMM" set of terms too, as a thin wrapper, if that's
technically feasible.

If we remove "SMM", that will not only affect OvmfPkg unpleasantly, but
a bunch of other, out-of-tree code. And the reason I suddenly care about
out-of-tree code is that such code was written against a public industry
spec (= earlier versions of the PI spec).

I don't think that the simplicity that would come from removing "SMM"
altogether, is well-balanced against the pain that it would cause for
platforms.

Earlier this was solved in the following commits:
- 07c6a47e70ba ("MdePkg: Add new definitions for Management Mode.",
2017-08-29)
- 2f208e59e4b9 ("MdePkg: Reference new definitions for Management
Mode.", 2017-08-29)

Thanks
Laszlo

I prefer to remove the files in MdeModulePkg to avoid future confusion.

Thanks,
Ray

-----Original Message-----
From: Chen, Marc W
Sent: Tuesday, August 6, 2019 4:57 PM
To: devel@edk2.groups.io; lersek@...; Ni, Ray <ray.ni@...>;
Gao, Liming <liming.gao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl
definition.

Hi Liming and Ray

Do you also agree?

Thanks,
Marc

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Friday, August 2, 2019 10:14 AM
To: devel@edk2.groups.io; Chen, Marc W <marc.w.chen@...>; Ni,
Ray <ray.ni@...>; Gao, Liming <liming.gao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

On 08/01/19 12:15, Marc W Chen wrote:
Yes, my purpose is to avoid platform code update if the package is
allowed
to use MdeModulePkg like OvmfPkg.
For those packages that cannot depend on MdeModulePkg must change
to
use new definition.

Agreed.

Thanks!
Laszlo

-----Original Message-----
From: Ni, Ray
Sent: Thursday, August 1, 2019 6:04 PM
To: Chen, Marc W <marc.w.chen@...>; Gao, Liming
<liming.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

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.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
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.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
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@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

Marc:
The change is good. Reviewed-by: Liming Gao
<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 <michael.d.kinney@...>; Gao, Liming
<liming.gao@...>; Ni, Ray <ray.ni@...>
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@...>
Cc: Liming Gao <liming.gao@...>
Cc: Ray Ni <ray.ni@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2023
Signed-off-by: Marc W Chen <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.<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





Join devel@edk2.groups.io to automatically receive all group messages.