Re: [edk2] [PATCH v1 02/18] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

2018-05-04 Thread Supreeth Venkatesh
My response inline.

-Original Message-
From: Achin Gupta
Sent: Wednesday, April 11, 2018 9:00 AM
To: Supreeth Venkatesh 
Cc: edk2-devel@lists.01.org; michael.d.kin...@intel.com; liming@intel.com; 
jiewen@intel.com; leif.lindh...@linaro.org; ard.biesheu...@linaro.org; nd 

Subject: Re: [PATCH v1 02/18] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL 
DXE driver.

Hi Supreeth,

CIL.

On Fri, Apr 06, 2018 at 03:42:07PM +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.
>
> This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
> driver for AARCH64 platforms. It uses SMCs allocated from the standard
> SMC range defined in DEN0060A_ARM_MM_Interface_Specification.pdf
> to communicate with the standalone MM environment in the secure world.
>
> This patch also 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: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 339 
> +
>  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 +++
>  2 files changed, 389 insertions(+)
>  create mode 100644
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
>  create mode 100644
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
>
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> new file mode 100644
> index 00..e801c1c601
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -0,0 +1,339 @@
> +/** @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 
> +#include 
> +#include 
> +#include 
> +#include  #include 
> +#include  #include
> +
> +#include 
> +
> +#include 
> +
> +#include 
> +
> +//
> +// 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;
> +
> +/**
> +  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]  ThisThe EFI_MM_COMMUNICATION_PROTOCOL
> +  instance.
> +  @param[in, out] CommBuffer  A pointer to the buffer to convey
> +  into MMRAM.
> +  @param[in, out] CommSizeThe size of the data buffer being
> +  passed in. This is optional.
> +
> +  @retval EFI_SUCCESS The message was successfully posted.
> +  @retval EFI_INVALID_PARAMETER   The CommBuffer was NULL.
> +  @retval EFI_BAD_BUFFER_SIZE The buffer size is incorrect 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
> +**/ STATIC EFI_STATUS EFIAPI MmCommunicationCommunicate (
> +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> +  IN OUT VOID

Re: [edk2] [PATCH v1 02/18] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

2018-04-11 Thread Achin Gupta
Hi Supreeth,

CIL.

On Fri, Apr 06, 2018 at 03:42:07PM +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.
> 
> This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE runtime
> driver for AARCH64 platforms. It uses SMCs allocated from the standard
> SMC range defined in DEN0060A_ARM_MM_Interface_Specification.pdf
> to communicate with the standalone MM environment in the secure world.
> 
> This patch also 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: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 339 
> +
>  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 +++
>  2 files changed, 389 insertions(+)
>  create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
>  create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> 
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> new file mode 100644
> index 00..e801c1c601
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -0,0 +1,339 @@
> +/** @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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +
> +//
> +// 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;
> +
> +/**
> +  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]  ThisThe EFI_MM_COMMUNICATION_PROTOCOL
> +  instance.
> +  @param[in, out] CommBuffer  A pointer to the buffer to convey
> +  into MMRAM.
> +  @param[in, out] CommSizeThe size of the data buffer being
> +  passed in. This is optional.
> +
> +  @retval EFI_SUCCESS The message was successfully posted.
> +  @retval EFI_INVALID_PARAMETER   The CommBuffer was NULL.
> +  @retval EFI_BAD_BUFFER_SIZE The buffer size is incorrect 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
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +MmCommunicationCommunicate (
> +  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
> +  IN OUT VOID *CommBuffer,
> +  IN OUT UINTN*CommSize OPTIONAL
> +  )
> +{
> +  EFI_MM_COMMUNICATE_HEADER   *CommunicateHeader;
> +  ARM_SMC_ARGSCommunicateSmcArgs;
> +  EFI_STATUS  Status;
> +  UINTN   BufferSize;
> +
> +  CommunicateHeader = CommBuffer;
> +  Status = EFI_ACCESS_DENIED;
> +  BufferSize = 0;
> +
> 

[edk2] [PATCH v1 02/18] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

2018-04-06 Thread 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 defined in DEN0060A_ARM_MM_Interface_Specification.pdf
to communicate with the standalone MM environment in the secure world.

This patch also 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: Achin Gupta 
Signed-off-by: Supreeth Venkatesh 
---
 .../Drivers/MmCommunicationDxe/MmCommunication.c   | 339 +
 .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 +++
 2 files changed, 389 insertions(+)
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
 create mode 100644 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
new file mode 100644
index 00..e801c1c601
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -0,0 +1,339 @@
+/** @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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+//
+// 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;
+
+/**
+  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]  ThisThe EFI_MM_COMMUNICATION_PROTOCOL
+  instance.
+  @param[in, out] CommBuffer  A pointer to the buffer to convey
+  into MMRAM.
+  @param[in, out] CommSizeThe size of the data buffer being
+  passed in. This is optional.
+
+  @retval EFI_SUCCESS The message was successfully posted.
+  @retval EFI_INVALID_PARAMETER   The CommBuffer was NULL.
+  @retval EFI_BAD_BUFFER_SIZE The buffer size is incorrect 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
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+MmCommunicationCommunicate (
+  IN CONST EFI_MM_COMMUNICATION_PROTOCOL  *This,
+  IN OUT VOID *CommBuffer,
+  IN OUT UINTN*CommSize OPTIONAL
+  )
+{
+  EFI_MM_COMMUNICATE_HEADER   *CommunicateHeader;
+  ARM_SMC_ARGSCommunicateSmcArgs;
+  EFI_STATUS  Status;
+  UINTN   BufferSize;
+
+  CommunicateHeader = CommBuffer;
+  Status = EFI_ACCESS_DENIED;
+  BufferSize = 0;
+
+  ZeroMem (, 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) {
+if (*CommSize == 0) {
+  *CommSize = mNsCommBuffMemRegion.Length;
+  return EFI_BAD_BUFFER_SIZE;