[PATCH v1 0/3] *** EFI_MM_COMMUNICATION_PROTOCOL ***

[PATCH v1 0/3] *** EFI_MM_COMMUNICATION_PROTOCOL ***

[Supreeth Venkatesh]

*** 
PI v1.5 Specification Volume 4 defines Management Mode Core Interface
and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
means of communicating between drivers outside of MM and MMI
handlers inside of MM.

EFI_MM_COMMUNICATION_PROTOCOL
Summary
  This protocol provides a means of communicating between drivers outside of MM and MMI
  handlers inside of MM.

GUID
  #define EFI_MM_COMMUNICATION_PROTOCOL_GUID \
  { 0xc68ed8e2, 0x9dc6, 0x4cbd, 0x9d, 0x94, 0xdb, 0x65, 0xac, 0xc5, 0xc3, 0x32 }

Prototype
  typedef struct _EFI_MM_COMMUNICATION_PROTOCOL {
  EFI_MM_COMMUNICATE Communicate;
  } EFI_MM_COMMUNICATION_PROTOCOL;

Members
Communicate
  Sends/receives a message for a registered handler. See the Communicate() 
  function description.

Description
  This protocol provides runtime services for communicating between DXE drivers and a registered
  MMI handler.

EFI_MM_COMMUNICATION_PROTOCOL.Communicate()
Summary
  Communicates with a registered handler.

Prototype
  typedef
  EFI_STATUS
  (EFIAPI *EFI_MM_COMMUNICATE) (
  IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
  IN OUT VOID *CommBuffer,
  IN OUT UINTN *CommSize OPTIONAL
  );

Parameters
  This - The EFI_MM_COMMUNICATION_PROTOCOL instance.
  CommBuffer - Pointer to the buffer to convey into MMRAM.
  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. This parameter is optional
                     and may be NULL.

Description
  This function provides a service to send and receive messages from a registered UEFI service. The EFI_MM_COMMUNICATION_PROTOCOL driver is responsible for doing any of the 
  copies such that the data lives in boot-service-accessible RAM.
  A given implementation of the EFI_MM_COMMUNICATION_PROTOCOL may choose to use the EFI_MM_CONTROL_PROTOCOL for effecting the mode transition, or it may use some other method.
  The agent invoking the communication interface at runtime may be virtually mapped. The MM infrastructure code and handlers, on the other hand, execute in physical mode. As a result, the non-MM agent, 
  which may be executing in the virtual-mode OS context (as a result of an OS invocation of the UEFI SetVirtualAddressMap() service), should use a contiguous memory buffer with a physical address before 
  invoking this service. If the virtual address of the buffer is used, the MM Driver may not know how to do the appropriate virtual-to-physical conversion.
  To avoid confusion in interpreting frames, the CommunicateBuffer parameter should always begin with EFI_MM_COMMUNICATE_HEADER, which is defined in Related Definitions below. 
  The header data is mandatory for messages sent into the MM agent.
  If the CommSize parameter is omitted the MessageLength field in the EFI_MM_COMMUNICATE_HEADER, 
  in conjunction with the size of the header itself, can be used to ascertain the total size of the communication payload.
  If the MessageLength is zero, or too large for the MM implementation to manage, the MM implementation must update the MessageLength to reflect the size of the Data buffer that it can tolerate.
  If the CommSize parameter is passed into the call, but the integer it points to, has a value of 0, then this must be updated to reflect the maximum size of the CommBuffer that the implementation can tolerate.
  Once inside of MM, the MM infrastructure will call all registered handlers with the same HandlerType as the GUID specified by HeaderGuid and the CommBuffer pointing to Data.
  This function is not reentrant.
  The standard header is used at the beginning of the EFI_MM_INITIALIATION_HEADER structure during MM initialization. See "Related Definitions" below for more information.

Related Definitions
  typedef struct {
  EFI_GUID HeaderGuid;
  UINTN MessageLength;
  UINT8 Data[ANYSIZE_ARRAY];
  } EFI_MM_COMMUNICATE_HEADER;

  HeaderGuid - Allows for disambiguation of the message format. Type EFI_GUID is defined in
                       InstallProtocolInterface() in the UEFI Specification.
  MessageLength - Describes the size of Data (in bytes) and does not include the size of the header.
  Data - Designates an array of bytes that is MessageLength in size.

Status Codes Returned
  EFI_SUCCESS - The message was successfully posted.
  EFI_INVALID_PARAMETER - The buffer was NULL.
  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.
                                           See the function description above for more details.
  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 patchset implements it on AARCH64 Platform. 
***

Supreeth Venkatesh (3):
  ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
  ArmPkg/Drivers: Add ArmMmCommunication Driver information file.
  ArmPkg: Add PCDs needed for MM communication driver.

 ArmPkg/ArmPkg.dec                                  |   3 +
 .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314 +++++++++++++++++++++
 .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 ++++
 3 files changed, 367 insertions(+)
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf

-- 
2.14.1

[PATCH v1 1/3] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Supreeth Venkatesh]

PI v1.5 Specification Volume 4 defines Management Mode Core Interface
and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
means of communicating between drivers outside of MM and MMI
handlers inside of MM.

This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
driver for AARCH64 platforms. It uses SMCs allocated from the standard
SMC range in the SMC Calling Convention specification to communicate
with the standalone MM environment in the secure world.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314 +++++++++++++++++++++
 1 file changed, 314 insertions(+)
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
new file mode 100644
index 0000000000..6064c7d345
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -0,0 +1,314 @@
+/** @file
+
+  Copyright (c) 2016-2017, ARM Limited. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/ArmLib.h>
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Protocol/MmCommunication.h>
+
+#include <IndustryStandard/ArmStdSmc.h>
+
+/**
+  Communicates with a registered handler.
+
+  This function provides an interface to send and receive messages to the
+  Standalone MM environment on behalf of UEFI services.  This function is part
+  of the MM Communication Protocol that may be called in physical mode prior to
+  SetVirtualAddressMap() and in virtual mode after SetVirtualAddressMap().
+
+  @param[in]      This                The EFI_MM_COMMUNICATION_PROTOCOL instance.
+  @param[in, out] CommBuffer          A pointer to the buffer to convey into MMRAM.
+  @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       The CommBuffer was NULL.
+  @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
+MmCommunicationCommunicate (
+  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
+  IN OUT VOID                             *CommBuffer,
+  IN OUT UINTN                            *CommSize    OPTIONAL
+  );
+
+//
+// Address, Length of the pre-allocated buffer for communication with the secure
+// world.
+//
+STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
+
+// Notification event when virtual address map is set.
+STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
+
+//
+// Handle to install the MM Communication Protocol
+//
+STATIC EFI_HANDLE  mMmCommunicateHandle;
+
+//
+// MM Communication Protocol instance
+//
+EFI_MM_COMMUNICATION_PROTOCOL  mMmCommunication = {
+  MmCommunicationCommunicate
+};
+
+/**
+  Communicates with a registered handler.
+
+  This function provides an interface to send and receive messages to the
+  Standalone MM environment on behalf of UEFI services.  This function is part
+  of the MM Communication Protocol that may be called in physical mode prior to
+  SetVirtualAddressMap() and in virtual mode after SetVirtualAddressMap().
+
+  @param[in]      This                The EFI_MM_COMMUNICATION_PROTOCOL instance.
+  @param[in, out] CommBuffer          A pointer to the buffer to convey into SMRAM.
+  @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       The CommBuffer was NULL.
+  @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
+MmCommunicationCommunicate (
+  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
+  IN OUT VOID                             *CommBuffer,
+  IN OUT UINTN                            *CommSize
+  )
+{
+  EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;
+  ARM_SMC_ARGS                CommunicateSmcArgs;
+  EFI_STATUS                  Status;
+  UINTN                       BufferSize;
+
+  CommunicateHeader = CommBuffer;
+  Status = EFI_SUCCESS;
+  BufferSize = 0;
+
+  ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
+
+  //
+  // Check parameters
+  //
+  if (CommBuffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // If the length of the CommBuffer is 0 then return the expected length.
+  if (CommSize) {
+    BufferSize = *CommSize;
+    //
+    // CommSize must hold HeaderGuid and MessageLength
+    //
+    if ( BufferSize < sizeof(EFI_MM_COMMUNICATE_HEADER) ) {
+        return EFI_INVALID_PARAMETER;
+    }
+  } else {
+    BufferSize = CommunicateHeader->MessageLength + sizeof(CommunicateHeader->HeaderGuid) + sizeof(CommunicateHeader->MessageLength);
+  }
+
+  //
+  // If the buffer size if 0 or greater than what can be tolerated by the MM
+  // environment then return the expected size.
+  //
+  if ( (BufferSize == 0) ||
+       (BufferSize > mNsCommBuffMemRegion.Length) ) {
+    CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length;
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // SMC Function ID
+  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
+
+  // Reserved for Future. Must be Zero.
+  CommunicateSmcArgs.Arg1 = 0;
+
+  CopyMem((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBuffer, BufferSize);
+
+  // For the SMC64 version, this parameter is a 64-bit Physical Address (PA)
+  // or Intermediate Physical Address (IPA).
+  // For the SMC32 version, this parameter is a 32-bit PA or IPA.
+  CommunicateSmcArgs.Arg2 = (UINTN) mNsCommBuffMemRegion.PhysicalBase;
+
+  CommunicateSmcArgs.Arg3 = (UINTN) BufferSize;
+
+  // Call the Standalone MM environment.
+  ArmCallSmc(&CommunicateSmcArgs);
+
+  Status = CommunicateSmcArgs.Arg0;
+  switch (Status) {
+  case EFI_SUCCESS:
+    break;
+
+  case ARM_SMC_MM_RET_NOT_SUPPORTED:
+  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
+  default:
+          ASSERT (0);
+  }
+
+  return Status;
+}
+
+/**
+  Notification callback on SetVirtualAddressMap event.
+
+  This function notifies the MM communication protocol interface on
+  SetVirtualAddressMap event and converts pointers used in this driver
+  from physical to virtual address.
+
+  @param  Event          SetVirtualAddressMap event.
+  @param  Context        A context when the SetVirtualAddressMap triggered.
+
+  @retval EFI_SUCCESS    The function executed successfully.
+  @retval Other          Some error occurred when executing this function.
+
+**/
+STATIC VOID
+EFIAPI
+NotifySetVirtualAddressMap (
+  IN EFI_EVENT  Event,
+  IN VOID      *Context
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gRT->ConvertPointer (EFI_OPTIONAL_PTR,
+                                (VOID **)&mNsCommBuffMemRegion.VirtualBase
+                               );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "NotifySetVirtualAddressMap(): Unable to convert MM runtime pointer. Status:0x%x\n", Status));
+  }
+
+}
+
+/**
+  The Entry Point for MM Communication
+
+  This function installs the MM communication protocol interface and finds out
+  what type of buffer management will be required prior to invoking the
+  communication SMC.
+
+  @param  ImageHandle    The firmware allocated handle for the EFI image.
+  @param  SystemTable    A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS    The entry point is executed successfully.
+  @retval Other          Some error occurred when executing this entry point.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationInitialize (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS                 Status;
+
+  mNsCommBuffMemRegion.PhysicalBase = 0;
+  mNsCommBuffMemRegion.VirtualBase = 0;
+  mNsCommBuffMemRegion.Length = 0;
+
+  mNsCommBuffMemRegion.PhysicalBase = PcdGet64(PcdMmBufferBase);
+  // During boot , Virtual and Physical are same
+  mNsCommBuffMemRegion.VirtualBase = mNsCommBuffMemRegion.PhysicalBase;
+  mNsCommBuffMemRegion.Length = PcdGet64(PcdMmBufferSize);
+
+  if (mNsCommBuffMemRegion.PhysicalBase == 0) {
+    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Invalid MM Buffer Base Address.\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (mNsCommBuffMemRegion.Length == 0) {
+    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Maximum Buffer Size is zero.\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory,
+                                mNsCommBuffMemRegion.PhysicalBase,
+                                mNsCommBuffMemRegion.Length,
+                                EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to add MM-NS Buffer Memory Space\n"));
+  }
+
+  Status = gDS->SetMemorySpaceAttributes(mNsCommBuffMemRegion.PhysicalBase,
+                                         mNsCommBuffMemRegion.Length,
+                                         EFI_MEMORY_UC);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to set MM-NS Buffer Memory attributes\n"));
+  }
+
+  Status = gBS->AllocatePages (AllocateAddress,
+                               EfiRuntimeServicesData,
+                               EFI_SIZE_TO_PAGES (mNsCommBuffMemRegion.Length),
+                               &mNsCommBuffMemRegion.PhysicalBase);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to allocate MM-NS Buffer Memory Space\n"));
+  }
+
+  DEBUG ((DEBUG_INFO, "MmCommunicateInitialize: NsBufferAddress - 0x%x, mNsCommBuffMemRegion.Length - 0x%x\n", mNsCommBuffMemRegion.PhysicalBase, mNsCommBuffMemRegion.Length));
+
+  // Install the communication protocol
+  Status = gBS->InstallProtocolInterface (&mMmCommunicateHandle,
+                                          &gEfiMmCommunicationProtocolGuid,
+                                          EFI_NATIVE_INTERFACE,
+                                          &mMmCommunication);
+  if (EFI_ERROR(Status)) {
+    DEBUG((EFI_D_ERROR, "MmCommunicationInitialize: Failed to install MM communication protocol\n"));
+  }
+
+  // Register notification callback when  virtual address is associated
+  // with the physical address.
+  // Create a Set Virtual Address Map event.
+  //
+  Status = gBS->CreateEvent (EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE,  // Type
+                             TPL_NOTIFY,                         // NotifyTpl
+                             NotifySetVirtualAddressMap,         // NotifyFunction
+                             NULL,                               // NotifyContext
+                             &mSetVirtualAddressMapEvent         // Event
+                            );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
-- 
2.14.1

Re: [PATCH v1 1/3] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Ard Biesheuvel]

 Hi Supreeth,

On 12 October 2017 at 18:13, Supreeth Venkatesh
<supreeth.venkatesh@arm.com> wrote:
> PI v1.5 Specification Volume 4 defines Management Mode Core Interface
> and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> means of communicating between drivers outside of MM and MMI
> handlers inside of MM.
>
> This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
> driver for AARCH64 platforms. It uses SMCs allocated from the standard
> SMC range in the SMC Calling Convention specification to communicate
> with the standalone MM environment in the secure world.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
>  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314 +++++++++++++++++++++
>  1 file changed, 314 insertions(+)
>  create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
>
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> new file mode 100644
> index 0000000000..6064c7d345
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -0,0 +1,314 @@
> +/** @file
> +
> +  Copyright (c) 2016-2017, ARM Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/ArmLib.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/MmCommunication.h>
> +
> +#include <IndustryStandard/ArmStdSmc.h>
> +
> +/**
> +  Communicates with a registered handler.
> +
> +  This function provides an interface to send and receive messages to the
> +  Standalone MM environment on behalf of UEFI services.  This function is part
> +  of the MM Communication Protocol that may be called in physical mode prior to
> +  SetVirtualAddressMap() and in virtual mode after SetVirtualAddressMap().
> +
> +  @param[in]      This                The EFI_MM_COMMUNICATION_PROTOCOL instance.
> +  @param[in, out] CommBuffer          A pointer to the buffer to convey into MMRAM.
> +  @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       The CommBuffer was NULL.
> +  @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

Can this be static? If yes, please remove the forward declaration
entirely, and reorder the definition with the reference.

> +EFIAPI
> +MmCommunicationCommunicate (
> +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> +  IN OUT VOID                             *CommBuffer,
> +  IN OUT UINTN                            *CommSize    OPTIONAL
> +  );
> +
> +//
> +// Address, Length of the pre-allocated buffer for communication with the secure
> +// world.
> +//
> +STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
> +
> +// Notification event when virtual address map is set.
> +STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
> +
> +//
> +// Handle to install the MM Communication Protocol
> +//
> +STATIC EFI_HANDLE  mMmCommunicateHandle;
> +
> +//
> +// MM Communication Protocol instance
> +//
> +EFI_MM_COMMUNICATION_PROTOCOL  mMmCommunication = {

Can this be static?

> +  MmCommunicationCommunicate
> +};
> +
> +/**
> +  Communicates with a registered handler.
> +
> +  This function provides an interface to send and receive messages to the
> +  Standalone MM environment on behalf of UEFI services.  This function is part
> +  of the MM Communication Protocol that may be called in physical mode prior to
> +  SetVirtualAddressMap() and in virtual mode after SetVirtualAddressMap().
> +
> +  @param[in]      This                The EFI_MM_COMMUNICATION_PROTOCOL instance.
> +  @param[in, out] CommBuffer          A pointer to the buffer to convey into SMRAM.
> +  @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       The CommBuffer was NULL.
> +  @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
> +MmCommunicationCommunicate (
> +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> +  IN OUT VOID                             *CommBuffer,
> +  IN OUT UINTN                            *CommSize
> +  )
> +{
> +  EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;

Please align the * with the other variable names

> +  ARM_SMC_ARGS                CommunicateSmcArgs;
> +  EFI_STATUS                  Status;
> +  UINTN                       BufferSize;
> +
> +  CommunicateHeader = CommBuffer;
> +  Status = EFI_SUCCESS;
> +  BufferSize = 0;
> +
> +  ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
> +
> +  //
> +  // Check parameters
> +  //
> +  if (CommBuffer == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // If the length of the CommBuffer is 0 then return the expected length.
> +  if (CommSize) {
> +    BufferSize = *CommSize;
> +    //
> +    // CommSize must hold HeaderGuid and MessageLength
> +    //
> +    if ( BufferSize < sizeof(EFI_MM_COMMUNICATE_HEADER) ) {

No spaces inside () please

> +        return EFI_INVALID_PARAMETER;
> +    }
> +  } else {
> +    BufferSize = CommunicateHeader->MessageLength + sizeof(CommunicateHeader->HeaderGuid) + sizeof(CommunicateHeader->MessageLength);

Spaces before ( please, and limit the line length to 80 columns.

> +  }
> +
> +  //
> +  // If the buffer size if 0 or greater than what can be tolerated by the MM
> +  // environment then return the expected size.
> +  //
> +  if ( (BufferSize == 0) ||
> +       (BufferSize > mNsCommBuffMemRegion.Length) ) {

No spaces inside () please

> +    CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length;
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
> +
> +  // SMC Function ID
> +  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
> +
> +  // Reserved for Future. Must be Zero.
> +  CommunicateSmcArgs.Arg1 = 0;
> +
> +  CopyMem((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBuffer, BufferSize);

Space before (

> +
> +  // For the SMC64 version, this parameter is a 64-bit Physical Address (PA)
> +  // or Intermediate Physical Address (IPA).
> +  // For the SMC32 version, this parameter is a 32-bit PA or IPA.
> +  CommunicateSmcArgs.Arg2 = (UINTN) mNsCommBuffMemRegion.PhysicalBase;

No spaces after (type) casts please.

> +
> +  CommunicateSmcArgs.Arg3 = (UINTN) BufferSize;
> +
> +  // Call the Standalone MM environment.
> +  ArmCallSmc(&CommunicateSmcArgs);

Space before (

> +
> +  Status = CommunicateSmcArgs.Arg0;
> +  switch (Status) {
> +  case EFI_SUCCESS:

Please don't overload the EFI_STATUS type. If EFI_SUCCESS and
ARM_SMC_MM_RET_SUCCESS happen to have the same numerical value, that
doesn't mean you should cut corners like this. So instead,

switch (CommunicateSmcArgs.Arg0) {
case ARM_SMC_MM_RET_SUCCESS:
  Status = EFI_SUCCESS;

> +    break;
> +
> +  case ARM_SMC_MM_RET_NOT_SUPPORTED:
> +  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
> +  default:
> +          ASSERT (0);

and don't forget to assign Status here.


> +  }
> +
> +  return Status;
> +}
> +
> +/**
> +  Notification callback on SetVirtualAddressMap event.
> +
> +  This function notifies the MM communication protocol interface on
> +  SetVirtualAddressMap event and converts pointers used in this driver
> +  from physical to virtual address.
> +
> +  @param  Event          SetVirtualAddressMap event.
> +  @param  Context        A context when the SetVirtualAddressMap triggered.
> +
> +  @retval EFI_SUCCESS    The function executed successfully.
> +  @retval Other          Some error occurred when executing this function.
> +
> +**/
> +STATIC VOID

VOID on a separate line please

> +EFIAPI
> +NotifySetVirtualAddressMap (
> +  IN EFI_EVENT  Event,
> +  IN VOID      *Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = gRT->ConvertPointer (EFI_OPTIONAL_PTR,
> +                                (VOID **)&mNsCommBuffMemRegion.VirtualBase
> +                               );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "NotifySetVirtualAddressMap(): Unable to convert MM runtime pointer. Status:0x%x\n", Status));

please use DEBUG_xxx not EFI_D_xxx (those are deprecated)

> +  }
> +
> +}
> +
> +/**
> +  The Entry Point for MM Communication
> +
> +  This function installs the MM communication protocol interface and finds out
> +  what type of buffer management will be required prior to invoking the
> +  communication SMC.
> +
> +  @param  ImageHandle    The firmware allocated handle for the EFI image.
> +  @param  SystemTable    A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS    The entry point is executed successfully.
> +  @retval Other          Some error occurred when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MmCommunicationInitialize (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS                 Status;
> +
> +  mNsCommBuffMemRegion.PhysicalBase = 0;
> +  mNsCommBuffMemRegion.VirtualBase = 0;
> +  mNsCommBuffMemRegion.Length = 0;
> +
> +  mNsCommBuffMemRegion.PhysicalBase = PcdGet64(PcdMmBufferBase);

Space before (

> +  // During boot , Virtual and Physical are same
> +  mNsCommBuffMemRegion.VirtualBase = mNsCommBuffMemRegion.PhysicalBase;
> +  mNsCommBuffMemRegion.Length = PcdGet64(PcdMmBufferSize);

and here

> +
> +  if (mNsCommBuffMemRegion.PhysicalBase == 0) {
> +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Invalid MM Buffer Base Address.\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mNsCommBuffMemRegion.Length == 0) {
> +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Maximum Buffer Size is zero.\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory,
> +                                mNsCommBuffMemRegion.PhysicalBase,
> +                                mNsCommBuffMemRegion.Length,
> +                                EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to add MM-NS Buffer Memory Space\n"));

Are you sure it makes sense to proceed after this error?

> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes(mNsCommBuffMemRegion.PhysicalBase,
> +                                         mNsCommBuffMemRegion.Length,
> +                                         EFI_MEMORY_UC);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to set MM-NS Buffer Memory attributes\n"));

And here

> +  }
> +
> +  Status = gBS->AllocatePages (AllocateAddress,
> +                               EfiRuntimeServicesData,
> +                               EFI_SIZE_TO_PAGES (mNsCommBuffMemRegion.Length),
> +                               &mNsCommBuffMemRegion.PhysicalBase);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to allocate MM-NS Buffer Memory Space\n"));

And here

> +  }
> +
> +  DEBUG ((DEBUG_INFO, "MmCommunicateInitialize: NsBufferAddress - 0x%x, mNsCommBuffMemRegion.Length - 0x%x\n", mNsCommBuffMemRegion.PhysicalBase, mNsCommBuffMemRegion.Length));

Line length < 80 columns please

> +
> +  // Install the communication protocol
> +  Status = gBS->InstallProtocolInterface (&mMmCommunicateHandle,
> +                                          &gEfiMmCommunicationProtocolGuid,
> +                                          EFI_NATIVE_INTERFACE,
> +                                          &mMmCommunication);
> +  if (EFI_ERROR(Status)) {
> +    DEBUG((EFI_D_ERROR, "MmCommunicationInitialize: Failed to install MM communication protocol\n"));

return Status

> +  }
> +
> +  // Register notification callback when  virtual address is associated
> +  // with the physical address.
> +  // Create a Set Virtual Address Map event.
> +  //
> +  Status = gBS->CreateEvent (EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE,  // Type
> +                             TPL_NOTIFY,                         // NotifyTpl
> +                             NotifySetVirtualAddressMap,         // NotifyFunction
> +                             NULL,                               // NotifyContext
> +                             &mSetVirtualAddressMapEvent         // Event
> +                            );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> --
> 2.14.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v1 1/3] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Supreeth Venkatesh]

On Thu, 2017-10-12 at 18:29 +0100, Ard Biesheuvel wrote:
>  Hi Supreeth,
Hi Ard,

Thanks for your comments.
I have rolled all of them in v2 of this patch series except for
"static" comment below.
> 
> On 12 October 2017 at 18:13, Supreeth Venkatesh
> <supreeth.venkatesh@arm.com> wrote:
> > 
> > PI v1.5 Specification Volume 4 defines Management Mode Core
> > Interface
> > and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> > means of communicating between drivers outside of MM and MMI
> > handlers inside of MM.
> > 
> > This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
> > driver for AARCH64 platforms. It uses SMCs allocated from the
> > standard
> > SMC range in the SMC Calling Convention specification to
> > communicate
> > with the standalone MM environment in the secure world.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > ---
> >  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314
> > +++++++++++++++++++++
> >  1 file changed, 314 insertions(+)
> >  create mode 100644
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > 
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > new file mode 100644
> > index 0000000000..6064c7d345
> > --- /dev/null
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > @@ -0,0 +1,314 @@
> > +/** @file
> > +
> > +  Copyright (c) 2016-2017, ARM Limited. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions
> > of the BSD License
> > +  which accompanies this distribution.  The full text of the
> > license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include <Library/ArmLib.h>
> > +#include <Library/ArmSmcLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/DxeServicesTableLib.h>
> > +#include <Library/HobLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +
> > +#include <Protocol/MmCommunication.h>
> > +
> > +#include <IndustryStandard/ArmStdSmc.h>
> > +
> > +/**
> > +  Communicates with a registered handler.
> > +
> > +  This function provides an interface to send and receive messages
> > to the
> > +  Standalone MM environment on behalf of UEFI services.  This
> > function is part
> > +  of the MM Communication Protocol that may be called in physical
> > mode prior to
> > +  SetVirtualAddressMap() and in virtual mode after
> > SetVirtualAddressMap().
> > +
> > +  @param[in]      This                The
> > EFI_MM_COMMUNICATION_PROTOCOL instance.
> > +  @param[in, out] CommBuffer          A pointer to the buffer to
> > convey into MMRAM.
> > +  @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       The CommBuffer was NULL.
> > +  @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
> Can this be static? If yes, please remove the forward declaration
> entirely, and reorder the definition with the reference.
> 
> > 
> > +EFIAPI
> > +MmCommunicationCommunicate (
> > +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> > +  IN OUT VOID                             *CommBuffer,
> > +  IN OUT UINTN                            *CommSize    OPTIONAL
> > +  );
> > +
> > +//
> > +// Address, Length of the pre-allocated buffer for communication
> > with the secure
> > +// world.
> > +//
> > +STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
> > +
> > +// Notification event when virtual address map is set.
> > +STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
> > +
> > +//
> > +// Handle to install the MM Communication Protocol
> > +//
> > +STATIC EFI_HANDLE  mMmCommunicateHandle;
> > +
> > +//
> > +// MM Communication Protocol instance
> > +//
> > +EFI_MM_COMMUNICATION_PROTOCOL  mMmCommunication = {
> Can this be static?
> 
> > 
> > +  MmCommunicationCommunicate
> > +};
> > +
> > +/**
> > +  Communicates with a registered handler.
> > +
> > +  This function provides an interface to send and receive messages
> > to the
> > +  Standalone MM environment on behalf of UEFI services.  This
> > function is part
> > +  of the MM Communication Protocol that may be called in physical
> > mode prior to
> > +  SetVirtualAddressMap() and in virtual mode after
> > SetVirtualAddressMap().
> > +
> > +  @param[in]      This                The
> > EFI_MM_COMMUNICATION_PROTOCOL instance.
> > +  @param[in, out] CommBuffer          A pointer to the buffer to
> > convey into SMRAM.
> > +  @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       The CommBuffer was NULL.
> > +  @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
> > +MmCommunicationCommunicate (
> > +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> > +  IN OUT VOID                             *CommBuffer,
> > +  IN OUT UINTN                            *CommSize
> > +  )
> > +{
> > +  EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;
> Please align the * with the other variable names
> 
> > 
> > +  ARM_SMC_ARGS                CommunicateSmcArgs;
> > +  EFI_STATUS                  Status;
> > +  UINTN                       BufferSize;
> > +
> > +  CommunicateHeader = CommBuffer;
> > +  Status = EFI_SUCCESS;
> > +  BufferSize = 0;
> > +
> > +  ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
> > +
> > +  //
> > +  // Check parameters
> > +  //
> > +  if (CommBuffer == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // If the length of the CommBuffer is 0 then return the expected
> > length.
> > +  if (CommSize) {
> > +    BufferSize = *CommSize;
> > +    //
> > +    // CommSize must hold HeaderGuid and MessageLength
> > +    //
> > +    if ( BufferSize < sizeof(EFI_MM_COMMUNICATE_HEADER) ) {
> No spaces inside () please
> 
> > 
> > +        return EFI_INVALID_PARAMETER;
> > +    }
> > +  } else {
> > +    BufferSize = CommunicateHeader->MessageLength +
> > sizeof(CommunicateHeader->HeaderGuid) + sizeof(CommunicateHeader-
> > >MessageLength);
> Spaces before ( please, and limit the line length to 80 columns.
> 
> > 
> > +  }
> > +
> > +  //
> > +  // If the buffer size if 0 or greater than what can be tolerated
> > by the MM
> > +  // environment then return the expected size.
> > +  //
> > +  if ( (BufferSize == 0) ||
> > +       (BufferSize > mNsCommBuffMemRegion.Length) ) {
> No spaces inside () please
> 
> > 
> > +    CommunicateHeader->MessageLength =
> > mNsCommBuffMemRegion.Length;
> > +    return EFI_BAD_BUFFER_SIZE;
> > +  }
> > +
> > +  // SMC Function ID
> > +  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
> > +
> > +  // Reserved for Future. Must be Zero.
> > +  CommunicateSmcArgs.Arg1 = 0;
> > +
> > +  CopyMem((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBuffer,
> > BufferSize);
> Space before (
> 
> > 
> > +
> > +  // For the SMC64 version, this parameter is a 64-bit Physical
> > Address (PA)
> > +  // or Intermediate Physical Address (IPA).
> > +  // For the SMC32 version, this parameter is a 32-bit PA or IPA.
> > +  CommunicateSmcArgs.Arg2 = (UINTN)
> > mNsCommBuffMemRegion.PhysicalBase;
> No spaces after (type) casts please.
> 
> > 
> > +
> > +  CommunicateSmcArgs.Arg3 = (UINTN) BufferSize;
> > +
> > +  // Call the Standalone MM environment.
> > +  ArmCallSmc(&CommunicateSmcArgs);
> Space before (
> 
> > 
> > +
> > +  Status = CommunicateSmcArgs.Arg0;
> > +  switch (Status) {
> > +  case EFI_SUCCESS:
> Please don't overload the EFI_STATUS type. If EFI_SUCCESS and
> ARM_SMC_MM_RET_SUCCESS happen to have the same numerical value, that
> doesn't mean you should cut corners like this. So instead,
> 
> switch (CommunicateSmcArgs.Arg0) {
> case ARM_SMC_MM_RET_SUCCESS:
>   Status = EFI_SUCCESS;
> 
> > 
> > +    break;
> > +
> > +  case ARM_SMC_MM_RET_NOT_SUPPORTED:
> > +  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
> > +  default:
> > +          ASSERT (0);
> and don't forget to assign Status here.
> 
> 
> > 
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> > +/**
> > +  Notification callback on SetVirtualAddressMap event.
> > +
> > +  This function notifies the MM communication protocol interface
> > on
> > +  SetVirtualAddressMap event and converts pointers used in this
> > driver
> > +  from physical to virtual address.
> > +
> > +  @param  Event          SetVirtualAddressMap event.
> > +  @param  Context        A context when the SetVirtualAddressMap
> > triggered.
> > +
> > +  @retval EFI_SUCCESS    The function executed successfully.
> > +  @retval Other          Some error occurred when executing this
> > function.
> > +
> > +**/
> > +STATIC VOID
> VOID on a separate line please
> 
> > 
> > +EFIAPI
> > +NotifySetVirtualAddressMap (
> > +  IN EFI_EVENT  Event,
> > +  IN VOID      *Context
> > +  )
> > +{
> > +  EFI_STATUS  Status;
> > +
> > +  Status = gRT->ConvertPointer (EFI_OPTIONAL_PTR,
> > +                                (VOID
> > **)&mNsCommBuffMemRegion.VirtualBase
> > +                               );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((EFI_D_ERROR, "NotifySetVirtualAddressMap(): Unable to
> > convert MM runtime pointer. Status:0x%x\n", Status));
> please use DEBUG_xxx not EFI_D_xxx (those are deprecated)
> 
> > 
> > +  }
> > +
> > +}
> > +
> > +/**
> > +  The Entry Point for MM Communication
> > +
> > +  This function installs the MM communication protocol interface
> > and finds out
> > +  what type of buffer management will be required prior to
> > invoking the
> > +  communication SMC.
> > +
> > +  @param  ImageHandle    The firmware allocated handle for the EFI
> > image.
> > +  @param  SystemTable    A pointer to the EFI System Table.
> > +
> > +  @retval EFI_SUCCESS    The entry point is executed successfully.
> > +  @retval Other          Some error occurred when executing this
> > entry point.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +MmCommunicationInitialize (
> > +  IN EFI_HANDLE         ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  EFI_STATUS                 Status;
> > +
> > +  mNsCommBuffMemRegion.PhysicalBase = 0;
> > +  mNsCommBuffMemRegion.VirtualBase = 0;
> > +  mNsCommBuffMemRegion.Length = 0;
> > +
> > +  mNsCommBuffMemRegion.PhysicalBase = PcdGet64(PcdMmBufferBase);
> Space before (
> 
> > 
> > +  // During boot , Virtual and Physical are same
> > +  mNsCommBuffMemRegion.VirtualBase =
> > mNsCommBuffMemRegion.PhysicalBase;
> > +  mNsCommBuffMemRegion.Length = PcdGet64(PcdMmBufferSize);
> and here
> 
> > 
> > +
> > +  if (mNsCommBuffMemRegion.PhysicalBase == 0) {
> > +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Invalid MM
> > Buffer Base Address.\n"));
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (mNsCommBuffMemRegion.Length == 0) {
> > +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Maximum Buffer
> > Size is zero.\n"));
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory,
> > +                                mNsCommBuffMemRegion.PhysicalBase,
> > +                                mNsCommBuffMemRegion.Length,
> > +                                EFI_MEMORY_UC |
> > EFI_MEMORY_RUNTIME);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to add
> > MM-NS Buffer Memory Space\n"));
> Are you sure it makes sense to proceed after this error?
> 
> > 
> > +  }
> > +
> > +  Status = gDS-
> > >SetMemorySpaceAttributes(mNsCommBuffMemRegion.PhysicalBase,
> > +                                         mNsCommBuffMemRegion.Leng
> > th,
> > +                                         EFI_MEMORY_UC);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to set
> > MM-NS Buffer Memory attributes\n"));
> And here
> 
> > 
> > +  }
> > +
> > +  Status = gBS->AllocatePages (AllocateAddress,
> > +                               EfiRuntimeServicesData,
> > +                               EFI_SIZE_TO_PAGES
> > (mNsCommBuffMemRegion.Length),
> > +                               &mNsCommBuffMemRegion.PhysicalBase)
> > ;
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((EFI_D_ERROR, "MmCommunicateInitialize: Failed to
> > allocate MM-NS Buffer Memory Space\n"));
> And here
> 
> > 
> > +  }
> > +
> > +  DEBUG ((DEBUG_INFO, "MmCommunicateInitialize: NsBufferAddress -
> > 0x%x, mNsCommBuffMemRegion.Length - 0x%x\n",
> > mNsCommBuffMemRegion.PhysicalBase, mNsCommBuffMemRegion.Length));
> Line length < 80 columns please
> 
> > 
> > +
> > +  // Install the communication protocol
> > +  Status = gBS->InstallProtocolInterface (&mMmCommunicateHandle,
> > +                                          &gEfiMmCommunicationProt
> > ocolGuid,
> > +                                          EFI_NATIVE_INTERFACE,
> > +                                          &mMmCommunication);
> > +  if (EFI_ERROR(Status)) {
> > +    DEBUG((EFI_D_ERROR, "MmCommunicationInitialize: Failed to
> > install MM communication protocol\n"));
> return Status
> 
> > 
> > +  }
> > +
> > +  // Register notification callback when  virtual address is
> > associated
> > +  // with the physical address.
> > +  // Create a Set Virtual Address Map event.
> > +  //
> > +  Status = gBS->CreateEvent
> > (EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE,  // Type
> > +                             TPL_NOTIFY,                         /
> > / NotifyTpl
> > +                             NotifySetVirtualAddressMap,         /
> > / NotifyFunction
> > +                             NULL,                               /
> > / NotifyContext
> > +                             &mSetVirtualAddressMapEvent         /
> > / Event
> > +                            );
> > +  ASSERT_EFI_ERROR (Status);
> > +
> > +  return Status;
> > +}
> > --
> > 2.14.1
> > 
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

[PATCH v1 2/3] ArmPkg/Drivers: Add ArmMmCommunication Driver information file.

[Supreeth Venkatesh]

This patch adds the MM Communication driver (.inf) file to define entry
point for this driver and other compile related information the driver
needs.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 .../Drivers/MmCommunicationDxe/MmCommunication.inf | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
new file mode 100644
index 0000000000..d39ee5fdd7
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
@@ -0,0 +1,50 @@
+#/** @file
+#
+#  DXE MM Communicate driver
+#
+#  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = ArmMmCommunication
+  FILE_GUID                      = 09EE81D3-F15E-43F4-85B4-CB9873DA5D6B
+  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = MmCommunicationInitialize
+
+[Sources.Common]
+  MmCommunication.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  ArmSmcLib
+  BaseMemoryLib
+  DebugLib
+  DxeServicesTableLib
+  HobLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiMmCommunicationProtocolGuid              ## PRODUCES
+
+[Pcd.common]
+  gArmTokenSpaceGuid.PcdMmBufferBase
+  gArmTokenSpaceGuid.PcdMmBufferSize
+
+[Depex]
+  TRUE
-- 
2.14.1

[PATCH v1 3/3] ArmPkg: Add PCDs needed for MM communication driver.

[Supreeth Venkatesh]

This patch defines PCDs to describe the base address and size of
communication buffer between normal world (uefi) and standalone MM
environment in the secure world.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 ArmPkg/ArmPkg.dec | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index f99054a7de..d871ecc654 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -229,6 +229,9 @@
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0|UINT64|0x00000029
   gArmTokenSpaceGuid.PcdSystemMemorySize|0|UINT64|0x0000002A
 
+  gArmTokenSpaceGuid.PcdMmBufferBase|0|UINT64|0x00000045
+  gArmTokenSpaceGuid.PcdMmBufferSize|0|UINT64|0x00000046
+
 [PcdsFixedAtBuild.common, PcdsDynamic.common]
   #
   # ARM Architectural Timer
-- 
2.14.1

Re: [PATCH v1 3/3] ArmPkg: Add PCDs needed for MM communication driver.

[Ard Biesheuvel]

Hi Supreeth,

On 12 October 2017 at 18:13, Supreeth Venkatesh
<supreeth.venkatesh@arm.com> wrote:
> This patch defines PCDs to describe the base address and size of
> communication buffer between normal world (uefi) and standalone MM
> environment in the secure world.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
>  ArmPkg/ArmPkg.dec | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index f99054a7de..d871ecc654 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -229,6 +229,9 @@
>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0|UINT64|0x00000029
>    gArmTokenSpaceGuid.PcdSystemMemorySize|0|UINT64|0x0000002A
>
> +  gArmTokenSpaceGuid.PcdMmBufferBase|0|UINT64|0x00000045
> +  gArmTokenSpaceGuid.PcdMmBufferSize|0|UINT64|0x00000046
> +
>  [PcdsFixedAtBuild.common, PcdsDynamic.common]
>    #
>    # ARM Architectural Timer

This patch should be first in the series if the other patches depend on it.

Re: [PATCH v1 3/3] ArmPkg: Add PCDs needed for MM communication driver.

[Supreeth Venkatesh]

On Thu, 2017-10-12 at 18:18 +0100, Ard Biesheuvel wrote:
> Hi Supreeth,
> 
> On 12 October 2017 at 18:13, Supreeth Venkatesh
> <supreeth.venkatesh@arm.com> wrote:
> > 
> > This patch defines PCDs to describe the base address and size of
> > communication buffer between normal world (uefi) and standalone MM
> > environment in the secure world.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > ---
> >  ArmPkg/ArmPkg.dec | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > index f99054a7de..d871ecc654 100644
> > --- a/ArmPkg/ArmPkg.dec
> > +++ b/ArmPkg/ArmPkg.dec
> > @@ -229,6 +229,9 @@
> >    gArmTokenSpaceGuid.PcdSystemMemoryBase|0|UINT64|0x00000029
> >    gArmTokenSpaceGuid.PcdSystemMemorySize|0|UINT64|0x0000002A
> > 
> > +  gArmTokenSpaceGuid.PcdMmBufferBase|0|UINT64|0x00000045
> > +  gArmTokenSpaceGuid.PcdMmBufferSize|0|UINT64|0x00000046
> > +
> >  [PcdsFixedAtBuild.common, PcdsDynamic.common]
> >    #
> >    # ARM Architectural Timer
> This patch should be first in the series if the other patches depend
> on it.
Thanks! Ard. I Will re-order the patch set and will also wait to
accommodate any other review comments before sending v2.

Re: [PATCH v1 0/3] *** EFI_MM_COMMUNICATION_PROTOCOL ***

[Achin Gupta]

Hi Supreeth,

Could you acknowledge me as a contributor in the relevant patches and repost?

cheers,
Achin

On Thu, Oct 12, 2017 at 06:13:49PM +0100, Supreeth Venkatesh wrote:
> ***
> PI v1.5 Specification Volume 4 defines Management Mode Core Interface
> and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> means of communicating between drivers outside of MM and MMI
> handlers inside of MM.
>
> EFI_MM_COMMUNICATION_PROTOCOL
> Summary
>   This protocol provides a means of communicating between drivers outside of MM and MMI
>   handlers inside of MM.
>
> GUID
>   #define EFI_MM_COMMUNICATION_PROTOCOL_GUID \
>   { 0xc68ed8e2, 0x9dc6, 0x4cbd, 0x9d, 0x94, 0xdb, 0x65, 0xac, 0xc5, 0xc3, 0x32 }
>
> Prototype
>   typedef struct _EFI_MM_COMMUNICATION_PROTOCOL {
>   EFI_MM_COMMUNICATE Communicate;
>   } EFI_MM_COMMUNICATION_PROTOCOL;
>
> Members
> Communicate
>   Sends/receives a message for a registered handler. See the Communicate()
>   function description.
>
> Description
>   This protocol provides runtime services for communicating between DXE drivers and a registered
>   MMI handler.
>
> EFI_MM_COMMUNICATION_PROTOCOL.Communicate()
> Summary
>   Communicates with a registered handler.
>
> Prototype
>   typedef
>   EFI_STATUS
>   (EFIAPI *EFI_MM_COMMUNICATE) (
>   IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
>   IN OUT VOID *CommBuffer,
>   IN OUT UINTN *CommSize OPTIONAL
>   );
>
> Parameters
>   This - The EFI_MM_COMMUNICATION_PROTOCOL instance.
>   CommBuffer - Pointer to the buffer to convey into MMRAM.
>   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. This parameter is optional
>                      and may be NULL.
>
> Description
>   This function provides a service to send and receive messages from a registered UEFI service. The EFI_MM_COMMUNICATION_PROTOCOL driver is responsible for doing any of the
>   copies such that the data lives in boot-service-accessible RAM.
>   A given implementation of the EFI_MM_COMMUNICATION_PROTOCOL may choose to use the EFI_MM_CONTROL_PROTOCOL for effecting the mode transition, or it may use some other method.
>   The agent invoking the communication interface at runtime may be virtually mapped. The MM infrastructure code and handlers, on the other hand, execute in physical mode. As a result, the non-MM agent,
>   which may be executing in the virtual-mode OS context (as a result of an OS invocation of the UEFI SetVirtualAddressMap() service), should use a contiguous memory buffer with a physical address before
>   invoking this service. If the virtual address of the buffer is used, the MM Driver may not know how to do the appropriate virtual-to-physical conversion.
>   To avoid confusion in interpreting frames, the CommunicateBuffer parameter should always begin with EFI_MM_COMMUNICATE_HEADER, which is defined in Related Definitions below.
>   The header data is mandatory for messages sent into the MM agent.
>   If the CommSize parameter is omitted the MessageLength field in the EFI_MM_COMMUNICATE_HEADER,
>   in conjunction with the size of the header itself, can be used to ascertain the total size of the communication payload.
>   If the MessageLength is zero, or too large for the MM implementation to manage, the MM implementation must update the MessageLength to reflect the size of the Data buffer that it can tolerate.
>   If the CommSize parameter is passed into the call, but the integer it points to, has a value of 0, then this must be updated to reflect the maximum size of the CommBuffer that the implementation can tolerate.
>   Once inside of MM, the MM infrastructure will call all registered handlers with the same HandlerType as the GUID specified by HeaderGuid and the CommBuffer pointing to Data.
>   This function is not reentrant.
>   The standard header is used at the beginning of the EFI_MM_INITIALIATION_HEADER structure during MM initialization. See "Related Definitions" below for more information.
>
> Related Definitions
>   typedef struct {
>   EFI_GUID HeaderGuid;
>   UINTN MessageLength;
>   UINT8 Data[ANYSIZE_ARRAY];
>   } EFI_MM_COMMUNICATE_HEADER;
>
>   HeaderGuid - Allows for disambiguation of the message format. Type EFI_GUID is defined in
>                        InstallProtocolInterface() in the UEFI Specification.
>   MessageLength - Describes the size of Data (in bytes) and does not include the size of the header.
>   Data - Designates an array of bytes that is MessageLength in size.
>
> Status Codes Returned
>   EFI_SUCCESS - The message was successfully posted.
>   EFI_INVALID_PARAMETER - The buffer was NULL.
>   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.
>                                            See the function description above for more details.
>   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 patchset implements it on AARCH64 Platform.
> ***
>
> Supreeth Venkatesh (3):
>   ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
>   ArmPkg/Drivers: Add ArmMmCommunication Driver information file.
>   ArmPkg: Add PCDs needed for MM communication driver.
>
>  ArmPkg/ArmPkg.dec                                  |   3 +
>  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314 +++++++++++++++++++++
>  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 ++++
>  3 files changed, 367 insertions(+)
>  create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
>  create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
>
> --
> 2.14.1
>

Re: [PATCH v1 0/3] *** EFI_MM_COMMUNICATION_PROTOCOL ***

[Supreeth Venkatesh]

On Thu, 2017-10-12 at 18:38 +0100, Achin Gupta wrote:
> Hi Supreeth,
> 
> Could you acknowledge me as a contributor in the relevant patches and
> repost?
Can sure do as done in previous patches. However, Since I wanted you to
review as well the latest changes, I was not sure whether co-author can
be reviewer as well. I can add you as co-author in the next patchset,
if that is what you prefer.
  
> 
> cheers,
> Achin
> 
> On Thu, Oct 12, 2017 at 06:13:49PM +0100, Supreeth Venkatesh wrote:
> > 
> > ***
> > PI v1.5 Specification Volume 4 defines Management Mode Core
> > Interface
> > and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> > means of communicating between drivers outside of MM and MMI
> > handlers inside of MM.
> > 
> > EFI_MM_COMMUNICATION_PROTOCOL
> > Summary
> >   This protocol provides a means of communicating between drivers
> > outside of MM and MMI
> >   handlers inside of MM.
> > 
> > GUID
> >   #define EFI_MM_COMMUNICATION_PROTOCOL_GUID \
> >   { 0xc68ed8e2, 0x9dc6, 0x4cbd, 0x9d, 0x94, 0xdb, 0x65, 0xac, 0xc5,
> > 0xc3, 0x32 }
> > 
> > Prototype
> >   typedef struct _EFI_MM_COMMUNICATION_PROTOCOL {
> >   EFI_MM_COMMUNICATE Communicate;
> >   } EFI_MM_COMMUNICATION_PROTOCOL;
> > 
> > Members
> > Communicate
> >   Sends/receives a message for a registered handler. See the
> > Communicate()
> >   function description.
> > 
> > Description
> >   This protocol provides runtime services for communicating between
> > DXE drivers and a registered
> >   MMI handler.
> > 
> > EFI_MM_COMMUNICATION_PROTOCOL.Communicate()
> > Summary
> >   Communicates with a registered handler.
> > 
> > Prototype
> >   typedef
> >   EFI_STATUS
> >   (EFIAPI *EFI_MM_COMMUNICATE) (
> >   IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
> >   IN OUT VOID *CommBuffer,
> >   IN OUT UINTN *CommSize OPTIONAL
> >   );
> > 
> > Parameters
> >   This - The EFI_MM_COMMUNICATION_PROTOCOL instance.
> >   CommBuffer - Pointer to the buffer to convey into MMRAM.
> >   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. This parameter is optional
> >                      and may be NULL.
> > 
> > Description
> >   This function provides a service to send and receive messages
> > from a registered UEFI service. The EFI_MM_COMMUNICATION_PROTOCOL
> > driver is responsible for doing any of the
> >   copies such that the data lives in boot-service-accessible RAM.
> >   A given implementation of the EFI_MM_COMMUNICATION_PROTOCOL may
> > choose to use the EFI_MM_CONTROL_PROTOCOL for effecting the mode
> > transition, or it may use some other method.
> >   The agent invoking the communication interface at runtime may be
> > virtually mapped. The MM infrastructure code and handlers, on the
> > other hand, execute in physical mode. As a result, the non-MM
> > agent,
> >   which may be executing in the virtual-mode OS context (as a
> > result of an OS invocation of the UEFI SetVirtualAddressMap()
> > service), should use a contiguous memory buffer with a physical
> > address before
> >   invoking this service. If the virtual address of the buffer is
> > used, the MM Driver may not know how to do the appropriate virtual-
> > to-physical conversion.
> >   To avoid confusion in interpreting frames, the CommunicateBuffer
> > parameter should always begin with EFI_MM_COMMUNICATE_HEADER, which
> > is defined in Related Definitions below.
> >   The header data is mandatory for messages sent into the MM agent.
> >   If the CommSize parameter is omitted the MessageLength field in
> > the EFI_MM_COMMUNICATE_HEADER,
> >   in conjunction with the size of the header itself, can be used to
> > ascertain the total size of the communication payload.
> >   If the MessageLength is zero, or too large for the MM
> > implementation to manage, the MM implementation must update the
> > MessageLength to reflect the size of the Data buffer that it can
> > tolerate.
> >   If the CommSize parameter is passed into the call, but the
> > integer it points to, has a value of 0, then this must be updated
> > to reflect the maximum size of the CommBuffer that the
> > implementation can tolerate.
> >   Once inside of MM, the MM infrastructure will call all registered
> > handlers with the same HandlerType as the GUID specified by
> > HeaderGuid and the CommBuffer pointing to Data.
> >   This function is not reentrant.
> >   The standard header is used at the beginning of the
> > EFI_MM_INITIALIATION_HEADER structure during MM initialization. See
> > "Related Definitions" below for more information.
> > 
> > Related Definitions
> >   typedef struct {
> >   EFI_GUID HeaderGuid;
> >   UINTN MessageLength;
> >   UINT8 Data[ANYSIZE_ARRAY];
> >   } EFI_MM_COMMUNICATE_HEADER;
> > 
> >   HeaderGuid - Allows for disambiguation of the message format.
> > Type EFI_GUID is defined in
> >                        InstallProtocolInterface() in the UEFI
> > Specification.
> >   MessageLength - Describes the size of Data (in bytes) and does
> > not include the size of the header.
> >   Data - Designates an array of bytes that is MessageLength in
> > size.
> > 
> > Status Codes Returned
> >   EFI_SUCCESS - The message was successfully posted.
> >   EFI_INVALID_PARAMETER - The buffer was NULL.
> >   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.
> >                                            See the function
> > description above for more details.
> >   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 patchset implements it on AARCH64 Platform.
> > ***
> > 
> > Supreeth Venkatesh (3):
> >   ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
> >   ArmPkg/Drivers: Add ArmMmCommunication Driver information file.
> >   ArmPkg: Add PCDs needed for MM communication driver.
> > 
> >  ArmPkg/ArmPkg.dec                                  |   3 +
> >  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314
> > +++++++++++++++++++++
> >  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 ++++
> >  3 files changed, 367 insertions(+)
> >  create mode 100644
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> >  create mode 100644
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > 
> > --
> > 2.14.1
> > 

Re: [PATCH v1 0/3] *** EFI_MM_COMMUNICATION_PROTOCOL ***

[Achin Gupta]

On Thu, Oct 12, 2017 at 12:43:13PM -0500, Supreeth Venkatesh wrote:
> On Thu, 2017-10-12 at 18:38 +0100, Achin Gupta wrote:
> > Hi Supreeth,
> >
> > Could you acknowledge me as a contributor in the relevant patches and
> > repost?
> Can sure do as done in previous patches. However, Since I wanted you to
> review as well the latest changes, I was not sure whether co-author can
> be reviewer as well. I can add you as co-author in the next patchset,
> if that is what you prefer.

Yes I would prefer that!

Thanks!
Achin

>   
> >
> > cheers,
> > Achin
> >
> > On Thu, Oct 12, 2017 at 06:13:49PM +0100, Supreeth Venkatesh wrote:
> > >
> > > ***
> > > PI v1.5 Specification Volume 4 defines Management Mode Core
> > > Interface
> > > and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides a
> > > means of communicating between drivers outside of MM and MMI
> > > handlers inside of MM.
> > >
> > > EFI_MM_COMMUNICATION_PROTOCOL
> > > Summary
> > >   This protocol provides a means of communicating between drivers
> > > outside of MM and MMI
> > >   handlers inside of MM.
> > >
> > > GUID
> > >   #define EFI_MM_COMMUNICATION_PROTOCOL_GUID \
> > >   { 0xc68ed8e2, 0x9dc6, 0x4cbd, 0x9d, 0x94, 0xdb, 0x65, 0xac, 0xc5,
> > > 0xc3, 0x32 }
> > >
> > > Prototype
> > >   typedef struct _EFI_MM_COMMUNICATION_PROTOCOL {
> > >   EFI_MM_COMMUNICATE Communicate;
> > >   } EFI_MM_COMMUNICATION_PROTOCOL;
> > >
> > > Members
> > > Communicate
> > >   Sends/receives a message for a registered handler. See the
> > > Communicate()
> > >   function description.
> > >
> > > Description
> > >   This protocol provides runtime services for communicating between
> > > DXE drivers and a registered
> > >   MMI handler.
> > >
> > > EFI_MM_COMMUNICATION_PROTOCOL.Communicate()
> > > Summary
> > >   Communicates with a registered handler.
> > >
> > > Prototype
> > >   typedef
> > >   EFI_STATUS
> > >   (EFIAPI *EFI_MM_COMMUNICATE) (
> > >   IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
> > >   IN OUT VOID *CommBuffer,
> > >   IN OUT UINTN *CommSize OPTIONAL
> > >   );
> > >
> > > Parameters
> > >   This - The EFI_MM_COMMUNICATION_PROTOCOL instance.
> > >   CommBuffer - Pointer to the buffer to convey into MMRAM.
> > >   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. This parameter is optional
> > >                      and may be NULL.
> > >
> > > Description
> > >   This function provides a service to send and receive messages
> > > from a registered UEFI service. The EFI_MM_COMMUNICATION_PROTOCOL
> > > driver is responsible for doing any of the
> > >   copies such that the data lives in boot-service-accessible RAM.
> > >   A given implementation of the EFI_MM_COMMUNICATION_PROTOCOL may
> > > choose to use the EFI_MM_CONTROL_PROTOCOL for effecting the mode
> > > transition, or it may use some other method.
> > >   The agent invoking the communication interface at runtime may be
> > > virtually mapped. The MM infrastructure code and handlers, on the
> > > other hand, execute in physical mode. As a result, the non-MM
> > > agent,
> > >   which may be executing in the virtual-mode OS context (as a
> > > result of an OS invocation of the UEFI SetVirtualAddressMap()
> > > service), should use a contiguous memory buffer with a physical
> > > address before
> > >   invoking this service. If the virtual address of the buffer is
> > > used, the MM Driver may not know how to do the appropriate virtual-
> > > to-physical conversion.
> > >   To avoid confusion in interpreting frames, the CommunicateBuffer
> > > parameter should always begin with EFI_MM_COMMUNICATE_HEADER, which
> > > is defined in Related Definitions below.
> > >   The header data is mandatory for messages sent into the MM agent.
> > >   If the CommSize parameter is omitted the MessageLength field in
> > > the EFI_MM_COMMUNICATE_HEADER,
> > >   in conjunction with the size of the header itself, can be used to
> > > ascertain the total size of the communication payload.
> > >   If the MessageLength is zero, or too large for the MM
> > > implementation to manage, the MM implementation must update the
> > > MessageLength to reflect the size of the Data buffer that it can
> > > tolerate.
> > >   If the CommSize parameter is passed into the call, but the
> > > integer it points to, has a value of 0, then this must be updated
> > > to reflect the maximum size of the CommBuffer that the
> > > implementation can tolerate.
> > >   Once inside of MM, the MM infrastructure will call all registered
> > > handlers with the same HandlerType as the GUID specified by
> > > HeaderGuid and the CommBuffer pointing to Data.
> > >   This function is not reentrant.
> > >   The standard header is used at the beginning of the
> > > EFI_MM_INITIALIATION_HEADER structure during MM initialization. See
> > > "Related Definitions" below for more information.
> > >
> > > Related Definitions
> > >   typedef struct {
> > >   EFI_GUID HeaderGuid;
> > >   UINTN MessageLength;
> > >   UINT8 Data[ANYSIZE_ARRAY];
> > >   } EFI_MM_COMMUNICATE_HEADER;
> > >
> > >   HeaderGuid - Allows for disambiguation of the message format.
> > > Type EFI_GUID is defined in
> > >                        InstallProtocolInterface() in the UEFI
> > > Specification.
> > >   MessageLength - Describes the size of Data (in bytes) and does
> > > not include the size of the header.
> > >   Data - Designates an array of bytes that is MessageLength in
> > > size.
> > >
> > > Status Codes Returned
> > >   EFI_SUCCESS - The message was successfully posted.
> > >   EFI_INVALID_PARAMETER - The buffer was NULL.
> > >   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.
> > >                                            See the function
> > > description above for more details.
> > >   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 patchset implements it on AARCH64 Platform.
> > > ***
> > >
> > > Supreeth Venkatesh (3):
> > >   ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
> > >   ArmPkg/Drivers: Add ArmMmCommunication Driver information file.
> > >   ArmPkg: Add PCDs needed for MM communication driver.
> > >
> > >  ArmPkg/ArmPkg.dec                                  |   3 +
> > >  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 314
> > > +++++++++++++++++++++
> > >  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 ++++
> > >  3 files changed, 367 insertions(+)
> > >  create mode 100644
> > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > >  create mode 100644
> > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > >
> > > --
> > > 2.14.1
> > >