On Mon, 26 Jun 2023 at 22:00, Kun Qin <kuqi...@gmail.com> wrote:
From: Kun Qin <ku...@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4464
This change introduced the MM communicate support in PEI phase for ARM
based platforms. Similar to the DXE counterpart, `PcdMmBufferBase` is
used as communicate buffer and SMC will be invoked to communicate to
TrustZone when MMI is requested.
Cc: Leif Lindholm <quic_llind...@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
Cc: Sami Mujawar <sami.muja...@arm.com>
Co-authored-by: Ronny Hansen <hansen.ro...@microsoft.com>
Co-authored-by: Shriram Masanamuthu Chinnathurai <shrira...@microsoft.com>
Co-authored-by: Preshit Harlikar <pharli...@microsoft.com>
Signed-off-by: Kun Qin <ku...@microsoft.com>
---
Notes:
v2:
- Adjustment to CommSize checks [Sami]
- Added more debug prints for error returns.
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c | 212
++++++++++++++++++++
ArmPkg/ArmPkg.dsc | 2 +
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h | 76 +++++++
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf | 41 ++++
4 files changed, 331 insertions(+)
diff --git a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
new file mode 100644
index 000000000000..8661f0c8ee49
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
@@ -0,0 +1,212 @@
+/** @file -- MmCommunicationPei.c
+ Provides an interface to send MM request in PEI
+
+ Copyright (c) 2016-2021, Arm Limited. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "MmCommunicationPei.h"
+
+//
+// Module globals
+//
STATIC CONST
+EFI_PEI_MM_COMMUNICATION_PPI mPeiMmCommunication = {
+ MmCommunicationPeim
+};
+
STATIC CONST
+EFI_PEI_PPI_DESCRIPTOR mPeiMmCommunicationPpi = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEfiPeiMmCommunicationPpiGuid,
+ &mPeiMmCommunication
+};
+
+/**
+ Entry point of PEI MM Communication driver
+
+ @param FileHandle Handle of the file being invoked.
+ Type EFI_PEI_FILE_HANDLE is defined in
FfsFindNextFile().
+ @param PeiServices General purpose services available to every PEIM.
+
+ @retval EFI_SUCCESS If the interface could be successfully installed
+ @retval Others Returned from PeiServicesInstallPpi()
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationPeiInitialize (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ return PeiServicesInstallPpi (&mPeiMmCommunicationPpi);
+}
+
+/**
+ MmCommunicationPeim
+ Communicates with a registered handler.
+ This function provides a service to send and receive messages from a
registered UEFI service during PEI.
+
+ @param[in] This The EFI_PEI_MM_COMMUNICATION_PPI instance.
+ @param[in, out] CommBuffer Pointer to the data buffer
+ @param[in, out] CommSize The size of the data buffer being passed in.
On exit, the
+ size of data being returned. Zero if the
handler does not
+ wish to reply with any data.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER CommBuffer or CommSize was NULL, or
*CommSize does not
+ match MessageLength + sizeof
(EFI_MM_COMMUNICATE_HEADER).
+ @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM
implementation.
+ If this error is returned, the MessageLength
field
+ in the CommBuffer header or the integer
pointed by
+ CommSize, are updated to reflect the maximum
payload
+ size the implementation can accommodate.
+ @retval EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize
parameter,
+ if not omitted, are in address range that
cannot be
+ accessed by the MM environment.
+**/
This can be STATIC as well - no need to declare it in the header file
either (but you may need to reorder this file to avoid the need for a
forward declaration)
+EFI_STATUS
+EFIAPI
+MmCommunicationPeim (
+ IN CONST EFI_PEI_MM_COMMUNICATION_PPI *This,
+ IN OUT VOID *CommBuffer,
+ IN OUT UINTN *CommSize
+ )
+{
+ EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
+ EFI_MM_COMMUNICATE_HEADER *TempCommHeader;
+ ARM_SMC_ARGS CommunicateSmcArgs;
+ EFI_STATUS Status;
+ UINTN BufferSize;
+
+ ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
+
+ // Check that our static buffer is looking good.
+ // We are using PcdMmBufferBase to transfer variable data.
+ // We are not using the full size of the buffer since there is a cost
+ // of copying data between Normal and Secure World.
+ ASSERT (PcdGet64 (PcdMmBufferSize) > 0 && PcdGet64 (PcdMmBufferBase) != 0);
+
Please split these up into two separate ASSERT()s
+ //
+ // Check parameters
+ //
+ if ((CommBuffer == NULL) || (CommSize == NULL)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a One or more incoming pointers are NULL %p and %p!\n",
+ __func__,
+ CommBuffer,
+ CommSize
+ ));
Please drop the DEBUG() and use an ASSERT() for each parameter.
(Asserts are self documenting)
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // If the length of the CommBuffer is 0 then return the expected length.
+ // This case can be used by the consumer of this driver to find out the
+ // max size that can be used for allocating CommBuffer.
+ if ((*CommSize == 0) || (*CommSize > (UINTN)PcdGet64 (PcdMmBufferSize))) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a Invalid CommSize value 0x%llx!\n",
+ __func__,
+ *CommSize
+ ));
+ *CommSize = (UINTN)PcdGet64 (PcdMmBufferSize);
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ // Given CommBuffer is not NULL here, we use it to test the legitimacy of
CommSize.
+ TempCommHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)CommBuffer;
+
+ // CommBuffer is a mandatory parameter. Hence, Rely on
+ // MessageLength + Header to ascertain the
+ // total size of the communication payload rather than
+ // rely on optional CommSize parameter
+ BufferSize = TempCommHeader->MessageLength +
+ sizeof (TempCommHeader->HeaderGuid) +
+ sizeof (TempCommHeader->MessageLength);
+
+ //
+ // If CommSize is supplied it must match MessageLength + sizeof
(EFI_MM_COMMUNICATE_HEADER);
+ //
+ if (*CommSize != BufferSize) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a Unexpected CommSize value, has: 0x%llx vs. expected: 0x%llx!\n",
+ __func__,
+ *CommSize,
+ BufferSize
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Now we know that the size is something we can handle, copy it over to the
designated comm buffer.
+ CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)(PcdGet64
(PcdMmBufferBase));
+
+ CopyMem ((VOID *)CommunicateHeader, CommBuffer, *CommSize);
No need for a (VOID*) cast
+
+ // SMC Function ID
+ CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
+
+ // Cookie
+ CommunicateSmcArgs.Arg1 = 0;
+
+ // comm_buffer_address (64-bit physical address)
+ CommunicateSmcArgs.Arg2 = (UINTN)CommunicateHeader;
+
+ // comm_size_address (not used, indicated by setting to zero)
+ CommunicateSmcArgs.Arg3 = 0;
+
+ // Call the Standalone MM environment.
+ ArmCallSmc (&CommunicateSmcArgs);
+
+ switch (CommunicateSmcArgs.Arg0) {
+ case ARM_SMC_MM_RET_SUCCESS:
+ // On successful return, the size of data being returned is inferred from
+ // MessageLength + Header.
+ BufferSize = CommunicateHeader->MessageLength +
+ sizeof (CommunicateHeader->HeaderGuid) +
+ sizeof (CommunicateHeader->MessageLength);
+ if (BufferSize > (UINTN)PcdGet64 (PcdMmBufferSize)) {
+ // Something bad has happened, we should have landed in
ARM_SMC_MM_RET_NO_MEMORY
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a Returned buffer exceeds communication buffer limit. Has: 0x%llx vs.
max: 0x%llx!\n",
+ __func__,
+ BufferSize,
+ (UINTN)PcdGet64 (PcdMmBufferSize)
+ ));
+ Status = EFI_BAD_BUFFER_SIZE;
+ break;
+ }
+
+ CopyMem (CommBuffer, (VOID *)CommunicateHeader, BufferSize);
No need for the (VOID *) cast
+ if (CommSize != NULL) {
Can CommSize be NULL?
+ *CommSize = BufferSize;
+ }
+
+ Status = EFI_SUCCESS;
+ break;
+
+ case ARM_SMC_MM_RET_INVALID_PARAMS:
+ Status = EFI_INVALID_PARAMETER;
+ break;
+
+ case ARM_SMC_MM_RET_DENIED:
+ Status = EFI_ACCESS_DENIED;
+ break;
+
+ case ARM_SMC_MM_RET_NO_MEMORY:
+ // Unexpected error since the CommSize was checked for zero length
+ // prior to issuing the SMC
+ Status = EFI_OUT_OF_RESOURCES;
+ ASSERT (0);
+ break;
+
+ default:
+ Status = EFI_ACCESS_DENIED;
+ ASSERT (0);
+ break;
+ }
+
+ return Status;
+}
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 6b938ce8b671..4939b3d59b7f 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -162,6 +162,8 @@ [Components.common]
ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
+ ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
+
[Components.AARCH64]
ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
diff --git a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h
b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h
new file mode 100644
index 000000000000..a99baa2496a9
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h
I don't think we need this header file at all.
@@ -0,0 +1,76 @@
+/** @file -- MmCommunicationPei.h
+ Provides an interface to send MM request in PEI
+
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MM_COMMUNICATION_PEI_H_
+#define MM_COMMUNICATION_PEI_H_
+
+#include <PiPei.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/ArmSmcLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeimEntryPoint.h>
+#include <Library/PeiServicesLib.h>
+#include <Library/HobLib.h>
+
+#include <Protocol/MmCommunication.h>
+
+#include <IndustryStandard/ArmStdSmc.h>
+
+#include <Ppi/MmCommunication.h>
+
+/**
+ Entry point of PEI MM Communication driver
+
+ @param FileHandle Handle of the file being invoked.
+ Type EFI_PEI_FILE_HANDLE is defined in
FfsFindNextFile().
+ @param PeiServices General purpose services available to every PEIM.
+
+ @retval EFI_SUCCESS If the interface could be successfully installed
+ @retval Others Returned from PeiServicesInstallPpi()
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationPeiInitialize (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ );
+
+/**
+ MmCommunicationPeim
+ Communicates with a registered handler.
+ This function provides a service to send and receive messages from a
registered UEFI service during PEI.
+
+ @param[in] This The EFI_PEI_MM_COMMUNICATION_PPI instance.
+ @param[in, out] CommBuffer Pointer to the data buffer
+ @param[in, out] CommSize The size of the data buffer being passed in.
On exit, the
+ size of data being returned. Zero if the
handler does not
+ wish to reply with any data.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER CommBuffer was NULL or *CommSize does not
match
+ MessageLength + sizeof
(EFI_MM_COMMUNICATE_HEADER).
+ @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM
implementation.
+ If this error is returned, the MessageLength
field
+ in the CommBuffer header or the integer
pointed by
+ CommSize, are updated to reflect the maximum
payload
+ size the implementation can accommodate.
+ @retval EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize
parameter,
+ if not omitted, are in address range that
cannot be
+ accessed by the MM environment.
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationPeim (
+ IN CONST EFI_PEI_MM_COMMUNICATION_PPI *This,
+ IN OUT VOID *CommBuffer,
+ IN OUT UINTN *CommSize
+ );
+
+#endif /* MM_COMMUNICATION_PEI_H_ */
diff --git a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
new file mode 100644
index 000000000000..7a0adbd9bd2f
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
@@ -0,0 +1,41 @@
+## @file -- MmCommunicationPei.inf
+# PEI MM Communicate driver
+#
+# Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = MmCommunicationPei
+ FILE_GUID = 58FFB346-1B75-42C7-AD69-37C652423C1A
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ ENTRY_POINT = MmCommunicationPeiInitialize
+
+[Sources]
+ MmCommunicationPei.c
+ MmCommunicationPei.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ ArmPkg/ArmPkg.dec
+
+[LibraryClasses]
+ DebugLib
+ ArmSmcLib
+ PeimEntryPoint
+ PeiServicesLib
+ HobLib
+
+[Pcd]
+ gArmTokenSpaceGuid.PcdMmBufferBase
+ gArmTokenSpaceGuid.PcdMmBufferSize
+
+[Ppis]
+ gEfiPeiMmCommunicationPpiGuid ## PRODUCES
+
+[Depex]
+ TRUE
--
2.41.0.windows.1