From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
To: Supreeth Venkatesh <supreeth.venkatesh@arm.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>
Subject: Re: [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
Date: Wed, 13 Dec 2017 11:26:04 +0000 [thread overview]
Message-ID: <DB5PR04MB09980196E1FFCF4DA8307D9C8E350@DB5PR04MB0998.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20171117164805.13119-3-supreeth.venkatesh@arm.com>
Hi Supreeth,
Few comments inline.
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Supreeth Venkatesh
> Sent: Friday, November 17, 2017 10:18 PM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org
> Subject: [edk2] [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add
> EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
>
> 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
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Finfo
> center.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0060a%2FDEN0060A_
> ARM_MM_Interface_Specification.pdf&data=02%7C01%7Cmeenakshi.aggar
> wal%40nxp.com%7C27ae12fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C1%7C636465341155830632&sdata=pYKl2uUUF
> VCS4M87y%2BRK8TE852QqcusN0Fm208IkjtU%3D&reserved=0
> 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 <achin.gupta@arm.com>
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
> .../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 0000000000..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
> +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C27ae12
> fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636465341155830632&sdata=GPd9o7ovyTU10etIxP%2BBYNsYUKq
> m37tPcc%2BQDKtext4%3D&reserved=0
> +
> + 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>
> +
> +//
> +// 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] 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. 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_ARGS CommunicateSmcArgs;
> + EFI_STATUS Status;
> + UINTN BufferSize;
> +
> + CommunicateHeader = CommBuffer;
> + Status = EFI_ACCESS_DENIED;
> + 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) {
> + if (*CommSize == 0) {
> + *CommSize = mNsCommBuffMemRegion.Length;
> + return EFI_BAD_BUFFER_SIZE;
> + }
> + //
> + // CommSize must hold HeaderGuid and MessageLength
> + //
> + if (*CommSize < sizeof (EFI_MM_COMMUNICATE_HEADER)) {
> + return EFI_INVALID_PARAMETER;
> + }
> + BufferSize = *CommSize;
> + } else {
> + BufferSize = CommunicateHeader->MessageLength +
> + sizeof (CommunicateHeader->HeaderGuid) +
> + sizeof (CommunicateHeader->MessageLength);
> + }
> +
> + //
> + // If the buffer size is 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 -
> + sizeof (CommunicateHeader->HeaderGuid) -
> + sizeof (CommunicateHeader->MessageLength);
> + 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;
> +
> + if (mNsCommBuffMemRegion.VirtualBase) {
> + CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase,
> CommBuffer, BufferSize);
> + } else {
> + return EFI_ACCESS_DENIED;
> + }
> +
> + // 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;
> +
> + // comm_size_address is a PA or an IPA that holds the size of the
> + // communication buffer being passed in. This parameter is optional
> + // and can be omitted by passing a zero.
> + // ARM does not recommend using it since this might require the
> + // implementation to create a separate memory mapping for the
> parameter.
> + // ARM recommends storing the buffer size in the buffer itself.
> + CommunicateSmcArgs.Arg3 = 0;
> +
> + // Call the Standalone MM environment.
> + ArmCallSmc (&CommunicateSmcArgs);
> +
> + switch (CommunicateSmcArgs.Arg0) {
> + case ARM_SMC_MM_RET_SUCCESS:
> + // On exit, the size of data being returned is inferred from
> + // CommSize or MessageLength + Header.
> + CopyMem (CommBuffer,
> + (const VOID *)mNsCommBuffMemRegion.VirtualBase,
> + BufferSize);
> + 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:
> + Status = EFI_ACCESS_DENIED;
> + ASSERT (0);
> + }
> +
> + return Status;
> +}
> +
> +//
> +// MM Communication Protocol instance
> +//
> +EFI_MM_COMMUNICATION_PROTOCOL mMmCommunication = {
> + MmCommunicationCommunicate
> +};
> +
> +/**
> + 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 ((DEBUG_ERROR, "NotifySetVirtualAddressMap():"
> + " Unable to convert MM runtime pointer. Status:0x%r\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 = PcdGet64 (PcdMmBufferBase);
> + // During boot , Virtual and Physical are same
> + mNsCommBuffMemRegion.VirtualBase =
> mNsCommBuffMemRegion.PhysicalBase;
> + mNsCommBuffMemRegion.Length = PcdGet64 (PcdMmBufferSize);
> +
> + if (mNsCommBuffMemRegion.PhysicalBase == 0) {
> + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: "
> + "Invalid MM Buffer Base Address.\n"));
> + goto ReturnErrorStatus;
> + }
> +
[Meenakshi Aggarwal] VirtualBase address assignment should be done after checking PhysicalBase.
> + if (mNsCommBuffMemRegion.Length == 0) {
> + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: "
> + "Maximum Buffer Size is zero.\n"));
> + goto ReturnErrorStatus;
> + }
> +
> + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory,
> + mNsCommBuffMemRegion.PhysicalBase,
> + mNsCommBuffMemRegion.Length,
> + EFI_MEMORY_WB |
> + EFI_MEMORY_XP |
> + EFI_MEMORY_RUNTIME);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: "
> + "Failed to add MM-NS Buffer Memory Space\n"));
> + goto ReturnErrorStatus;
[Meenakshi Aggarwal] no need of goto statement here, we can directly return error as no cleanup is being performed under goto statement.
> + }
> +
> + Status = gDS-
> >SetMemorySpaceAttributes(mNsCommBuffMemRegion.PhysicalBase,
[Meenakshi Aggarwal] space before opening bracket please
> + mNsCommBuffMemRegion.Length,
> + EFI_MEMORY_WB | EFI_MEMORY_XP);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: "
> + "Failed to set MM-NS Buffer Memory attributes\n"));
> + goto CleanAddedMemorySpace;
> + }
> +
> + Status = gBS->AllocatePages (AllocateAddress,
> + EfiRuntimeServicesData,
> + EFI_SIZE_TO_PAGES (mNsCommBuffMemRegion.Length),
> + &mNsCommBuffMemRegion.PhysicalBase);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: "
> + "Failed to allocate MM-NS Buffer Memory Space\n"));
> + goto CleanAddedMemorySpace;
> + }
> +
> + // Install the communication protocol
> + Status = gBS->InstallProtocolInterface (&mMmCommunicateHandle,
> + &gEfiMmCommunicationProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + &mMmCommunication);
> + if (EFI_ERROR(Status)) {
[Meenakshi Aggarwal] Space after EFI_ERROR please
> + DEBUG ((DEBUG_ERROR, "MmCommunicationInitialize: "
> + "Failed to install MM communication protocol\n"));
> + goto CleanAllocatedPages;
> + }
> +
> + // 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
> + );
> + if (Status == EFI_SUCCESS) {
> + return Status;
> + }
> +
> + gBS->UninstallProtocolInterface(mMmCommunicateHandle,
> + &gEfiMmCommunicationProtocolGuid,
> + &mMmCommunication);
> +
> +CleanAllocatedPages:
> + gBS->FreePages (mNsCommBuffMemRegion.PhysicalBase,
> + EFI_SIZE_TO_PAGES (mNsCommBuffMemRegion.Length));
> +
> +CleanAddedMemorySpace:
> + gDS->RemoveMemorySpace (mNsCommBuffMemRegion.PhysicalBase,
> + mNsCommBuffMemRegion.Length);
> +
> +ReturnErrorStatus:
> + return EFI_INVALID_PARAMETER;
> +}
> 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
> +#
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C27ae12
> fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636465341155830632&sdata=GPd9o7ovyTU10etIxP%2BBYNsYUKq
> m37tPcc%2BQDKtext4%3D&reserved=0
> +#
> +# 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
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C27ae12fc4141
> 4e501f8208d52ddb09f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636465341155830632&sdata=pWv6hx1oHdtDyEnKx1HpyBnpmn7XikuwA85
> 8yu7148E%3D&reserved=0
next prev parent reply other threads:[~2017-12-13 11:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-17 16:48 [edk2 PATCH v3 0/2] *** EFI_MM_COMMUNICATION_PROTOCOL *** Supreeth Venkatesh
2017-11-17 16:48 ` [edk2 PATCH v3 1/2] ArmPkg: Add PCDs needed for MM communication driver Supreeth Venkatesh
2017-11-17 16:48 ` [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver Supreeth Venkatesh
2017-12-13 11:26 ` Meenakshi Aggarwal [this message]
2018-01-09 20:18 ` Supreeth Venkatesh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB5PR04MB09980196E1FFCF4DA8307D9C8E350@DB5PR04MB0998.eurprd04.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox