From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D4379210E38B9 for ; Tue, 3 Jul 2018 07:12:11 -0700 (PDT) Received: by mail-wm0-x241.google.com with SMTP id n17-v6so2466613wmh.2 for ; Tue, 03 Jul 2018 07:12:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=p+kGi+2m+XYZxzookprpRjaYhnAtUX7mOrTclOaa9tg=; b=dKoSXZnhpA1lv4iOq6xYt6XTcpMUHBcnDH8H02+dspQP0M7q/IuHKhKF0gx5Qugr+I T+g2ZWYyrLY2fot0rFyZzeuZg+r41h6MX6MO/KXHoch1XmQfAOoXAwx+Zv0jzrXui71K znRYffKRpX+xFXATGBkAcm1+5NM5D9B9/eirM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=p+kGi+2m+XYZxzookprpRjaYhnAtUX7mOrTclOaa9tg=; b=tetRWXF2d950z9Jj3oayGtOKw2a2d/tNQmxQtBv3YWBwvnXJP4Tl+JLVWcAb6asPV9 4wSZ+2YyADB9rPInVcwf4MaR4lC/MMNPfkESUmIxZyzDg4wTtzkxX6WA9ZixW6cBbyv4 IUAUgowEoVxprktyiRJwa00PRvA3gj8geg7dUN0iUIeoxplGFu1pVaYVB52dux2+hvlR 86IJVDANAfp3Bkq78mx0ujC62u1soT53+GJip9/D4P+XX8vO28F+ea4DWnQUy5oVnZhK KcGJU/AC45z/xLTrRpSug8o/tWanNd6x1qrbfxeLjjwKyylfH7526QJHlptRdsNTc4jP JxwQ== X-Gm-Message-State: APt69E3pAeo9T1dNwt+zuG6LBPhZ+Rl9Dr+Jai7B/V5pCaTYmKMaicrC LzRHAQKW6r6XyPn/skiE10Yh7Q== X-Google-Smtp-Source: AAOMgpcc71alTsRR7cLTn+b6d5eWjYFNlKt84kthA2TIUCVejOtBrvUbrjCupfLhVMSdRbFrCl5eZw== X-Received: by 2002:a1c:7a19:: with SMTP id v25-v6mr10376380wmc.81.1530627129405; Tue, 03 Jul 2018 07:12:09 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id n8-v6sm2559615wmg.27.2018.07.03.07.12.08 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 03 Jul 2018 07:12:08 -0700 (PDT) Date: Tue, 3 Jul 2018 15:12:06 +0100 From: Leif Lindholm To: Supreeth Venkatesh Cc: edk2-devel@lists.01.org, Achin Gupta , Ard Biesheuvel Message-ID: <20180703141206.xyqthexlyucxknoo@bivouac.eciton.net> References: <1530611715-9819-1-git-send-email-supreeth.venkatesh@arm.com> <1530611715-9819-3-git-send-email-supreeth.venkatesh@arm.com> MIME-Version: 1.0 In-Reply-To: <1530611715-9819-3-git-send-email-supreeth.venkatesh@arm.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 2/6] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Jul 2018 14:12:12 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jul 03, 2018 at 03:25:11PM +0530, 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 I would prefer the document to be referred to by its official name and its document number: ARM Management Mode Interface Specification (ARM DEN0060A) > 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 Oh, and only one Signed-off-by per patch please. If authorship is to be indicated, ensure that's correct in git before calling format-patch. > Cc: Leif Lindholm > Cc: Ard Biesheuvel > --- > .../Drivers/MmCommunicationDxe/MmCommunication.c | 408 +++++++++++++++++++++ > .../Drivers/MmCommunicationDxe/MmCommunication.inf | 50 +++ And --stat/--stat-graph-width from https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 would mean I wouldn't have to cross-reference below to see which files are being added/deleted/modified. Also, the diff order makes a difference. So please look into the orderfile as described there - noting that this can now be set permanently in newer revisions of git with git config diff.orderFile > 2 files changed, 458 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 0000000..8ba1790 > --- /dev/null > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > @@ -0,0 +1,408 @@ > +/** @file > + > + Copyright (c) 2016-2018, 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 > + > +#define MM_MAJOR_VER_MASK 0xFFFF0000 Should this not actually be 0xEFFF0000? > +#define MM_MINOR_VER_MASK 0x0000FFFF > +#define MM_MAJOR_VER_SHIFT 16 Nothing wrong with this, but later code would be more easy to read if these were exposed through macros rather than explicitly extracted: #define MM_MAJOR_VER(x) (((x) & MM_MAJOR_VER_MASK) >> MM_MAJOR_VER_SHIFT) #define MM_MINOR_VER(x) ((x) & MM_MINOR_VER_MASK) > + > +const UINT32 MM_MAJOR_VER = 1; > +const UINT32 MM_MINOR_VER = 0; Meanwhile, these 1) Don't need to be given explicit variables (#define would work fine) 2) Have too generic names. What we're really talking about is MM_CALLER_MAJOR_VER and MM_CALLER_MINOR_VER? Finally please put these macros into a local .h file to be included. > + > +// > +// 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; > + > + Status = EFI_ACCESS_DENIED; > + BufferSize = 0; > + > + ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS)); > + > + // > + // Check parameters > + // > + if (CommBuffer == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + CommunicateHeader = CommBuffer; > + // CommBuffer is a mandatory parameter. Hence, Rely on > + // MessageLength + Header to ascertain the > + // total size of the communication payload rather than > + // rely on optional CommSize parameter > + BufferSize = CommunicateHeader->MessageLength + > + sizeof (CommunicateHeader->HeaderGuid) + > + sizeof (CommunicateHeader->MessageLength); > + > + // If the length of the CommBuffer is 0 then return the expected length. > + if (CommSize) { > + // This case can be used by the consumer of this driver to find out the > + // max size that can be used for allocating CommBuffer. > + if ((*CommSize == 0) || > + (*CommSize > mNsCommBuffMemRegion.Length)) { > + *CommSize = mNsCommBuffMemRegion.Length; > + return EFI_BAD_BUFFER_SIZE; > + } > + // > + // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER); > + // > + if (*CommSize != BufferSize) { > + return EFI_INVALID_PARAMETER; > + } > + } > + > + // > + // 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; > + > + // Copy Communication Payload > + CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBuffer, BufferSize); > + > + // For the SMC64 version, this parameter is a 64-bit Physical Address (PA) > + // or Intermediate Physical Address (IPA). PA or IPA is not really relevant to this context. On the whole, there are a lot of comments in this patch that are verbatim from the specification. While that does make it easier to review the patch for initial inclusion, it adds a lot of text not generally relevant to what the code is doing. Where comments are needed, they should mention what is being done and why. > + // For the SMC32 version, this parameter is a 32-bit PA or IPA. But we've already hardcoded above that we always use AARCH64? The use of (UINTN) below is sufficient to indicate that the intent is to use the native register width of the current execution state. I don't think the above 3 comment lines convey anything useful here. Just say // comm_buffer_address (64-bit physical address) ? > + 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. Here, the following would suffice: // comm_size_address (not used, indicated by setting to zero) > + CommunicateSmcArgs.Arg3 = 0; > + > + // Call the Standalone MM environment. > + ArmCallSmc (&CommunicateSmcArgs); > + > + switch (CommunicateSmcArgs.Arg0) { > + case ARM_SMC_MM_RET_SUCCESS: > + ZeroMem (CommBuffer, BufferSize); > + // On exit, the size of data being returned is inferred from exit? > + // MessageLength + Header. > + CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)mNsCommBuffMemRegion.VirtualBase; > + BufferSize = CommunicateHeader->MessageLength + > + sizeof (CommunicateHeader->HeaderGuid) + > + sizeof (CommunicateHeader->MessageLength); > + > + // Note: Very important to ensure that the consumer of this driver > + // has allocated CommBuffer sufficiently so that the return data > + // can be copied. Otherwise, this will lead to buffer overflow. > + // Assumption: CommBuffer = malloc (mNsCommBuffMemRegion.Length) > + // This guidance should be in the PI specification. TODO: ECR. Do we need to point out anywhere that passes a buffer and a size that the buffer must not be greater than the size? Am I missing some subtlety here? > + CopyMem (CommBuffer, > + (const VOID *)mNsCommBuffMemRegion.VirtualBase, Why an explicit const? If needed, it should be CONST. > + BufferSize); > + Status = EFI_SUCCESS; > + break; > + > + case ARM_SMC_MM_RET_INVALID_PARAMS: > + Status = EFI_INVALID_PARAMETER; > + break; > + > + case ARM_SMC_MM_RET_DENIED: > + Status = EFI_ACCESS_DENIED; > + break; > + > + case ARM_SMC_MM_RET_NO_MEMORY: > + // Unexpected error since the CommSize was checked for zero length > + // prior to issuing the SMC Worth stating that you're intentionally falling through to default. It would potentially be useful to have a separate assert here, and a different return value. > + 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)); > + } > + > +} > + > +STATIC > +EFI_STATUS > +GetMmVersion () This function name is misleading. We are checking whether what's on the other side of the SMC call is compatible with us. Nothing is returned about what version it is. > +{ > + EFI_STATUS Status; > + UINT16 MmMajorVersion; > + UINT16 MmMinorVersion; > + UINT32 MmVersion; So, I would suggest simplifying the function as follows: Drop the 16-bit variables, they can use the macros in place. Rename MmVersion Implementation. > + ARM_SMC_ARGS MmVersionArgs; > + > + MmVersionArgs.Arg0 = ARM_SMC_ID_MM_VERSION_AARCH32; This could also use a comment on why hard-coded AARCH32. > + > + ArmCallSmc (&MmVersionArgs); > + > + MmVersion = MmVersionArgs.Arg0; > + > + MmMajorVersion = ((MmVersion & MM_MAJOR_VER_MASK) >> MM_MAJOR_VER_SHIFT); > + MmMinorVersion = ((MmVersion & MM_MINOR_VER_MASK) >> 0); So with the above macros, this would be MmMajorVersion = MM_MAJOR_VER (MmVersion); MmMinorVersion = MM_MINOR_VER (MmVersion); > + > + // Different major revision values indicate possibly incompatible functions. > + // For two revisions, A and B, for which the major revision values are > + // identical, if the minor revision value of revision B is greater than > + // the minor revision value of revision A, then every function in > + // revision A must work in a compatible way with revision B. > + // However, it is possible for revision B to have a higher > + // function count than revision A. Again, verbatim from specification, not commenting on the code. I would expect (given the spec) that the implementation must always be - equal to MM_CALLER_MAJOR_VER and - equal to or newer than MM_CALLER_MINOR_VER which is what the code below does. But it doesn't distinguish between which version belongs to the implementation and which belongs to the caller. > + if ((MmMajorVersion == MM_MAJOR_VER) && > + (MmMinorVersion >= MM_MINOR_VER)) If the above is replaced with: if ((MM_MAJOR_VER (Implementation) == CALLER_MAJOR_VER) && (MM_MINOR_VER (Implementation) >= CALLER_MINOR_VER)) then this logic is already clear, and the whole comment block above can be dropped. > + { Curly bracket at end of previous line. > + DEBUG ((DEBUG_INFO, "MM Version: Major=0x%x, Minor=0x%x\n", > + MmMajorVersion, MmMinorVersion)); > + Status = EFI_SUCCESS; > + } > + else > + { } else { > + DEBUG ((DEBUG_ERROR, "Incompatible MM Versions.\n Current Version: Major=0x%x, Minor=0x%x.\n Expected: Major=0x%x, Minor>=0x%x.\n", > + MmMajorVersion, MmMinorVersion, MM_MAJOR_VER, MM_MINOR_VER)); > + Status = EFI_UNSUPPORTED; > + } > + > + return 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; > + > + // Get Secure Partition Manager Version Information Indentation. > + Status = GetMmVersion (); > + if (EFI_ERROR(Status)) { > + goto ReturnErrorStatus; > + } > + > + 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; > + } > + > + 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; > + } > + > + Status = gDS->SetMemorySpaceAttributes (mNsCommBuffMemRegion.PhysicalBase, > + 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)) { > + 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 > + ); The comments for every parameter are a bit overkill. There's nothing fancy going on. > + 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 0000000..7797e3d > --- /dev/null > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > @@ -0,0 +1,50 @@ > +#/** @file > +# > +# DXE MM Communicate driver > +# > +# Copyright (c) 2016 - 2018, 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 > + Please drop this blank line. > + ENTRY_POINT = MmCommunicationInitialize > + For added clarity, it would be useful to add # # The following information is for reference only and not required by # the build tools. # # VALID_ARCHITECTURES = AARCH64 # / Leif > +[Sources.AARCH64] > + 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] > + gEfiCpuArchProtocolGuid > -- > 2.7.4 >