[PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

[PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

[Ard Biesheuvel]

This series proposes one possible approach to work around the issue that the
traditional MM and standalone MM implement versions of the communicate protocol
that are fundamentally incompatible from the point of view of the caller.

In traditional MM, the MM communicate protocol takes a physical pointer for
the buffer, so that the SMM execution context can access the memory directly
without having to translate it according to the translation regime of the
caller.

In standalone MM, the buffer that is shared with the MM context is preallocated,
and so it is up to the implementation of the MM communicate protocol to copy the
data from the caller allocated buffer into the preallocated shared buffer. In
order to be able to do so, the DXE driver needs to copy the contents, and for
this it needs to know the virtual address not the physical address.

So this means we have two incompatible versions of the same protocol, and given
that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID for the new
EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish programmatically between a
MM context that takes physical addresses vs one that takes virtual ones.

Since this is known at build time, one way to deal with this is to have two
different implementations of a library that defines an abstract MmCommunicate()
function, allowing the correct implementation to be selected at integration
time.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>

Ard Biesheuvel (4):
  MdeModulePkg: introduce MmCommunicationLib library class
  MdeModulePkg: add implementation of MmCommunicateLib
  StandaloneMmPkg: add implementation of MmCommunicateLib
  MdeModulePkg/VariableSmmRuntimeDxe: switch to MmCommunicateLib library

 MdeModulePkg/MdeModulePkg.dec                                                     |   4 +
 MdeModulePkg/MdeModulePkg.dsc                                                     |   2 +
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf    |  51 +++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf              |   4 +-
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf |  51 +++++++++
 MdeModulePkg/Include/Library/MmCommunicateLib.h                                   |  50 +++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c      | 114 ++++++++++++++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c                |  10 +-
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c   | 113 +++++++++++++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni    |  19 ++++
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni |  19 ++++
 11 files changed, 428 insertions(+), 9 deletions(-)
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 create mode 100644 MdeModulePkg/Include/Library/MmCommunicateLib.h
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni

-- 
2.20.1

[PATCH 1/4] MdeModulePkg: introduce MmCommunicationLib library class

[Ard Biesheuvel]

In order to abstract away the difference between traditional and
standalone MM implementations of the MM communicate protocol, which
have different requirements when it comes to the way the address of
the communication buffer is passed, introduce a library class that
can encapsulate calls to the MM communicate protocols, and which
takes both the physical and virtual adresses of the buffer. This
way, it is left up to the library implementation to decide which
address is passed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/MdeModulePkg.dec                   |  4 ++
 MdeModulePkg/Include/Library/MmCommunicateLib.h | 50 ++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a2130bc43991..0778bf01edca 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -182,6 +182,10 @@ [LibraryClasses]
   #
   DisplayUpdateProgressLib|Include/Library/DisplayUpdateProgressLib.h
 
+  ## @libraryclass  Provides an abstraction for invoking the MM communicate protocol
+  #
+  MmCommunicateLib|Include/Library/MmCommunicateLib.h
+
 [Guids]
   ## MdeModule package token space guid
   # Include/Guid/MdeModulePkgTokenSpace.h
diff --git a/MdeModulePkg/Include/Library/MmCommunicateLib.h b/MdeModulePkg/Include/Library/MmCommunicateLib.h
new file mode 100644
index 000000000000..b302e47f6f8f
--- /dev/null
+++ b/MdeModulePkg/Include/Library/MmCommunicateLib.h
@@ -0,0 +1,50 @@
+/** @file
+  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+
+  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+
+  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.
+
+**/
+
+#ifndef MM_COMMUNICATE_LIB_H__
+#define MM_COMMUNICATE_LIB_H__
+
+/**
+  Invoke the MM communication protocol
+
+  @param[in] PhysicalCommBuffer  Physical address of the communication buffer.
+  @param[in] VirtualCommBuffer   Virtual address of the communication buffer.
+  @param[in] 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.
+
+  @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(s) or CommSize
+                                 parameter, if not omitted, are in address range
+                                 that cannot be accessed by the MM environment.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicate (
+  IN OUT VOID                              *PhysicalCommBuffer,
+  IN OUT VOID                              *VirtualCommBuffer,
+  IN OUT UINTN                             *CommSize OPTIONAL
+  );
+
+#endif
-- 
2.20.1

[PATCH 2/4] MdeModulePkg: add implementation of MmCommunicateLib

[Ard Biesheuvel]

Add an implementation of MmCommunicateLib based on traditional SMM.
This version passes the physical address of the communication buffer
into the MM communicate protocol method.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/MdeModulePkg.dsc                                                  |   1 +
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf |  51 +++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c   | 114 ++++++++++++++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni |  19 ++++
 4 files changed, 185 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 6cd1727a0d61..84c2629d5adc 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -317,6 +317,7 @@ [Components]
   MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
   MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
   MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.inf
+  MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
diff --git a/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
new file mode 100644
index 000000000000..93b924d5eefb
--- /dev/null
+++ b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
@@ -0,0 +1,51 @@
+## @file
+#  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+#
+#  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+#
+#  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                    = 0x0001001B
+  BASE_NAME                      = RuntimeDxeMmCommunicateLib
+  MODULE_UNI_FILE                = RuntimeDxeMmCommunicateLib.uni
+  FILE_GUID                      = cb3ee7d3-ea6f-494c-ac57-c5f4dc0ab3b9
+  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = MmCommunicateLib|DXE_RUNTIME_DRIVER
+  CONSTRUCTOR                    = RuntimeDxeMmCommunicateLibConstructor
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = ARM AARCH64 IA32 X64 EBC
+#
+
+[Sources]
+  RuntimeDxeMmCommunicateLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  UefiBootServicesTableLib
+  UefiRuntimeLib
+
+[Guids]
+  gEfiEventVirtualAddressChangeGuid             ## CONSUMES ## Event
+
+[Protocols]
+  gEfiMmCommunicationProtocolGuid               ## CONSUMES
+
+[Depex]
+  gEfiMmCommunicationProtocolGuid
diff --git a/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
new file mode 100644
index 000000000000..b4ae5dd1fd9a
--- /dev/null
+++ b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
@@ -0,0 +1,114 @@
+/** @file
+  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+
+  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+
+  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 <PiDxe.h>
+
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeLib.h>
+
+#include <Protocol/MmCommunication.h>
+
+STATIC EFI_MM_COMMUNICATION_PROTOCOL    *mMmCommunication;
+STATIC EFI_EVENT                        mVirtualAddressChangeEvent;
+
+/**
+  Invoke the MM communication protocol
+
+  @param[in] PhysicalCommBuffer  Physical address of the communication buffer.
+  @param[in] VirtualCommBuffer   Virtual address of the communication buffer.
+  @param[in] 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.
+
+  @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(s) or CommSize
+                                 parameter, if not omitted, are in address range
+                                 that cannot be accessed by the MM environment.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicate (
+  IN OUT VOID                              *PhysicalCommBuffer,
+  IN OUT VOID                              *VirtualCommBuffer,
+  IN OUT UINTN                             *CommSize OPTIONAL
+  )
+{
+  ASSERT (EfiAtRuntime () || (PhysicalCommBuffer == VirtualCommBuffer));
+
+  return mMmCommunication->Communicate (mMmCommunication, PhysicalCommBuffer,
+                             CommSize);
+}
+
+/**
+  Notification function of EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE.
+
+  @param[in]  Event        Event whose notification function is being invoked.
+  @param[in]  Context      Pointer to the notification function's context.
+
+**/
+STATIC
+VOID
+EFIAPI
+VariableAddressChangeEvent (
+  IN EFI_EVENT                              Event,
+  IN VOID                                   *Context
+  )
+{
+  EfiConvertPointer (0x0, (VOID **)&mMmCommunication);
+}
+
+/**
+  Library entry point
+
+  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
+  @param[in] SystemTable    A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS       Library successfully initialized.
+
+**/
+EFI_STATUS
+EFIAPI
+RuntimeDxeMmCommunicateLibConstructor (
+  IN EFI_HANDLE                             ImageHandle,
+  IN EFI_SYSTEM_TABLE                       *SystemTable
+  )
+{
+  EFI_STATUS        Status;
+
+  Status = gBS->LocateProtocol (&gEfiMmCommunicationProtocolGuid, NULL,
+                  (VOID **)&mMmCommunication);
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Register the event to convert the pointer for runtime.
+  //
+  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
+                  VariableAddressChangeEvent, NULL,
+                  &gEfiEventVirtualAddressChangeGuid,
+                  &mVirtualAddressChangeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
new file mode 100644
index 000000000000..1e3332a649b0
--- /dev/null
+++ b/MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
@@ -0,0 +1,19 @@
+///** @file
+//  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+//
+//  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+//
+//  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.
+//
+//**/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate."
+
-- 
2.20.1

[PATCH 3/4] StandaloneMmPkg: add implementation of MmCommunicateLib

[Ard Biesheuvel]

Add an implementation of MmCommunicateLib based on standalone MM.
This version passes the virtual address of the communication buffer
into the MM communicate protocol method.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf |  51 +++++++++
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c   | 113 ++++++++++++++++++++
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni |  19 ++++
 3 files changed, 183 insertions(+)

diff --git a/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
new file mode 100644
index 000000000000..c29a8618b3c2
--- /dev/null
+++ b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
@@ -0,0 +1,51 @@
+## @file
+#  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+#
+#  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+#
+#  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                    = 0x0001001B
+  BASE_NAME                      = RuntimeDxeMmCommunicateLib
+  MODULE_UNI_FILE                = RuntimeDxeMmCommunicateLib.uni
+  FILE_GUID                      = 15cd7ca8-ca1a-43a2-8f9c-eabbd3e7e5b5
+  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = MmCommunicateLib|DXE_RUNTIME_DRIVER
+  CONSTRUCTOR                    = RuntimeDxeMmCommunicateLibConstructor
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = ARM AARCH64 IA32 X64
+#
+
+[Sources]
+  RuntimeDxeMmCommunicateLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  UefiBootServicesTableLib
+  UefiRuntimeLib
+
+[Guids]
+  gEfiEventVirtualAddressChangeGuid             ## CONSUMES ## Event
+
+[Protocols]
+  gEfiMmCommunicationProtocolGuid               ## CONSUMES
+
+[Depex]
+  gEfiMmCommunicationProtocolGuid
diff --git a/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
new file mode 100644
index 000000000000..b5a203865360
--- /dev/null
+++ b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
@@ -0,0 +1,113 @@
+/** @file
+  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+
+  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+
+  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 <PiDxe.h>
+
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeLib.h>
+
+#include <Protocol/MmCommunication.h>
+
+STATIC EFI_MM_COMMUNICATION_PROTOCOL    *mMmCommunication;
+STATIC EFI_EVENT                        mVirtualAddressChangeEvent;
+
+/**
+  Invoke the MM communication protocol
+
+  @param[in] CommBuffer          A pointer to the buffer to convey into MMRAM.
+  @param[in] 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.
+
+  @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(s) or CommSize
+                                 parameter, if not omitted, are in address range
+                                 that cannot be accessed by the MM environment.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicate (
+  IN OUT VOID                              *PhysicalCommBuffer,
+  IN OUT VOID                              *VirtualCommBuffer,
+  IN OUT UINTN                             *CommSize OPTIONAL
+  )
+{
+  ASSERT (EfiAtRuntime () || (PhysicalCommBuffer == VirtualCommBuffer));
+
+  return mMmCommunication->Communicate (mMmCommunication, VirtualCommBuffer,
+                             CommSize);
+}
+
+/**
+  Notification function of EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE.
+
+  @param[in]  Event        Event whose notification function is being invoked.
+  @param[in]  Context      Pointer to the notification function's context.
+
+**/
+STATIC
+VOID
+EFIAPI
+VariableAddressChangeEvent (
+  IN EFI_EVENT                              Event,
+  IN VOID                                   *Context
+  )
+{
+  EfiConvertPointer (0x0, (VOID **)&mMmCommunication);
+}
+
+/**
+  Library entry point
+
+  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
+  @param[in] SystemTable    A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS       Library successfully initialized.
+
+**/
+EFI_STATUS
+EFIAPI
+RuntimeDxeMmCommunicateLibConstructor (
+  IN EFI_HANDLE                             ImageHandle,
+  IN EFI_SYSTEM_TABLE                       *SystemTable
+  )
+{
+  EFI_STATUS        Status;
+
+  Status = gBS->LocateProtocol (&gEfiMmCommunicationProtocolGuid, NULL,
+                  (VOID **)&mMmCommunication);
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Register the event to convert the pointer for runtime.
+  //
+  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
+                  VariableAddressChangeEvent, NULL,
+                  &gEfiEventVirtualAddressChangeGuid,
+                  &mVirtualAddressChangeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
new file mode 100644
index 000000000000..1e3332a649b0
--- /dev/null
+++ b/StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
@@ -0,0 +1,19 @@
+///** @file
+//  Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate
+//
+//  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+//
+//  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.
+//
+//**/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "Abstraction library for calls to EFI_MM_COMMUNICATE_PROTOCOL::Communicate."
+
-- 
2.20.1

[PATCH 4/4] MdeModulePkg/VariableSmmRuntimeDxe: switch to MmCommunicateLib library

[Ard Biesheuvel]

Replace direct calls to the EFI_SMM_COMMUNICATE protocol with calls
to the MmCommunicateLib library, which abstracts differences between
traditional MM and standalone MM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/MdeModulePkg.dsc                                        |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |  4 ++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 10 +++-------
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 84c2629d5adc..e8bafaf5007b 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -140,6 +140,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+  MmCommunicateLib|MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index bd73f7ac29f2..53ab1baa7974 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -56,13 +56,13 @@ [LibraryClasses]
   DebugLib
   UefiRuntimeLib
   DxeServicesTableLib
+  MmCommunicateLib
   UefiDriverEntryPoint
   TpmMeasurementLib
 
 [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES
   gEfiVariableArchProtocolGuid                  ## PRODUCES
-  gEfiSmmCommunicationProtocolGuid              ## CONSUMES
   ## CONSUMES
   ## NOTIFY
   ## UNDEFINED # Used to do smm communication
@@ -88,7 +88,7 @@ [Guids]
   gEfiImageSecurityDatabaseGuid
 
 [Depex]
-  gEfiSmmCommunicationProtocolGuid
+  TRUE
 
 [UserExtensions.TianoCore."ExtraFiles"]
   VariableSmmRuntimeDxeExtra.uni
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 85d655dc19ff..e52913a1eb6f 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -34,6 +34,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/MmCommunicateLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiRuntimeLib.h>
 #include <Library/BaseMemoryLib.h>
@@ -49,7 +50,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 EFI_HANDLE                       mHandle                    = NULL;
 EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable               = NULL;
 EFI_EVENT                        mVirtualAddressChangeEvent = NULL;
-EFI_SMM_COMMUNICATION_PROTOCOL  *mSmmCommunication          = NULL;
 UINT8                           *mVariableBuffer            = NULL;
 UINT8                           *mVariableBufferPhysical    = NULL;
 UINTN                            mVariableBufferSize;
@@ -179,7 +179,7 @@ SendCommunicateBuffer (
   SMM_VARIABLE_COMMUNICATE_HEADER           *SmmVariableFunctionHeader;
 
   CommSize = DataSize + SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;
-  Status = mSmmCommunication->Communicate (mSmmCommunication, mVariableBufferPhysical, &CommSize);
+  Status = MmCommunicate (mVariableBufferPhysical, mVariableBuffer, &CommSize);
   ASSERT_EFI_ERROR (Status);
 
   SmmCommunicateHeader      = (EFI_SMM_COMMUNICATE_HEADER *) mVariableBuffer;
@@ -898,7 +898,6 @@ VariableAddressChangeEvent (
   )
 {
   EfiConvertPointer (0x0, (VOID **) &mVariableBuffer);
-  EfiConvertPointer (0x0, (VOID **) &mSmmCommunication);
 }
 
 /**
@@ -954,7 +953,7 @@ GetVariablePayloadSize (
   //
   // Send data to SMM.
   //
-  Status = mSmmCommunication->Communicate (mSmmCommunication, CommBuffer, &CommSize);
+  Status = MmCommunicate (CommBuffer, CommBuffer, &CommSize);
   ASSERT_EFI_ERROR (Status);
 
   Status = SmmVariableFunctionHeader->ReturnStatus;
@@ -996,9 +995,6 @@ SmmVariableReady (
     return;
   }
 
-  Status = gBS->LocateProtocol (&gEfiSmmCommunicationProtocolGuid, NULL, (VOID **) &mSmmCommunication);
-  ASSERT_EFI_ERROR (Status);
-
   //
   // Allocate memory for variable communicate buffer.
   //
-- 
2.20.1

Re: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

[Wu, Hao A]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, March 11, 2019 11:36 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel; Wang, Jian J; Wu, Hao A; Zeng, Star; Kinney, Michael D;
> Gao, Liming; Achin Gupta; Yao, Jiewen; Supreeth Venkatesh; Jagadeesh Ujja
> Subject: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA
> vs PA ambiguity
> 
> This series proposes one possible approach to work around the issue that the
> traditional MM and standalone MM implement versions of the communicate
> protocol
> that are fundamentally incompatible from the point of view of the caller.
> 
> In traditional MM, the MM communicate protocol takes a physical pointer for
> the buffer, so that the SMM execution context can access the memory
> directly
> without having to translate it according to the translation regime of the
> caller.
> 
> In standalone MM, the buffer that is shared with the MM context is
> preallocated,
> and so it is up to the implementation of the MM communicate protocol to
> copy the
> data from the caller allocated buffer into the preallocated shared buffer. In
> order to be able to do so, the DXE driver needs to copy the contents, and for
> this it needs to know the virtual address not the physical address.
> 
> So this means we have two incompatible versions of the same protocol, and
> given
> that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID
> for the new
> EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish
> programmatically between a
> MM context that takes physical addresses vs one that takes virtual ones.
> 
> Since this is known at build time, one way to deal with this is to have two
> different implementations of a library that defines an abstract
> MmCommunicate()
> function, allowing the correct implementation to be selected at integration
> time.

Hello Ard,

It seems to me that for platforms that include the VariableSmmRuntimeDxe
driver, they need to add the 'MmCommunicateLib' dependency.

Please grant us some time to evaluate this proposal and its impact. We
will inform you as soon as there is a result. Thanks.


Best Regards,
Hao Wu

> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Achin Gupta <achin.gupta@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> 
> Ard Biesheuvel (4):
>   MdeModulePkg: introduce MmCommunicationLib library class
>   MdeModulePkg: add implementation of MmCommunicateLib
>   StandaloneMmPkg: add implementation of MmCommunicateLib
>   MdeModulePkg/VariableSmmRuntimeDxe: switch to MmCommunicateLib
> library
> 
>  MdeModulePkg/MdeModulePkg.dec                                                     |   4 +
>  MdeModulePkg/MdeModulePkg.dsc                                                     |   2 +
> 
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.inf    |  51 +++++++++
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> nf              |   4 +-
> 
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.inf |  51 +++++++++
>  MdeModulePkg/Include/Library/MmCommunicateLib.h                                   |
> 50 +++++++++
> 
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.c      | 114 ++++++++++++++++++++
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c                |  10 +-
> 
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.c   | 113 +++++++++++++++++++
> 
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.uni    |  19 ++++
> 
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.uni |  19 ++++
>  11 files changed, 428 insertions(+), 9 deletions(-)
>  create mode 100644
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.inf
>  create mode 100644
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.inf
>  create mode 100644
> MdeModulePkg/Include/Library/MmCommunicateLib.h
>  create mode 100644
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.c
>  create mode 100644
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.c
>  create mode 100644
> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC
> ommunicateLib.uni
>  create mode 100644
> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM
> mCommunicateLib.uni
> 
> --
> 2.20.1

Re: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

[Ard Biesheuvel]

On Fri, 15 Mar 2019 at 03:17, Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Monday, March 11, 2019 11:36 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel; Wang, Jian J; Wu, Hao A; Zeng, Star; Kinney, Michael D;
> > Gao, Liming; Achin Gupta; Yao, Jiewen; Supreeth Venkatesh; Jagadeesh Ujja
> > Subject: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA
> > vs PA ambiguity
> >
> > This series proposes one possible approach to work around the issue that the
> > traditional MM and standalone MM implement versions of the communicate
> > protocol
> > that are fundamentally incompatible from the point of view of the caller.
> >
> > In traditional MM, the MM communicate protocol takes a physical pointer for
> > the buffer, so that the SMM execution context can access the memory
> > directly
> > without having to translate it according to the translation regime of the
> > caller.
> >
> > In standalone MM, the buffer that is shared with the MM context is
> > preallocated,
> > and so it is up to the implementation of the MM communicate protocol to
> > copy the
> > data from the caller allocated buffer into the preallocated shared buffer. In
> > order to be able to do so, the DXE driver needs to copy the contents, and for
> > this it needs to know the virtual address not the physical address.
> >
> > So this means we have two incompatible versions of the same protocol, and
> > given
> > that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID
> > for the new
> > EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish
> > programmatically between a
> > MM context that takes physical addresses vs one that takes virtual ones.
> >
> > Since this is known at build time, one way to deal with this is to have two
> > different implementations of a library that defines an abstract
> > MmCommunicate()
> > function, allowing the correct implementation to be selected at integration
> > time.
>
> Hello Ard,
>
> It seems to me that for platforms that include the VariableSmmRuntimeDxe
> driver, they need to add the 'MmCommunicateLib' dependency.
>
> Please grant us some time to evaluate this proposal and its impact. We
> will inform you as soon as there is a result. Thanks.
>

Thank you Hao.

Note that we intend to discuss the issue addressed by this series in
the PIWG call, but the next one is at least two weeks away.

So there is no urgency to reviewing this patch for inclusion, but any
feedback you can give is appreciated.