public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements
@ 2019-01-16 20:22 Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules Ard Biesheuvel
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

This series addresses a number of issues I ran into while bringing up
the standalone MM based authenticated variable store on the SynQuacer
(AArch64) platform.

Patches #1 - #3 are based on Jagadeesh's patch that imports some staging
code into StandaloneMmPkg, with the following changes:
- drop unused source files, GUID references are other unused bit,
- clean up comments referring to the MM core implementation.

Patches #4 - #9 are obvious fixes/improvements.

Patch #10 adds support for TE formatted MM_CORE_STANDALONE binaries.
This is useful given that the 4 KB section alignment we require in
AArch64 implementations of standalone MM (due to the strict separation
between code and date) results in 8 KB of wasted space at the start of
the firmware volume. This can be reduced to 4 KB when using a TE image
and the FIXED attribute in the associated [Rule] section, by leveraging
an existing optimization in the FFS generation code that aligns TE images
by reducing FFS padding rather than adding more.

Patch #11 is another space optimization: it reuses the existing support
for encapsulated compressed firmware volumes in FFS files to shrink the
size of the primary standalone MM FV considerably. Again, due to
alignment requirements, there is significant bloat in the uncompressed
images (4 KB for the PE/COFF header, and up to 4 KB per section for the
.text, .data and .reloc sections), making the absolute minimum size of
any trivial MM_STANDALONE module 16 KB.

Changes since v1:
- add patches #1 - #3
- add Supreeth's ack to patches #4 - #7

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>

Ard Biesheuvel (11):
  StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules
  StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation
  StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE
  StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib
    ref
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII
    strings
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
    ASSERT_EFI_ERROR()s
  StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime
    attribute
  StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE
    images
  StandaloneMmPkg/Core: permit encapsulated firmware volumes

 StandaloneMmPkg/Core/Dispatcher.c             |  30 +-
 StandaloneMmPkg/Core/FwVol.c                  |  99 ++-
 StandaloneMmPkg/Core/StandaloneMmCore.inf     |   1 +
 .../StandaloneMmCpu/AArch64/EventHandle.c     |   2 +-
 .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c |   6 +-
 .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h |   8 +-
 .../AArch64/StandaloneMmCpu.inf               |   4 +-
 .../AArch64/SetPermissions.c                  | 109 +--
 .../AArch64/StandaloneMmCoreEntryPoint.c      |   7 +-
 .../StandaloneMmCoreEntryPoint.inf            |   4 +
 .../StandaloneMmCoreHobLib.inf                |   2 +-
 .../StandaloneMmHobLib/StandaloneMmHobLib.c   | 649 ++++++++++++++
 .../StandaloneMmHobLib/StandaloneMmHobLib.inf |  45 +
 .../StandaloneMmMemoryAllocationLib.c         | 822 ++++++++++++++++++
 .../StandaloneMmMemoryAllocationLib.inf       |  42 +
 .../StandaloneMmPeCoffExtraActionLib.c        |   9 +-
 16 files changed, 1716 insertions(+), 123 deletions(-)
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

-- 
2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:24   ` Yao, Jiewen
  2019-01-16 20:22 ` [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation Ard Biesheuvel
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

This HobLib code is based on the staging implementation of
StandaloneMmPkg, with the following changes:
- drop the unused AArch64/StandaloneMmCoreHobLibInternal.c source file
- remove hack from HobLibConstructor()
- update code comments referring the MM core

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c   | 649 ++++++++++++++++++++
 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf |  45 ++
 2 files changed, 694 insertions(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
new file mode 100644
index 000000000000..cc1a08166470
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
@@ -0,0 +1,649 @@
+/** @file
+  HOB Library implementation for Standalone MM Core.
+
+Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.<BR>
+Copyright (c) 2018, 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 <PiMm.h>
+
+#include <Library/HobLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MmServicesTableLib.h>
+
+//
+// Cache copy of HobList pointer.
+//
+STATIC VOID *gHobList = NULL;
+
+/**
+  The constructor function caches the pointer to HOB list.
+
+  The constructor function gets the start address of HOB list from system configuration table.
+  It will ASSERT() if that operation fails and it will always return EFI_SUCCESS.
+
+  @param  ImageHandle     The firmware allocated handle for the image.
+  @param  MmSystemTable   A pointer to the MM System Table.
+
+  @retval EFI_SUCCESS     The constructor successfully gets HobList.
+  @retval Other value     The constructor can't get HobList.
+
+**/
+EFI_STATUS
+EFIAPI
+HobLibConstructor (
+  IN EFI_HANDLE             ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE    *MmSystemTable
+  )
+{
+  UINTN       Index;
+
+  for (Index = 0; Index < gMmst->NumberOfTableEntries; Index++) {
+    if (CompareGuid (&gEfiHobListGuid, &gMmst->MmConfigurationTable[Index].VendorGuid)) {
+      gHobList = gMmst->MmConfigurationTable[Index].VendorTable;
+      break;
+    }
+  }
+  return EFI_SUCCESS;
+}
+
+/**
+  Returns the pointer to the HOB list.
+
+  This function returns the pointer to first HOB in the list.
+  If the pointer to the HOB list is NULL, then ASSERT().
+
+  @return The pointer to the HOB list.
+
+**/
+VOID *
+EFIAPI
+GetHobList (
+  VOID
+  )
+{
+  UINTN       Index;
+
+  if (gHobList == NULL) {
+    for (Index = 0; Index < gMmst->NumberOfTableEntries; Index++) {
+      if (CompareGuid (&gEfiHobListGuid, &gMmst->MmConfigurationTable[Index].VendorGuid)) {
+        gHobList = gMmst->MmConfigurationTable[Index].VendorTable;
+        break;
+      }
+    }
+  }
+  ASSERT (gHobList != NULL);
+  return gHobList;
+}
+
+/**
+  Returns the next instance of a HOB type from the starting HOB.
+
+  This function searches the first instance of a HOB type from the starting HOB pointer.
+  If there does not exist such HOB type from the starting HOB pointer, it will return NULL.
+  In contrast with macro GET_NEXT_HOB(), this function does not skip the starting HOB pointer
+  unconditionally: it returns HobStart back if HobStart itself meets the requirement;
+  caller is required to use GET_NEXT_HOB() if it wishes to skip current HobStart.
+
+  If HobStart is NULL, then ASSERT().
+
+  @param  Type          The HOB type to return.
+  @param  HobStart      The starting HOB pointer to search from.
+
+  @return The next instance of a HOB type from the starting HOB.
+
+**/
+VOID *
+EFIAPI
+GetNextHob (
+  IN UINT16                 Type,
+  IN CONST VOID             *HobStart
+  )
+{
+  EFI_PEI_HOB_POINTERS  Hob;
+
+  ASSERT (HobStart != NULL);
+
+  Hob.Raw = (UINT8 *) HobStart;
+  //
+  // Parse the HOB list until end of list or matching type is found.
+  //
+  while (!END_OF_HOB_LIST (Hob)) {
+    if (Hob.Header->HobType == Type) {
+      return Hob.Raw;
+    }
+    Hob.Raw = GET_NEXT_HOB (Hob);
+  }
+  return NULL;
+}
+
+/**
+  Returns the first instance of a HOB type among the whole HOB list.
+
+  This function searches the first instance of a HOB type among the whole HOB list.
+  If there does not exist such HOB type in the HOB list, it will return NULL.
+
+  If the pointer to the HOB list is NULL, then ASSERT().
+
+  @param  Type          The HOB type to return.
+
+  @return The next instance of a HOB type from the starting HOB.
+
+**/
+VOID *
+EFIAPI
+GetFirstHob (
+  IN UINT16                 Type
+  )
+{
+  VOID      *HobList;
+
+  HobList = GetHobList ();
+  return GetNextHob (Type, HobList);
+}
+
+/**
+  Returns the next instance of the matched GUID HOB from the starting HOB.
+
+  This function searches the first instance of a HOB from the starting HOB pointer.
+  Such HOB should satisfy two conditions:
+  its HOB type is EFI_HOB_TYPE_GUID_EXTENSION, and its GUID Name equals to the input Guid.
+  If such a HOB from the starting HOB pointer does not exist, it will return NULL.
+  Caller is required to apply GET_GUID_HOB_DATA () and GET_GUID_HOB_DATA_SIZE ()
+  to extract the data section and its size information, respectively.
+  In contrast with macro GET_NEXT_HOB(), this function does not skip the starting HOB pointer
+  unconditionally: it returns HobStart back if HobStart itself meets the requirement;
+  caller is required to use GET_NEXT_HOB() if it wishes to skip current HobStart.
+
+  If Guid is NULL, then ASSERT().
+  If HobStart is NULL, then ASSERT().
+
+  @param  Guid          The GUID to match with in the HOB list.
+  @param  HobStart      A pointer to a Guid.
+
+  @return The next instance of the matched GUID HOB from the starting HOB.
+
+**/
+VOID *
+EFIAPI
+GetNextGuidHob (
+  IN CONST EFI_GUID         *Guid,
+  IN CONST VOID             *HobStart
+  )
+{
+  EFI_PEI_HOB_POINTERS  GuidHob;
+
+  GuidHob.Raw = (UINT8 *) HobStart;
+  while ((GuidHob.Raw = GetNextHob (EFI_HOB_TYPE_GUID_EXTENSION, GuidHob.Raw)) != NULL) {
+    if (CompareGuid (Guid, &GuidHob.Guid->Name)) {
+      break;
+    }
+    GuidHob.Raw = GET_NEXT_HOB (GuidHob);
+  }
+  return GuidHob.Raw;
+}
+
+/**
+  Returns the first instance of the matched GUID HOB among the whole HOB list.
+
+  This function searches the first instance of a HOB among the whole HOB list.
+  Such HOB should satisfy two conditions:
+  its HOB type is EFI_HOB_TYPE_GUID_EXTENSION and its GUID Name equals to the input Guid.
+  If such a HOB from the starting HOB pointer does not exist, it will return NULL.
+  Caller is required to apply GET_GUID_HOB_DATA () and GET_GUID_HOB_DATA_SIZE ()
+  to extract the data section and its size information, respectively.
+
+  If the pointer to the HOB list is NULL, then ASSERT().
+  If Guid is NULL, then ASSERT().
+
+  @param  Guid          The GUID to match with in the HOB list.
+
+  @return The first instance of the matched GUID HOB among the whole HOB list.
+
+**/
+VOID *
+EFIAPI
+GetFirstGuidHob (
+  IN CONST EFI_GUID         *Guid
+  )
+{
+  VOID      *HobList;
+
+  HobList = GetHobList ();
+  return GetNextGuidHob (Guid, HobList);
+}
+
+/**
+  Get the system boot mode from the HOB list.
+
+  This function returns the system boot mode information from the
+  PHIT HOB in HOB list.
+
+  If the pointer to the HOB list is NULL, then ASSERT().
+
+  @param  VOID
+
+  @return The Boot Mode.
+
+**/
+EFI_BOOT_MODE
+EFIAPI
+GetBootModeHob (
+  VOID
+  )
+{
+  EFI_HOB_HANDOFF_INFO_TABLE    *HandOffHob;
+
+  HandOffHob = (EFI_HOB_HANDOFF_INFO_TABLE *) GetHobList ();
+
+  return  HandOffHob->BootMode;
+}
+
+VOID *
+CreateHob (
+  IN  UINT16    HobType,
+  IN  UINT16    HobLength
+  )
+{
+  EFI_HOB_HANDOFF_INFO_TABLE  *HandOffHob;
+  EFI_HOB_GENERIC_HEADER      *HobEnd;
+  EFI_PHYSICAL_ADDRESS        FreeMemory;
+  VOID                        *Hob;
+
+  HandOffHob = GetHobList ();
+
+  HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
+
+  FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
+
+  if (FreeMemory < HobLength) {
+      return NULL;
+  }
+
+  Hob = (VOID*) (UINTN) HandOffHob->EfiEndOfHobList;
+  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobType = HobType;
+  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobLength = HobLength;
+  ((EFI_HOB_GENERIC_HEADER*) Hob)->Reserved = 0;
+
+  HobEnd = (EFI_HOB_GENERIC_HEADER*) ((UINTN)Hob + HobLength);
+  HandOffHob->EfiEndOfHobList = (EFI_PHYSICAL_ADDRESS) (UINTN) HobEnd;
+
+  HobEnd->HobType   = EFI_HOB_TYPE_END_OF_HOB_LIST;
+  HobEnd->HobLength = sizeof(EFI_HOB_GENERIC_HEADER);
+  HobEnd->Reserved  = 0;
+  HobEnd++;
+  HandOffHob->EfiFreeMemoryBottom = (EFI_PHYSICAL_ADDRESS) (UINTN) HobEnd;
+
+  return Hob;
+}
+
+/**
+  Builds a HOB for a loaded PE32 module.
+
+  This function builds a HOB for a loaded PE32 module.
+  If ModuleName is NULL, then ASSERT().
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  ModuleName              The GUID File Name of the module.
+  @param  MemoryAllocationModule  The 64 bit physical address of the module.
+  @param  ModuleLength            The length of the module in bytes.
+  @param  EntryPoint              The 64 bit physical address of the module entry point.
+
+**/
+VOID
+EFIAPI
+BuildModuleHob (
+  IN CONST EFI_GUID         *ModuleName,
+  IN EFI_PHYSICAL_ADDRESS   MemoryAllocationModule,
+  IN UINT64                 ModuleLength,
+  IN EFI_PHYSICAL_ADDRESS   EntryPoint
+  )
+{
+  EFI_HOB_MEMORY_ALLOCATION_MODULE  *Hob;
+
+  ASSERT (((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
+          ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0));
+
+  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));
+
+  CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
+  Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
+  Hob->MemoryAllocationHeader.MemoryLength      = ModuleLength;
+  Hob->MemoryAllocationHeader.MemoryType        = EfiBootServicesCode;
+
+  //
+  // Zero the reserved space to match HOB spec
+  //
+  ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob->MemoryAllocationHeader.Reserved));
+
+  CopyGuid (&Hob->ModuleName, ModuleName);
+  Hob->EntryPoint = EntryPoint;
+}
+
+/**
+  Builds a HOB that describes a chunk of system memory.
+
+  This function builds a HOB that describes a chunk of system memory.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  ResourceType        The type of resource described by this HOB.
+  @param  ResourceAttribute   The resource attributes of the memory described by this HOB.
+  @param  PhysicalStart       The 64 bit physical address of memory described by this HOB.
+  @param  NumberOfBytes       The length of the memory described by this HOB in bytes.
+
+**/
+VOID
+EFIAPI
+BuildResourceDescriptorHob (
+  IN EFI_RESOURCE_TYPE            ResourceType,
+  IN EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttribute,
+  IN EFI_PHYSICAL_ADDRESS         PhysicalStart,
+  IN UINT64                       NumberOfBytes
+  )
+{
+  EFI_HOB_RESOURCE_DESCRIPTOR  *Hob;
+
+  Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_RESOURCE_DESCRIPTOR));
+  ASSERT(Hob != NULL);
+
+  Hob->ResourceType      = ResourceType;
+  Hob->ResourceAttribute = ResourceAttribute;
+  Hob->PhysicalStart     = PhysicalStart;
+  Hob->ResourceLength    = NumberOfBytes;
+}
+
+/**
+  Builds a GUID HOB with a certain data length.
+
+  This function builds a customized HOB tagged with a GUID for identification
+  and returns the start address of GUID HOB data so that caller can fill the customized data.
+  The HOB Header and Name field is already stripped.
+  If Guid is NULL, then ASSERT().
+  If there is no additional space for HOB creation, then ASSERT().
+  If DataLength >= (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then ASSERT().
+
+  @param  Guid          The GUID to tag the customized HOB.
+  @param  DataLength    The size of the data payload for the GUID HOB.
+
+  @return The start address of GUID HOB data.
+
+**/
+VOID *
+EFIAPI
+BuildGuidHob (
+  IN CONST EFI_GUID              *Guid,
+  IN UINTN                       DataLength
+  )
+{
+  EFI_HOB_GUID_TYPE *Hob;
+
+  //
+  // Make sure that data length is not too long.
+  //
+  ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
+
+  Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16) (sizeof (EFI_HOB_GUID_TYPE) + DataLength));
+  CopyGuid (&Hob->Name, Guid);
+  return Hob + 1;
+}
+
+
+/**
+  Copies a data buffer to a newly-built HOB.
+
+  This function builds a customized HOB tagged with a GUID for identification,
+  copies the input data to the HOB data field and returns the start address of the GUID HOB data.
+  The HOB Header and Name field is already stripped.
+  If Guid is NULL, then ASSERT().
+  If Data is NULL and DataLength > 0, then ASSERT().
+  If there is no additional space for HOB creation, then ASSERT().
+  If DataLength >= (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then ASSERT().
+
+  @param  Guid          The GUID to tag the customized HOB.
+  @param  Data          The data to be copied into the data field of the GUID HOB.
+  @param  DataLength    The size of the data payload for the GUID HOB.
+
+  @return The start address of GUID HOB data.
+
+**/
+VOID *
+EFIAPI
+BuildGuidDataHob (
+  IN CONST EFI_GUID              *Guid,
+  IN VOID                        *Data,
+  IN UINTN                       DataLength
+  )
+{
+  VOID  *HobData;
+
+  ASSERT (Data != NULL || DataLength == 0);
+
+  HobData = BuildGuidHob (Guid, DataLength);
+
+  return CopyMem (HobData, Data, DataLength);
+}
+
+/**
+  Builds a Firmware Volume HOB.
+
+  This function builds a Firmware Volume HOB.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The base address of the Firmware Volume.
+  @param  Length        The size of the Firmware Volume in bytes.
+
+**/
+VOID
+EFIAPI
+BuildFvHob (
+  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN UINT64                      Length
+  )
+{
+  EFI_HOB_FIRMWARE_VOLUME  *Hob;
+
+  Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME));
+
+  Hob->BaseAddress = BaseAddress;
+  Hob->Length      = Length;
+}
+
+
+/**
+  Builds a EFI_HOB_TYPE_FV2 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV2 HOB.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The base address of the Firmware Volume.
+  @param  Length        The size of the Firmware Volume in bytes.
+  @param  FvName       The name of the Firmware Volume.
+  @param  FileName      The name of the file.
+
+**/
+VOID
+EFIAPI
+BuildFv2Hob (
+  IN          EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN          UINT64                      Length,
+  IN CONST    EFI_GUID                    *FvName,
+  IN CONST    EFI_GUID                    *FileName
+  )
+{
+  EFI_HOB_FIRMWARE_VOLUME2  *Hob;
+
+  Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2));
+
+  Hob->BaseAddress = BaseAddress;
+  Hob->Length      = Length;
+  CopyGuid (&Hob->FvName, FvName);
+  CopyGuid (&Hob->FileName, FileName);
+}
+
+
+/**
+  Builds a HOB for the CPU.
+
+  This function builds a HOB for the CPU.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  SizeOfMemorySpace   The maximum physical memory addressability of the processor.
+  @param  SizeOfIoSpace       The maximum physical I/O addressability of the processor.
+
+**/
+VOID
+EFIAPI
+BuildCpuHob (
+  IN UINT8                       SizeOfMemorySpace,
+  IN UINT8                       SizeOfIoSpace
+  )
+{
+  EFI_HOB_CPU  *Hob;
+
+  Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
+
+  Hob->SizeOfMemorySpace = SizeOfMemorySpace;
+  Hob->SizeOfIoSpace     = SizeOfIoSpace;
+
+  //
+  // Zero the reserved space to match HOB spec
+  //
+  ZeroMem (Hob->Reserved, sizeof (Hob->Reserved));
+}
+
+/**
+  Builds a HOB for the memory allocation.
+
+  This function builds a HOB for the memory allocation.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The 64 bit physical address of the memory.
+  @param  Length        The length of the memory allocation in bytes.
+  @param  MemoryType    Type of memory allocated by this HOB.
+
+**/
+VOID
+EFIAPI
+BuildMemoryAllocationHob (
+  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN UINT64                      Length,
+  IN EFI_MEMORY_TYPE             MemoryType
+  )
+{
+  EFI_HOB_MEMORY_ALLOCATION  *Hob;
+
+  ASSERT (((BaseAddress & (EFI_PAGE_SIZE - 1)) == 0) &&
+          ((Length & (EFI_PAGE_SIZE - 1)) == 0));
+
+  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION));
+
+  ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
+  Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
+  Hob->AllocDescriptor.MemoryLength      = Length;
+  Hob->AllocDescriptor.MemoryType        = MemoryType;
+  //
+  // Zero the reserved space to match HOB spec
+  //
+  ZeroMem (Hob->AllocDescriptor.Reserved, sizeof (Hob->AllocDescriptor.Reserved));
+}
+
+/**
+  Builds a HOB that describes a chunk of system memory with Owner GUID.
+
+  This function builds a HOB that describes a chunk of system memory.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  ResourceType        The type of resource described by this HOB.
+  @param  ResourceAttribute   The resource attributes of the memory described by this HOB.
+  @param  PhysicalStart       The 64 bit physical address of memory described by this HOB.
+  @param  NumberOfBytes       The length of the memory described by this HOB in bytes.
+  @param  OwnerGUID           GUID for the owner of this resource.
+
+**/
+VOID
+EFIAPI
+BuildResourceDescriptorWithOwnerHob (
+  IN EFI_RESOURCE_TYPE            ResourceType,
+  IN EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttribute,
+  IN EFI_PHYSICAL_ADDRESS         PhysicalStart,
+  IN UINT64                       NumberOfBytes,
+  IN EFI_GUID                     *OwnerGUID
+  )
+{
+  ASSERT (FALSE);
+}
+
+/**
+  Builds a Capsule Volume HOB.
+
+  This function builds a Capsule Volume HOB.
+  If the platform does not support Capsule Volume HOBs, then ASSERT().
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The base address of the Capsule Volume.
+  @param  Length        The size of the Capsule Volume in bytes.
+
+**/
+VOID
+EFIAPI
+BuildCvHob (
+  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN UINT64                      Length
+  )
+{
+  ASSERT (FALSE);
+}
+
+
+/**
+  Builds a HOB for the BSP store.
+
+  This function builds a HOB for BSP store.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The 64 bit physical address of the BSP.
+  @param  Length        The length of the BSP store in bytes.
+  @param  MemoryType    Type of memory allocated by this HOB.
+
+**/
+VOID
+EFIAPI
+BuildBspStoreHob (
+  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN UINT64                      Length,
+  IN EFI_MEMORY_TYPE             MemoryType
+  )
+{
+  ASSERT (FALSE);
+}
+
+/**
+  Builds a HOB for the Stack.
+
+  This function builds a HOB for the stack.
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param  BaseAddress   The 64 bit physical address of the Stack.
+  @param  Length        The length of the stack in bytes.
+
+**/
+VOID
+EFIAPI
+BuildStackHob (
+  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
+  IN UINT64                      Length
+  )
+{
+  ASSERT (FALSE);
+}
diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
new file mode 100644
index 000000000000..542a19cc4bec
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
@@ -0,0 +1,45 @@
+## @file
+# Instance of HOB Library for Standalone MM modules.
+#
+# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2018, 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                      = HobLib
+  FILE_GUID                      = 8262551B-AB2D-4E76-99FC-5EBB83F4988E
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  LIBRARY_CLASS                  = HobLib|MM_STANDALONE
+  CONSTRUCTOR                    = HobLibConstructor
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  StandaloneMmHobLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  MmServicesTableLib
+
+[Guids]
+  gEfiHobListGuid                               ## CONSUMES  ## SystemTable
+  gEfiHobMemoryAllocModuleGuid                  ## CONSUMES
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:23   ` Yao, Jiewen
  2019-01-16 20:22 ` [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE Ard Biesheuvel
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

This MemoryAllocationLib code is based on the staging implementation of
StandaloneMmPkg, with the following changes:
- use correct MODULE_TYPE
- include MmServicesTableLib instead of declaring gMmst directly
- update code comments referring to the MM core

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c   | 822 ++++++++++++++++++++
 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf |  42 +
 2 files changed, 864 insertions(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
new file mode 100644
index 000000000000..c7c1282babff
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
@@ -0,0 +1,822 @@
+/** @file
+  Support routines for memory allocation routines based on Standalone MM Core internal functions.
+
+  Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2018, ARM Limited. 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 <PiMm.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/MmServicesTableLib.h>
+
+/**
+  Allocates one or more 4KB pages of a certain memory type.
+
+  Allocates the number of 4KB pages of a certain memory type and returns a pointer to the allocated
+  buffer.  The buffer returned is aligned on a 4KB boundary.  If Pages is 0, then NULL is returned.
+  If there is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  @param  MemoryType            The type of memory to allocate.
+  @param  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalAllocatePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTN            Pages
+  )
+{
+  EFI_STATUS            Status;
+  EFI_PHYSICAL_ADDRESS  Memory;
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  Status = gMmst->MmAllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+    return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiBootServicesData.
+
+  Allocates the number of 4KB pages of type EfiBootServicesData and returns a pointer to the
+  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If Pages is 0, then NULL
+  is returned.  If there is not enough memory remaining to satisfy the request, then NULL is
+  returned.
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePages (
+  IN UINTN  Pages
+  )
+{
+  return InternalAllocatePages (EfiRuntimeServicesData, Pages);
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiRuntimeServicesData.
+
+  Allocates the number of 4KB pages of type EfiRuntimeServicesData and returns a pointer to the
+  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If Pages is 0, then NULL
+  is returned.  If there is not enough memory remaining to satisfy the request, then NULL is
+  returned.
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateRuntimePages (
+  IN UINTN  Pages
+  )
+{
+  return InternalAllocatePages (EfiRuntimeServicesData, Pages);
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiReservedMemoryType.
+
+  Allocates the number of 4KB pages of type EfiReservedMemoryType and returns a pointer to the
+  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If Pages is 0, then NULL
+  is returned.  If there is not enough memory remaining to satisfy the request, then NULL is
+  returned.
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateReservedPages (
+  IN UINTN  Pages
+  )
+{
+  return NULL;
+}
+
+/**
+  Frees one or more 4KB pages that were previously allocated with one of the page allocation
+  functions in the Memory Allocation Library.
+
+  Frees the number of 4KB pages specified by Pages from the buffer specified by Buffer.  Buffer
+  must have been allocated on a previous call to the page allocation services of the Memory
+  Allocation Library.  If it is not possible to free allocated pages, then this function will
+  perform no actions.
+
+  If Buffer was not allocated with a page allocation function in the Memory Allocation Library,
+  then ASSERT().
+  If Pages is zero, then ASSERT().
+
+  @param  Buffer                Pointer to the buffer of pages to free.
+  @param  Pages                 The number of 4 KB pages to free.
+
+**/
+VOID
+EFIAPI
+FreePages (
+  IN VOID   *Buffer,
+  IN UINTN  Pages
+  )
+{
+  EFI_STATUS  Status;
+
+  ASSERT (Pages != 0);
+  Status = gMmst->MmFreePages ((EFI_PHYSICAL_ADDRESS) (UINTN)Buffer, Pages);
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Allocates one or more 4KB pages of a certain memory type at a specified alignment.
+
+  Allocates the number of 4KB pages specified by Pages of a certain memory type with an alignment
+  specified by Alignment.  The allocated buffer is returned.  If Pages is 0, then NULL is returned.
+  If there is not enough memory at the specified alignment remaining to satisfy the request, then
+  NULL is returned.
+  If Alignment is not a power of two and Alignment is not zero, then ASSERT().
+  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
+
+  @param  MemoryType            The type of memory to allocate.
+  @param  Pages                 The number of 4 KB pages to allocate.
+  @param  Alignment             The requested alignment of the allocation.  Must be a power of two.
+                                If Alignment is zero, then byte alignment is used.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalAllocateAlignedPages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTN            Pages,
+  IN UINTN            Alignment
+  )
+{
+  EFI_STATUS            Status;
+  EFI_PHYSICAL_ADDRESS  Memory;
+  UINTN                 AlignedMemory;
+  UINTN                 AlignmentMask;
+  UINTN                 UnalignedPages;
+  UINTN                 RealPages;
+
+  //
+  // Alignment must be a power of two or zero.
+  //
+  ASSERT ((Alignment & (Alignment - 1)) == 0);
+
+  if (Pages == 0) {
+    return NULL;
+  }
+  if (Alignment > EFI_PAGE_SIZE) {
+    //
+    // Calculate the total number of pages since alignment is larger than page size.
+    //
+    AlignmentMask  = Alignment - 1;
+    RealPages      = Pages + EFI_SIZE_TO_PAGES (Alignment);
+    //
+    // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not overflow.
+    //
+    ASSERT (RealPages > Pages);
+
+    Status         = gMmst->MmAllocatePages (AllocateAnyPages, MemoryType, RealPages, &Memory);
+    if (EFI_ERROR (Status)) {
+      return NULL;
+    }
+    AlignedMemory  = ((UINTN)Memory + AlignmentMask) & ~AlignmentMask;
+    UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) Memory);
+    if (UnalignedPages > 0) {
+      //
+      // Free first unaligned page(s).
+      //
+      Status = gMmst->MmFreePages (Memory, UnalignedPages);
+      ASSERT_EFI_ERROR (Status);
+    }
+    Memory         = (EFI_PHYSICAL_ADDRESS)(AlignedMemory + EFI_PAGES_TO_SIZE (Pages));
+    UnalignedPages = RealPages - Pages - UnalignedPages;
+    if (UnalignedPages > 0) {
+      //
+      // Free last unaligned page(s).
+      //
+      Status = gMmst->MmFreePages (Memory, UnalignedPages);
+      ASSERT_EFI_ERROR (Status);
+    }
+  } else {
+    //
+    // Do not over-allocate pages in this case.
+    //
+    Status = gMmst->MmAllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
+    if (EFI_ERROR (Status)) {
+      return NULL;
+    }
+    AlignedMemory  = (UINTN) Memory;
+  }
+  return (VOID *) AlignedMemory;
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiBootServicesData at a specified alignment.
+
+  Allocates the number of 4KB pages specified by Pages of type EfiBootServicesData with an
+  alignment specified by Alignment.  The allocated buffer is returned.  If Pages is 0, then NULL is
+  returned.  If there is not enough memory at the specified alignment remaining to satisfy the
+  request, then NULL is returned.
+
+  If Alignment is not a power of two and Alignment is not zero, then ASSERT().
+  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+  @param  Alignment             The requested alignment of the allocation.  Must be a power of two.
+                                If Alignment is zero, then byte alignment is used.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateAlignedPages (
+  IN UINTN  Pages,
+  IN UINTN  Alignment
+  )
+{
+  return InternalAllocateAlignedPages (EfiRuntimeServicesData, Pages, Alignment);
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiRuntimeServicesData at a specified alignment.
+
+  Allocates the number of 4KB pages specified by Pages of type EfiRuntimeServicesData with an
+  alignment specified by Alignment.  The allocated buffer is returned.  If Pages is 0, then NULL is
+  returned.  If there is not enough memory at the specified alignment remaining to satisfy the
+  request, then NULL is returned.
+
+  If Alignment is not a power of two and Alignment is not zero, then ASSERT().
+  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+  @param  Alignment             The requested alignment of the allocation.  Must be a power of two.
+                                If Alignment is zero, then byte alignment is used.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateAlignedRuntimePages (
+  IN UINTN  Pages,
+  IN UINTN  Alignment
+  )
+{
+  return InternalAllocateAlignedPages (EfiRuntimeServicesData, Pages, Alignment);
+}
+
+/**
+  Allocates one or more 4KB pages of type EfiReservedMemoryType at a specified alignment.
+
+  Allocates the number of 4KB pages specified by Pages of type EfiReservedMemoryType with an
+  alignment specified by Alignment.  The allocated buffer is returned.  If Pages is 0, then NULL is
+  returned.  If there is not enough memory at the specified alignment remaining to satisfy the
+  request, then NULL is returned.
+
+  If Alignment is not a power of two and Alignment is not zero, then ASSERT().
+  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
+
+  @param  Pages                 The number of 4 KB pages to allocate.
+  @param  Alignment             The requested alignment of the allocation.  Must be a power of two.
+                                If Alignment is zero, then byte alignment is used.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateAlignedReservedPages (
+  IN UINTN  Pages,
+  IN UINTN  Alignment
+  )
+{
+  return NULL;
+}
+
+/**
+  Frees one or more 4KB pages that were previously allocated with one of the aligned page
+  allocation functions in the Memory Allocation Library.
+
+  Frees the number of 4KB pages specified by Pages from the buffer specified by Buffer.  Buffer
+  must have been allocated on a previous call to the aligned page allocation services of the Memory
+  Allocation Library.  If it is not possible to free allocated pages, then this function will
+  perform no actions.
+
+  If Buffer was not allocated with an aligned page allocation function in the Memory Allocation
+  Library, then ASSERT().
+  If Pages is zero, then ASSERT().
+
+  @param  Buffer                Pointer to the buffer of pages to free.
+  @param  Pages                 The number of 4 KB pages to free.
+
+**/
+VOID
+EFIAPI
+FreeAlignedPages (
+  IN VOID   *Buffer,
+  IN UINTN  Pages
+  )
+{
+  EFI_STATUS  Status;
+
+  ASSERT (Pages != 0);
+  Status = gMmst->MmFreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, Pages);
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Allocates a buffer of a certain pool type.
+
+  Allocates the number bytes specified by AllocationSize of a certain pool type and returns a
+  pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
+  returned.  If there is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  @param  MemoryType            The type of memory to allocate.
+  @param  AllocationSize        The number of bytes to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalAllocatePool (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTN            AllocationSize
+  )
+{
+  EFI_STATUS  Status;
+  VOID        *Memory;
+
+  Memory = NULL;
+
+  Status = gMmst->MmAllocatePool (MemoryType, AllocationSize, &Memory);
+  if (EFI_ERROR (Status)) {
+    Memory = NULL;
+  }
+  return Memory;
+}
+
+/**
+  Allocates a buffer of type EfiBootServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiBootServicesData and returns a
+  pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
+  returned.  If there is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePool (
+  IN UINTN  AllocationSize
+  )
+{
+  return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
+}
+
+/**
+  Allocates a buffer of type EfiRuntimeServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiRuntimeServicesData and returns
+  a pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
+  returned.  If there is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateRuntimePool (
+  IN UINTN  AllocationSize
+  )
+{
+  return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
+}
+
+/**
+  Allocates a buffer of type EfiReservedMemoryType.
+
+  Allocates the number bytes specified by AllocationSize of type EfiReservedMemoryType and returns
+  a pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
+  returned.  If there is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateReservedPool (
+  IN UINTN  AllocationSize
+  )
+{
+  return NULL;
+}
+
+/**
+  Allocates and zeros a buffer of a certain pool type.
+
+  Allocates the number bytes specified by AllocationSize of a certain pool type, clears the buffer
+  with zeros, and returns a pointer to the allocated buffer.  If AllocationSize is 0, then a valid
+  buffer of 0 size is returned.  If there is not enough memory remaining to satisfy the request,
+  then NULL is returned.
+
+  @param  PoolType              The type of memory to allocate.
+  @param  AllocationSize        The number of bytes to allocate and zero.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalAllocateZeroPool (
+  IN EFI_MEMORY_TYPE  PoolType,
+  IN UINTN            AllocationSize
+  )
+{
+  VOID  *Memory;
+
+  Memory = InternalAllocatePool (PoolType, AllocationSize);
+  if (Memory != NULL) {
+    Memory = ZeroMem (Memory, AllocationSize);
+  }
+  return Memory;
+}
+
+/**
+  Allocates and zeros a buffer of type EfiBootServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiBootServicesData, clears the
+  buffer with zeros, and returns a pointer to the allocated buffer.  If AllocationSize is 0, then a
+  valid buffer of 0 size is returned.  If there is not enough memory remaining to satisfy the
+  request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateZeroPool (
+  IN UINTN  AllocationSize
+  )
+{
+  return InternalAllocateZeroPool (EfiRuntimeServicesData, AllocationSize);
+}
+
+/**
+  Allocates and zeros a buffer of type EfiRuntimeServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiRuntimeServicesData, clears the
+  buffer with zeros, and returns a pointer to the allocated buffer.  If AllocationSize is 0, then a
+  valid buffer of 0 size is returned.  If there is not enough memory remaining to satisfy the
+  request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateRuntimeZeroPool (
+  IN UINTN  AllocationSize
+  )
+{
+  return InternalAllocateZeroPool (EfiRuntimeServicesData, AllocationSize);
+}
+
+/**
+  Allocates and zeros a buffer of type EfiReservedMemoryType.
+
+  Allocates the number bytes specified by AllocationSize of type EfiReservedMemoryType, clears the
+  buffer with zeros, and returns a pointer to the allocated buffer.  If AllocationSize is 0, then a
+  valid buffer of 0 size is returned.  If there is not enough memory remaining to satisfy the
+  request, then NULL is returned.
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateReservedZeroPool (
+  IN UINTN  AllocationSize
+  )
+{
+  return NULL;
+}
+
+/**
+  Copies a buffer to an allocated buffer of a certain pool type.
+
+  Allocates the number bytes specified by AllocationSize of a certain pool type, copies
+  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
+  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
+  is not enough memory remaining to satisfy the request, then NULL is returned.
+  If Buffer is NULL, then ASSERT().
+  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  PoolType              The type of pool to allocate.
+  @param  AllocationSize        The number of bytes to allocate and zero.
+  @param  Buffer                The buffer to copy to the allocated buffer.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalAllocateCopyPool (
+  IN EFI_MEMORY_TYPE  PoolType,
+  IN UINTN            AllocationSize,
+  IN CONST VOID       *Buffer
+  )
+{
+  VOID  *Memory;
+
+  ASSERT (Buffer != NULL);
+  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
+
+  Memory = InternalAllocatePool (PoolType, AllocationSize);
+  if (Memory != NULL) {
+     Memory = CopyMem (Memory, Buffer, AllocationSize);
+  }
+  return Memory;
+}
+
+/**
+  Copies a buffer to an allocated buffer of type EfiBootServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiBootServicesData, copies
+  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
+  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
+  is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  If Buffer is NULL, then ASSERT().
+  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+  @param  Buffer                The buffer to copy to the allocated buffer.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateCopyPool (
+  IN UINTN       AllocationSize,
+  IN CONST VOID  *Buffer
+  )
+{
+  return InternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer);
+}
+
+/**
+  Copies a buffer to an allocated buffer of type EfiRuntimeServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiRuntimeServicesData, copies
+  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
+  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
+  is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  If Buffer is NULL, then ASSERT().
+  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+  @param  Buffer                The buffer to copy to the allocated buffer.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateRuntimeCopyPool (
+  IN UINTN       AllocationSize,
+  IN CONST VOID  *Buffer
+  )
+{
+  return InternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer);
+}
+
+/**
+  Copies a buffer to an allocated buffer of type EfiReservedMemoryType.
+
+  Allocates the number bytes specified by AllocationSize of type EfiReservedMemoryType, copies
+  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
+  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
+  is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  If Buffer is NULL, then ASSERT().
+  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+  @param  Buffer                The buffer to copy to the allocated buffer.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateReservedCopyPool (
+  IN UINTN       AllocationSize,
+  IN CONST VOID  *Buffer
+  )
+{
+  return NULL;
+}
+
+/**
+  Reallocates a buffer of a specified memory type.
+
+  Allocates and zeros the number bytes specified by NewSize from memory of the type
+  specified by PoolType.  If OldBuffer is not NULL, then the smaller of OldSize and
+  NewSize bytes are copied from OldBuffer to the newly allocated buffer, and
+  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
+  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
+  enough memory remaining to satisfy the request, then NULL is returned.
+
+  If the allocation of the new buffer is successful and the smaller of NewSize and OldSize
+  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
+
+  @param  PoolType       The type of pool to allocate.
+  @param  OldSize        The size, in bytes, of OldBuffer.
+  @param  NewSize        The size, in bytes, of the buffer to reallocate.
+  @param  OldBuffer      The buffer to copy to the allocated buffer.  This is an optional
+                         parameter that may be NULL.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+InternalReallocatePool (
+  IN EFI_MEMORY_TYPE  PoolType,
+  IN UINTN            OldSize,
+  IN UINTN            NewSize,
+  IN VOID             *OldBuffer  OPTIONAL
+  )
+{
+  VOID  *NewBuffer;
+
+  NewBuffer = InternalAllocateZeroPool (PoolType, NewSize);
+  if (NewBuffer != NULL && OldBuffer != NULL) {
+    CopyMem (NewBuffer, OldBuffer, MIN (OldSize, NewSize));
+    FreePool (OldBuffer);
+  }
+  return NewBuffer;
+}
+
+/**
+  Reallocates a buffer of type EfiBootServicesData.
+
+  Allocates and zeros the number bytes specified by NewSize from memory of type
+  EfiBootServicesData.  If OldBuffer is not NULL, then the smaller of OldSize and
+  NewSize bytes are copied from OldBuffer to the newly allocated buffer, and
+  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
+  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
+  enough memory remaining to satisfy the request, then NULL is returned.
+
+  If the allocation of the new buffer is successful and the smaller of NewSize and OldSize
+  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
+
+  @param  OldSize        The size, in bytes, of OldBuffer.
+  @param  NewSize        The size, in bytes, of the buffer to reallocate.
+  @param  OldBuffer      The buffer to copy to the allocated buffer.  This is an optional
+                         parameter that may be NULL.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+ReallocatePool (
+  IN UINTN  OldSize,
+  IN UINTN  NewSize,
+  IN VOID   *OldBuffer  OPTIONAL
+  )
+{
+  return InternalReallocatePool (EfiRuntimeServicesData, OldSize, NewSize, OldBuffer);
+}
+
+/**
+  Reallocates a buffer of type EfiRuntimeServicesData.
+
+  Allocates and zeros the number bytes specified by NewSize from memory of type
+  EfiRuntimeServicesData.  If OldBuffer is not NULL, then the smaller of OldSize and
+  NewSize bytes are copied from OldBuffer to the newly allocated buffer, and
+  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
+  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
+  enough memory remaining to satisfy the request, then NULL is returned.
+
+  If the allocation of the new buffer is successful and the smaller of NewSize and OldSize
+  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
+
+  @param  OldSize        The size, in bytes, of OldBuffer.
+  @param  NewSize        The size, in bytes, of the buffer to reallocate.
+  @param  OldBuffer      The buffer to copy to the allocated buffer.  This is an optional
+                         parameter that may be NULL.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+ReallocateRuntimePool (
+  IN UINTN  OldSize,
+  IN UINTN  NewSize,
+  IN VOID   *OldBuffer  OPTIONAL
+  )
+{
+  return InternalReallocatePool (EfiRuntimeServicesData, OldSize, NewSize, OldBuffer);
+}
+
+/**
+  Reallocates a buffer of type EfiReservedMemoryType.
+
+  Allocates and zeros the number bytes specified by NewSize from memory of type
+  EfiReservedMemoryType.  If OldBuffer is not NULL, then the smaller of OldSize and
+  NewSize bytes are copied from OldBuffer to the newly allocated buffer, and
+  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
+  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
+  enough memory remaining to satisfy the request, then NULL is returned.
+
+  If the allocation of the new buffer is successful and the smaller of NewSize and OldSize
+  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
+
+  @param  OldSize        The size, in bytes, of OldBuffer.
+  @param  NewSize        The size, in bytes, of the buffer to reallocate.
+  @param  OldBuffer      The buffer to copy to the allocated buffer.  This is an optional
+                         parameter that may be NULL.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+ReallocateReservedPool (
+  IN UINTN  OldSize,
+  IN UINTN  NewSize,
+  IN VOID   *OldBuffer  OPTIONAL
+  )
+{
+  return NULL;
+}
+
+/**
+  Frees a buffer that was previously allocated with one of the pool allocation functions in the
+  Memory Allocation Library.
+
+  Frees the buffer specified by Buffer.  Buffer must have been allocated on a previous call to the
+  pool allocation services of the Memory Allocation Library.  If it is not possible to free pool
+  resources, then this function will perform no actions.
+
+  If Buffer was not allocated with a pool allocation function in the Memory Allocation Library,
+  then ASSERT().
+
+  @param  Buffer                Pointer to the buffer to free.
+
+**/
+VOID
+EFIAPI
+FreePool (
+  IN VOID   *Buffer
+  )
+{
+  EFI_STATUS    Status;
+
+  Status = gMmst->MmFreePool (Buffer);
+  ASSERT_EFI_ERROR (Status);
+}
+
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
new file mode 100644
index 000000000000..8776e34e7c06
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
@@ -0,0 +1,42 @@
+## @file
+# Memory Allocation Library instance standalone MM modules.
+#
+# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2018, 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                      = MemoryAllocationLib
+  FILE_GUID                      = 54646378-A9DC-473F-9BE1-BD027C4C76DE
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  LIBRARY_CLASS                  = MemoryAllocationLib|MM_STANDALONE
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  StandaloneMmMemoryAllocationLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  MmServicesTableLib
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:24   ` Yao, Jiewen
  2019-01-16 20:22 ` [PATCH v2 04/11] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone Ard Biesheuvel
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

Remove MM_STANDALONE from the list of permitted modules for this library.
It should only be used by the standalone MM core.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
index db19d3c926e8..ac036e31cf5e 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
@@ -24,7 +24,7 @@ [Defines]
   MODULE_TYPE                    = MM_CORE_STANDALONE
   VERSION_STRING                 = 1.0
   PI_SPECIFICATION_VERSION       = 0x00010032
-  LIBRARY_CLASS                  = HobLib|MM_CORE_STANDALONE MM_STANDALONE
+  LIBRARY_CLASS                  = HobLib|MM_CORE_STANDALONE
 
 #
 #  VALID_ARCHITECTURES           = AARCH64
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 04/11] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref Ard Biesheuvel
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

Fix a couple of occurrences of typo Standlone -> Standalone

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c       | 2 +-
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c   | 6 +++---
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h   | 8 +-------
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf | 4 ++--
 4 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
index 2814577b3fcc..25114821448a 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
@@ -65,7 +65,7 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = {
 STATIC EFI_MM_ENTRY_POINT     mMmEntryPoint = NULL;
 
 EFI_STATUS
-PiMmStandloneArmTfCpuDriverEntry (
+PiMmStandaloneArmTfCpuDriverEntry (
   IN UINTN EventId,
   IN UINTN CpuNumber,
   IN UINTN NsCommBufferAddr
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 85a9c108aea4..203a32baaaf9 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -74,7 +74,7 @@ GetGuidedHobData (
 }
 
 EFI_STATUS
-StandloneMmCpuInitialize (
+StandaloneMmCpuInitialize (
   IN EFI_HANDLE         ImageHandle,  // not actual imagehandle
   IN EFI_MM_SYSTEM_TABLE   *SystemTable  // not actual systemtable
   )
@@ -147,8 +147,8 @@ StandloneMmCpuInitialize (
   // Share the entry point of the CPU driver
   DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
           (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
-          (UINT64) PiMmStandloneArmTfCpuDriverEntry));
-  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandloneArmTfCpuDriverEntry;
+          (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;
 
   // Find the descriptor that contains the whereabouts of the buffer for
   // communication with the Normal world.
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
index 7b38b65e1242..543467f67576 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
@@ -40,7 +40,7 @@ extern MP_INFORMATION_HOB_DATA       *mMpInformationHobData;
 extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig;
 
 EFI_STATUS
-PiMmStandloneArmTfCpuDriverEntry (
+PiMmStandaloneArmTfCpuDriverEntry (
   IN UINTN EventId,
   IN UINTN CpuNumber,
   IN UINTN NsCommBufferAddr
@@ -55,10 +55,4 @@ PiMmCpuTpFwRootMmiHandler (
   IN OUT UINTN                    *CommBufferSize  OPTIONAL
   );
 
-EFI_STATUS _PiMmStandloneArmTfCpuDriverEntry (
-  IN UINTN EventId,
-  IN UINTN CpuNumber,
-  IN UINTN NsCommBufferAddr
-  );
-
 #endif
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
index 9e6bbabdb103..d261e51ebc75 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
@@ -18,12 +18,12 @@
 
 [Defines]
   INF_VERSION                    = 0x0001001A
-  BASE_NAME                      = StandloneMmCpu
+  BASE_NAME                      = StandaloneMmCpu
   FILE_GUID                      = 58F7A62B-6280-42A7-BC38-10535A64A92C
   MODULE_TYPE                    = MM_STANDALONE
   VERSION_STRING                 = 1.0
   PI_SPECIFICATION_VERSION       = 0x00010032
-  ENTRY_POINT                    = StandloneMmCpuInitialize
+  ENTRY_POINT                    = StandaloneMmCpuInitialize
 
 [Sources]
   StandaloneMmCpu.c
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 04/11] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:27   ` Yao, Jiewen
  2019-01-16 20:22 ` [PATCH v2 06/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings Ard Biesheuvel
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
so add SerialPortLib to its list of LibraryClasses.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 3222cd359f3e..769eaeeefbea 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -43,6 +43,7 @@ [Packages.AARCH64]
 [LibraryClasses]
   BaseLib
   DebugLib
+  SerialPortLib
 
 [LibraryClasses.AARCH64]
   StandaloneMmMmuLib
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 06/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 07/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s Ard Biesheuvel
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

PE/COFF section names are ASCII strings so use %a not %s.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
index 60c1f66b83fa..3ca7f6660f47 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
@@ -78,7 +78,7 @@ UpdateMmFoundationPeCoffPermissions (
             "%a: Section %d of image at 0x%lx has 0x%x permissions\n",
             __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.Characteristics));
     DEBUG ((DEBUG_INFO,
-            "%a: Section %d of image at 0x%lx has %s name\n",
+            "%a: Section %d of image at 0x%lx has %a name\n",
             __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.Name));
     DEBUG ((DEBUG_INFO,
             "%a: Section %d of image at 0x%lx has 0x%x address\n",
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 07/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 06/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 08/11] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute Ard Biesheuvel
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

ASSERT_EFI_ERROR (x) is a shorthand for ASSERT(!EFI_ERROR(x)), and so
it should only be used with EFI_STATUS type expressions.

So drop two instances that operate on other types, since neither looks
particularly useful.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 05ed6c8dd0b5..5cca532456fd 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -295,7 +295,6 @@ _ModuleEntryPoint (
   //
   ProcessModuleEntryPointList (HobStart);
 
-  ASSERT_EFI_ERROR (CpuDriverEntryPoint);
   DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64) CpuDriverEntryPoint));
 
 finish:
@@ -303,5 +302,4 @@ finish:
   InitMmFoundationSvcArgs.Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
   InitMmFoundationSvcArgs.Arg1 = Status;
   DelegatedEventLoop (&InitMmFoundationSvcArgs);
-  ASSERT_EFI_ERROR (0);
 }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 08/11] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 07/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice Ard Biesheuvel
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

The special handling of the EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER
attribute is only necessary for images that are relocated twice, i.e.,
in the context of SetVirtualAddressMap (). This does not apply to
standalone MM modules, so drop the check.

Drop some redundant DEBUG output while at it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
index 1c9fec201916..f6bfcc875751 100644
--- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
@@ -145,8 +145,7 @@ UpdatePeCoffPermissions (
 
     if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) {
 
-      if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0 &&
-          TmpContext.ImageType != EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER) {
+      if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0) {
 
         DEBUG ((DEBUG_INFO,
           "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and size 0x%x\n",
@@ -158,14 +157,10 @@ UpdatePeCoffPermissions (
           __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
       }
     } else {
-        DEBUG ((DEBUG_INFO,
-          "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and size 0x%x\n",
-           __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
-        ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
-
         DEBUG ((DEBUG_INFO,
           "%a: Mapping section %d of image at 0x%lx with RO-X permissions and size 0x%x\n",
           __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+        ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
         NoExecUpdater (Base, SectionHeader.Misc.VirtualSize);
     }
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (7 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 08/11] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:34   ` Yao, Jiewen
  2019-01-16 20:22 ` [PATCH v2 10/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images Ard Biesheuvel
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

The dispatcher uses the PE/COFF loader to load images into the heap,
but only does so after copying the entire image first, leading to
two copies being made for no good reason.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Core/Dispatcher.c | 30 +-------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
index 8d009b4f80c1..8a2ad5118d92 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -294,7 +294,6 @@ MmLoadImage (
   IN OUT EFI_MM_DRIVER_ENTRY  *DriverEntry
   )
 {
-  VOID                           *Buffer;
   UINTN                          PageCount;
   EFI_STATUS                     Status;
   EFI_PHYSICAL_ADDRESS           DstBuffer;
@@ -302,17 +301,12 @@ MmLoadImage (
 
   DEBUG ((DEBUG_INFO, "MmLoadImage - %g\n", &DriverEntry->FileName));
 
-  Buffer = AllocateCopyPool (DriverEntry->Pe32DataSize, DriverEntry->Pe32Data);
-  if (Buffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
   Status               = EFI_SUCCESS;
 
   //
   // Initialize ImageContext
   //
-  ImageContext.Handle = Buffer;
+  ImageContext.Handle = DriverEntry->Pe32Data;
   ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
 
   //
@@ -320,9 +314,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderGetImageInfo (&ImageContext);
   if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      MmFreePool (Buffer);
-    }
     return Status;
   }
 
@@ -336,9 +327,6 @@ MmLoadImage (
              &DstBuffer
              );
   if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      MmFreePool (Buffer);
-    }
     return Status;
   }
 
@@ -355,9 +343,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderLoadImage (&ImageContext);
   if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      MmFreePool (Buffer);
-    }
     MmFreePages (DstBuffer, PageCount);
     return Status;
   }
@@ -367,9 +352,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderRelocateImage (&ImageContext);
   if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      MmFreePool (Buffer);
-    }
     MmFreePages (DstBuffer, PageCount);
     return Status;
   }
@@ -393,9 +375,6 @@ MmLoadImage (
                                               (VOID **)&DriverEntry->LoadedImage
                                               );
     if (EFI_ERROR (Status)) {
-      if (Buffer != NULL) {
-        MmFreePool (Buffer);
-      }
       MmFreePages (DstBuffer, PageCount);
       return Status;
     }
@@ -482,13 +461,6 @@ MmLoadImage (
 
   DEBUG_CODE_END ();
 
-  //
-  // Free buffer allocated by Fv->ReadSection.
-  //
-  // The UEFI Boot Services FreePool() function must be used because Fv->ReadSection
-  // used the UEFI Boot Services AllocatePool() function
-  //
-  MmFreePool (Buffer);
   return Status;
 }
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 10/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (8 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-16 20:22 ` [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
  2019-01-18  9:26 ` [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Achin Gupta
  11 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

TE images take up less space when using 4 KB section alignment, since
the FFS/FV generation code optimizes away the redundant, nested padding.
This saves 4 KB of space, which is a worthwhile improvement for code
that executes in place in secure context.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c | 107 +++++++++-----------
 1 file changed, 46 insertions(+), 61 deletions(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
index 3ca7f6660f47..90299ebbafb6 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
@@ -143,9 +143,12 @@ LocateStandaloneMmCorePeCoffData (
 
   Status = FfsFindSectionData (EFI_SECTION_PE32, FileHeader, TeData, TeDataSize);
   if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Section data - 0x%x\n",
-              Status));
-    return Status;
+    Status = FfsFindSectionData (EFI_SECTION_TE, FileHeader, TeData, TeDataSize);
+    if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Section data - %r\n",
+                Status));
+      return Status;
+    }
   }
 
   DEBUG ((DEBUG_INFO, "Found Standalone MM PE data - 0x%x\n", *TeData));
@@ -155,10 +158,9 @@ LocateStandaloneMmCorePeCoffData (
 STATIC
 EFI_STATUS
 GetPeCoffSectionInformation (
-  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT      *ImageContext,
-  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT      *TmpContext,
-  IN  OUT   UINT32                            *SectionHeaderOffset,
-  IN  OUT   UINT16                            *NumberOfSections
+  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT      *ImageContext,
+      OUT   UINT32                            *SectionHeaderOffset,
+      OUT   UINT16                            *NumberOfSections
   )
 {
   RETURN_STATUS                         Status;
@@ -168,44 +170,29 @@ GetPeCoffSectionInformation (
   UINTN                                 ReadSize;
 
   ASSERT (ImageContext != NULL);
-  ASSERT (TmpContext != NULL);
   ASSERT (SectionHeaderOffset != NULL);
   ASSERT (NumberOfSections != NULL);
 
-  //
-  // We need to copy ImageContext since PeCoffLoaderGetImageInfo ()
-  // will mangle the ImageAddress field
-  //
-  CopyMem (TmpContext, ImageContext, sizeof (*TmpContext));
-
-  if (TmpContext->PeCoffHeaderOffset == 0) {
-    Status = PeCoffLoaderGetImageInfo (TmpContext);
-    if (RETURN_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR,
-              "%a: PeCoffLoaderGetImageInfo () failed (Status = %r)\n",
-              __FUNCTION__, Status));
-      return Status;
-    }
-  }
-
-  if (TmpContext->IsTeImage &&
-      TmpContext->ImageAddress == ImageContext->ImageAddress) {
-    DEBUG ((DEBUG_INFO, "%a: ignoring XIP TE image at 0x%lx\n", __FUNCTION__,
-            ImageContext->ImageAddress));
-    return RETURN_UNSUPPORTED;
+  Status = PeCoffLoaderGetImageInfo (ImageContext);
+  if (RETURN_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+            "%a: PeCoffLoaderGetImageInfo () failed (Status == %r)\n",
+            __FUNCTION__, Status));
+    return Status;
   }
 
-  if (TmpContext->SectionAlignment < EFI_PAGE_SIZE) {
+  if (ImageContext->SectionAlignment < EFI_PAGE_SIZE) {
     //
     // The sections need to be at least 4 KB aligned, since that is the
     // granularity at which we can tighten permissions.
     //
-    if (!TmpContext->IsTeImage) {
+    if (!ImageContext->IsTeImage) {
       DEBUG ((DEBUG_WARN,
               "%a: non-TE Image at 0x%lx has SectionAlignment < 4 KB (%lu)\n",
-              __FUNCTION__, ImageContext->ImageAddress, TmpContext->SectionAlignment));
+              __FUNCTION__, ImageContext->ImageAddress, ImageContext->SectionAlignment));
+      return RETURN_UNSUPPORTED;
     }
-    return RETURN_UNSUPPORTED;
+    ImageContext->SectionAlignment = EFI_PAGE_SIZE;
   }
 
   //
@@ -217,9 +204,9 @@ GetPeCoffSectionInformation (
   Hdr.Union = &HdrData;
   Size = sizeof (EFI_IMAGE_OPTIONAL_HEADER_UNION);
   ReadSize = Size;
-  Status = TmpContext->ImageRead (
-                         TmpContext->Handle,
-                         TmpContext->PeCoffHeaderOffset,
+  Status = ImageContext->ImageRead (
+                         ImageContext->Handle,
+                         ImageContext->PeCoffHeaderOffset,
                          &Size,
                          Hdr.Pe32
                          );
@@ -231,23 +218,28 @@ GetPeCoffSectionInformation (
     return Status;
   }
 
-  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
-
-  *SectionHeaderOffset = TmpContext->PeCoffHeaderOffset + sizeof (UINT32) +
-                        sizeof (EFI_IMAGE_FILE_HEADER);
-  *NumberOfSections    = Hdr.Pe32->FileHeader.NumberOfSections;
-
-  switch (Hdr.Pe32->OptionalHeader.Magic) {
-  case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC:
-    *SectionHeaderOffset += Hdr.Pe32->FileHeader.SizeOfOptionalHeader;
-    break;
-  case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC:
-    *SectionHeaderOffset += Hdr.Pe32Plus->FileHeader.SizeOfOptionalHeader;
-    break;
-  default:
-    ASSERT (FALSE);
+  if (!ImageContext->IsTeImage) {
+    ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
+
+    *SectionHeaderOffset = ImageContext->PeCoffHeaderOffset + sizeof (UINT32) +
+                          sizeof (EFI_IMAGE_FILE_HEADER);
+    *NumberOfSections    = Hdr.Pe32->FileHeader.NumberOfSections;
+
+    switch (Hdr.Pe32->OptionalHeader.Magic) {
+    case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC:
+      *SectionHeaderOffset += Hdr.Pe32->FileHeader.SizeOfOptionalHeader;
+      break;
+    case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC:
+      *SectionHeaderOffset += Hdr.Pe32Plus->FileHeader.SizeOfOptionalHeader;
+      break;
+    default:
+      ASSERT (FALSE);
+    }
+  } else {
+    *SectionHeaderOffset = (UINTN)(sizeof (EFI_TE_IMAGE_HEADER));
+    *NumberOfSections = Hdr.Te->NumberOfSections;
+    ImageContext->ImageAddress -= (UINT32)Hdr.Te->StrippedSize - sizeof (EFI_TE_IMAGE_HEADER);
   }
-
   return RETURN_SUCCESS;
 }
 
@@ -261,7 +253,6 @@ GetStandaloneMmCorePeCoffSections (
   )
 {
   EFI_STATUS                   Status;
-  PE_COFF_LOADER_IMAGE_CONTEXT TmpContext;
 
   // Initialize the Image Context
   ZeroMem (ImageContext, sizeof (PE_COFF_LOADER_IMAGE_CONTEXT));
@@ -270,15 +261,9 @@ GetStandaloneMmCorePeCoffSections (
 
   DEBUG ((DEBUG_INFO, "Found Standalone MM PE data - 0x%x\n", TeData));
 
-  Status = PeCoffLoaderGetImageInfo (ImageContext);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Core PE-COFF Image information - 0x%x\n", Status));
-    return Status;
-  }
-
-  Status = GetPeCoffSectionInformation (ImageContext, &TmpContext, SectionHeaderOffset, NumberOfSections);
+  Status = GetPeCoffSectionInformation (ImageContext, SectionHeaderOffset, NumberOfSections);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Core PE-COFF Section information - 0x%x\n", Status));
+    DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Core PE-COFF Section information - %r\n", Status));
     return Status;
   }
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (9 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 10/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images Ard Biesheuvel
@ 2019-01-16 20:22 ` Ard Biesheuvel
  2019-01-18 15:39   ` Yao, Jiewen
  2019-01-18  9:26 ` [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Achin Gupta
  11 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-16 20:22 UTC (permalink / raw)
  To: edk2-devel
  Cc: Ard Biesheuvel, Achin Gupta, Jiewen Yao, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

Standalone MM requires 4 KB section alignment for all images, so that
strict permissions can be applied. Unfortunately, this results in a
lot of wasted space, which is usually costly in the secure world
environment that standalone MM is expected to operate in.

So let's permit the standalone MM drivers (but not the core) to be
delivered in a compressed firmware volume.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 StandaloneMmPkg/Core/FwVol.c                                                            | 99 ++++++++++++++++++--
 StandaloneMmPkg/Core/StandaloneMmCore.inf                                               |  1 +
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c |  5 +
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf       |  3 +
 4 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 5abf98c24797..8eb827dda5c4 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include "StandaloneMmCore.h"
 #include <Library/FvLib.h>
+#include <Library/ExtractGuidedSectionLib.h>
 
 //
 // List of file types supported by dispatcher
@@ -65,15 +66,25 @@ Returns:
 
 --*/
 {
-  EFI_STATUS          Status;
-  EFI_STATUS          DepexStatus;
-  EFI_FFS_FILE_HEADER *FileHeader;
-  EFI_FV_FILETYPE     FileType;
-  VOID                *Pe32Data;
-  UINTN               Pe32DataSize;
-  VOID                *Depex;
-  UINTN               DepexSize;
-  UINTN               Index;
+  EFI_STATUS                              Status;
+  EFI_STATUS                              DepexStatus;
+  EFI_FFS_FILE_HEADER                     *FileHeader;
+  EFI_FV_FILETYPE                         FileType;
+  VOID                                    *Pe32Data;
+  UINTN                                   Pe32DataSize;
+  VOID                                    *Depex;
+  UINTN                                   DepexSize;
+  UINTN                                   Index;
+  EFI_COMMON_SECTION_HEADER               *Section;
+  VOID                                    *SectionData;
+  UINTN                                   SectionDataSize;
+  UINT32                                  DstBufferSize;
+  VOID                                    *ScratchBuffer;
+  UINT32                                  ScratchBufferSize;
+  VOID                                    *DstBuffer;
+  UINT16                                  SectionAttribute;
+  UINT32                                  AuthenticationStatus;
+  EFI_FIRMWARE_VOLUME_HEADER              *InnerFvHeader;
 
   DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
 
@@ -83,6 +94,71 @@ Returns:
 
   FvIsBeingProcesssed (FwVolHeader);
 
+  //
+  // First check for encapsulated compressed firmware volumes
+  //
+  FileHeader = NULL;
+  do {
+    Status = FfsFindNextFile (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
+               FwVolHeader, &FileHeader);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+    Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
+               &SectionData, &SectionDataSize);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+    Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
+    Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
+               &ScratchBufferSize, &SectionAttribute);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+
+    //
+    // Allocate scratch buffer
+    //
+    ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (ScratchBufferSize));
+    if (ScratchBuffer == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    //
+    // Allocate destination buffer
+    //
+    DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
+    if (DstBuffer == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    //
+    // Call decompress function
+    //
+    Status = ExtractGuidedSectionDecode (Section, &DstBuffer, ScratchBuffer,
+               &AuthenticationStatus);
+    FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
+    if (EFI_ERROR (Status)) {
+      goto FreeDstBuffer;
+    }
+
+    DEBUG ((DEBUG_INFO,
+      "Processing compressed firmware volume (AuthenticationStatus == %x)\n",
+      AuthenticationStatus));
+
+    Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
+               EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
+    if (EFI_ERROR (Status)) {
+      goto FreeDstBuffer;
+    }
+
+    InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(Section + 1);
+    Status = MmCoreFfsFindMmDriver (InnerFvHeader);
+    if (EFI_ERROR (Status)) {
+      goto FreeDstBuffer;
+    }
+  } while (TRUE);
+
   for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]); Index++) {
     DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n", mMmFileTypes[Index]));
     FileType = mMmFileTypes[Index];
@@ -100,5 +176,10 @@ Returns:
     } while (!EFI_ERROR (Status));
   }
 
+  return EFI_SUCCESS;
+
+FreeDstBuffer:
+  FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
+
   return Status;
 }
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index ff2b8b9cef03..83d31e2d92c5 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -49,6 +49,7 @@ [LibraryClasses]
   BaseMemoryLib
   CacheMaintenanceLib
   DebugLib
+  ExtractGuidedSectionLib
   FvLib
   HobLib
   MemoryAllocationLib
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 5cca532456fd..67ff9112d5c0 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -205,6 +205,8 @@ GetSpmVersion (VOID)
   return Status;
 }
 
+STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
+
 /**
   The entry point of Standalone MM Foundation.
 
@@ -285,6 +287,9 @@ _ModuleEntryPoint (
     goto finish;
   }
 
+  PcdSet64 (PcdGuidedExtractHandlerTableAddress,
+    (UINT64)mExtractGuidedSectionHandlerInfo);
+
   //
   // Create Hoblist based upon boot information passed by privileged software
   //
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 769eaeeefbea..55d769fa77e4 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -54,3 +54,6 @@ [Guids]
   gEfiMmPeiMmramMemoryReserveGuid
   gEfiStandaloneMmNonSecureBufferGuid
   gEfiArmTfCpuDriverEpDescriptorGuid
+
+[PatchPcd]
+  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements
  2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
                   ` (10 preceding siblings ...)
  2019-01-16 20:22 ` [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
@ 2019-01-18  9:26 ` Achin Gupta
  2019-01-18 15:41   ` Yao, Jiewen
  11 siblings, 1 reply; 25+ messages in thread
From: Achin Gupta @ 2019-01-18  9:26 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org, Jiewen Yao
  Cc: Supreeth Venkatesh, Leif Lindholm, Jagadeesh Ujja, Thomas Abraham,
	Sami Mujawar, nd

Hi Ard,

For all the patches...

Reviewed-by: Achin Gupta <achin.gupta@arm.com>

Jiewen. There are changes to the generic Standalone MM code in this series. Do you want to have a look as well?

cheers,
Achin 
________________________________________
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: 16 January 2019 20:22
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel; Achin Gupta; Jiewen Yao; Supreeth Venkatesh; Leif Lindholm; Jagadeesh Ujja; Thomas Abraham; Sami Mujawar
Subject: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements

This series addresses a number of issues I ran into while bringing up
the standalone MM based authenticated variable store on the SynQuacer
(AArch64) platform.

Patches #1 - #3 are based on Jagadeesh's patch that imports some staging
code into StandaloneMmPkg, with the following changes:
- drop unused source files, GUID references are other unused bit,
- clean up comments referring to the MM core implementation.

Patches #4 - #9 are obvious fixes/improvements.

Patch #10 adds support for TE formatted MM_CORE_STANDALONE binaries.
This is useful given that the 4 KB section alignment we require in
AArch64 implementations of standalone MM (due to the strict separation
between code and date) results in 8 KB of wasted space at the start of
the firmware volume. This can be reduced to 4 KB when using a TE image
and the FIXED attribute in the associated [Rule] section, by leveraging
an existing optimization in the FFS generation code that aligns TE images
by reducing FFS padding rather than adding more.

Patch #11 is another space optimization: it reuses the existing support
for encapsulated compressed firmware volumes in FFS files to shrink the
size of the primary standalone MM FV considerably. Again, due to
alignment requirements, there is significant bloat in the uncompressed
images (4 KB for the PE/COFF header, and up to 4 KB per section for the
.text, .data and .reloc sections), making the absolute minimum size of
any trivial MM_STANDALONE module 16 KB.

Changes since v1:
- add patches #1 - #3
- add Supreeth's ack to patches #4 - #7

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>

Ard Biesheuvel (11):
  StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules
  StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation
  StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE
  StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib
    ref
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII
    strings
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
    ASSERT_EFI_ERROR()s
  StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime
    attribute
  StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE
    images
  StandaloneMmPkg/Core: permit encapsulated firmware volumes

 StandaloneMmPkg/Core/Dispatcher.c             |  30 +-
 StandaloneMmPkg/Core/FwVol.c                  |  99 ++-
 StandaloneMmPkg/Core/StandaloneMmCore.inf     |   1 +
 .../StandaloneMmCpu/AArch64/EventHandle.c     |   2 +-
 .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c |   6 +-
 .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h |   8 +-
 .../AArch64/StandaloneMmCpu.inf               |   4 +-
 .../AArch64/SetPermissions.c                  | 109 +--
 .../AArch64/StandaloneMmCoreEntryPoint.c      |   7 +-
 .../StandaloneMmCoreEntryPoint.inf            |   4 +
 .../StandaloneMmCoreHobLib.inf                |   2 +-
 .../StandaloneMmHobLib/StandaloneMmHobLib.c   | 649 ++++++++++++++
 .../StandaloneMmHobLib/StandaloneMmHobLib.inf |  45 +
 .../StandaloneMmMemoryAllocationLib.c         | 822 ++++++++++++++++++
 .../StandaloneMmMemoryAllocationLib.inf       |  42 +
 .../StandaloneMmPeCoffExtraActionLib.c        |   9 +-
 16 files changed, 1716 insertions(+), 123 deletions(-)
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

--
2.17.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation
  2019-01-16 20:22 ` [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation Ard Biesheuvel
@ 2019-01-18 15:23   ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:23 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE
> MemoryAllocationLib implementation
> 
> This MemoryAllocationLib code is based on the staging implementation of
> StandaloneMmPkg, with the following changes:
> - use correct MODULE_TYPE
> - include MmServicesTableLib instead of declaring gMmst directly
> - update code comments referring to the MM core
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.c   | 822 ++++++++++++++++++++
> 
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.inf |  42 +
>  2 files changed, 864 insertions(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.c
> b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.c
> new file mode 100644
> index 000000000000..c7c1282babff
> --- /dev/null
> +++
> b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.c
> @@ -0,0 +1,822 @@
> +/** @file
> +  Support routines for memory allocation routines based on Standalone
> MM Core internal functions.
> +
> +  Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2018, ARM Limited. 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 <PiMm.h>
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/MmServicesTableLib.h>
> +
> +/**
> +  Allocates one or more 4KB pages of a certain memory type.
> +
> +  Allocates the number of 4KB pages of a certain memory type and returns
> a pointer to the allocated
> +  buffer.  The buffer returned is aligned on a 4KB boundary.  If Pages is 0,
> then NULL is returned.
> +  If there is not enough memory remaining to satisfy the request, then
> NULL is returned.
> +
> +  @param  MemoryType            The type of memory to allocate.
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalAllocatePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages
> +  )
> +{
> +  EFI_STATUS            Status;
> +  EFI_PHYSICAL_ADDRESS  Memory;
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  Status = gMmst->MmAllocatePages (AllocateAnyPages, MemoryType,
> Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +    return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiBootServicesData.
> +
> +  Allocates the number of 4KB pages of type EfiBootServicesData and
> returns a pointer to the
> +  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If
> Pages is 0, then NULL
> +  is returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is
> +  returned.
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePages (
> +  IN UINTN  Pages
> +  )
> +{
> +  return InternalAllocatePages (EfiRuntimeServicesData, Pages);
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiRuntimeServicesData.
> +
> +  Allocates the number of 4KB pages of type EfiRuntimeServicesData and
> returns a pointer to the
> +  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If
> Pages is 0, then NULL
> +  is returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is
> +  returned.
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateRuntimePages (
> +  IN UINTN  Pages
> +  )
> +{
> +  return InternalAllocatePages (EfiRuntimeServicesData, Pages);
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiReservedMemoryType.
> +
> +  Allocates the number of 4KB pages of type EfiReservedMemoryType and
> returns a pointer to the
> +  allocated buffer.  The buffer returned is aligned on a 4KB boundary.  If
> Pages is 0, then NULL
> +  is returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is
> +  returned.
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateReservedPages (
> +  IN UINTN  Pages
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Frees one or more 4KB pages that were previously allocated with one of
> the page allocation
> +  functions in the Memory Allocation Library.
> +
> +  Frees the number of 4KB pages specified by Pages from the buffer
> specified by Buffer.  Buffer
> +  must have been allocated on a previous call to the page allocation
> services of the Memory
> +  Allocation Library.  If it is not possible to free allocated pages, then this
> function will
> +  perform no actions.
> +
> +  If Buffer was not allocated with a page allocation function in the
> Memory Allocation Library,
> +  then ASSERT().
> +  If Pages is zero, then ASSERT().
> +
> +  @param  Buffer                Pointer to the buffer of pages to
> free.
> +  @param  Pages                 The number of 4 KB pages to free.
> +
> +**/
> +VOID
> +EFIAPI
> +FreePages (
> +  IN VOID   *Buffer,
> +  IN UINTN  Pages
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  ASSERT (Pages != 0);
> +  Status = gMmst->MmFreePages ((EFI_PHYSICAL_ADDRESS)
> (UINTN)Buffer, Pages);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of a certain memory type at a specified
> alignment.
> +
> +  Allocates the number of 4KB pages specified by Pages of a certain
> memory type with an alignment
> +  specified by Alignment.  The allocated buffer is returned.  If Pages is 0,
> then NULL is returned.
> +  If there is not enough memory at the specified alignment remaining to
> satisfy the request, then
> +  NULL is returned.
> +  If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> +  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> +  @param  MemoryType            The type of memory to allocate.
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +  @param  Alignment             The requested alignment of the
> allocation.  Must be a power of two.
> +                                If Alignment is zero, then byte
> alignment is used.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalAllocateAlignedPages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages,
> +  IN UINTN            Alignment
> +  )
> +{
> +  EFI_STATUS            Status;
> +  EFI_PHYSICAL_ADDRESS  Memory;
> +  UINTN                 AlignedMemory;
> +  UINTN                 AlignmentMask;
> +  UINTN                 UnalignedPages;
> +  UINTN                 RealPages;
> +
> +  //
> +  // Alignment must be a power of two or zero.
> +  //
> +  ASSERT ((Alignment & (Alignment - 1)) == 0);
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +  if (Alignment > EFI_PAGE_SIZE) {
> +    //
> +    // Calculate the total number of pages since alignment is larger than
> page size.
> +    //
> +    AlignmentMask  = Alignment - 1;
> +    RealPages      = Pages + EFI_SIZE_TO_PAGES (Alignment);
> +    //
> +    // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> overflow.
> +    //
> +    ASSERT (RealPages > Pages);
> +
> +    Status         = gMmst->MmAllocatePages (AllocateAnyPages,
> MemoryType, RealPages, &Memory);
> +    if (EFI_ERROR (Status)) {
> +      return NULL;
> +    }
> +    AlignedMemory  = ((UINTN)Memory + AlignmentMask) &
> ~AlignmentMask;
> +    UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN)
> Memory);
> +    if (UnalignedPages > 0) {
> +      //
> +      // Free first unaligned page(s).
> +      //
> +      Status = gMmst->MmFreePages (Memory, UnalignedPages);
> +      ASSERT_EFI_ERROR (Status);
> +    }
> +    Memory         = (EFI_PHYSICAL_ADDRESS)(AlignedMemory +
> EFI_PAGES_TO_SIZE (Pages));
> +    UnalignedPages = RealPages - Pages - UnalignedPages;
> +    if (UnalignedPages > 0) {
> +      //
> +      // Free last unaligned page(s).
> +      //
> +      Status = gMmst->MmFreePages (Memory, UnalignedPages);
> +      ASSERT_EFI_ERROR (Status);
> +    }
> +  } else {
> +    //
> +    // Do not over-allocate pages in this case.
> +    //
> +    Status = gMmst->MmAllocatePages (AllocateAnyPages, MemoryType,
> Pages, &Memory);
> +    if (EFI_ERROR (Status)) {
> +      return NULL;
> +    }
> +    AlignedMemory  = (UINTN) Memory;
> +  }
> +  return (VOID *) AlignedMemory;
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiBootServicesData at a
> specified alignment.
> +
> +  Allocates the number of 4KB pages specified by Pages of type
> EfiBootServicesData with an
> +  alignment specified by Alignment.  The allocated buffer is returned.  If
> Pages is 0, then NULL is
> +  returned.  If there is not enough memory at the specified alignment
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> +  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +  @param  Alignment             The requested alignment of the
> allocation.  Must be a power of two.
> +                                If Alignment is zero, then byte
> alignment is used.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateAlignedPages (
> +  IN UINTN  Pages,
> +  IN UINTN  Alignment
> +  )
> +{
> +  return InternalAllocateAlignedPages (EfiRuntimeServicesData, Pages,
> Alignment);
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiRuntimeServicesData at a
> specified alignment.
> +
> +  Allocates the number of 4KB pages specified by Pages of type
> EfiRuntimeServicesData with an
> +  alignment specified by Alignment.  The allocated buffer is returned.  If
> Pages is 0, then NULL is
> +  returned.  If there is not enough memory at the specified alignment
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> +  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +  @param  Alignment             The requested alignment of the
> allocation.  Must be a power of two.
> +                                If Alignment is zero, then byte
> alignment is used.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateAlignedRuntimePages (
> +  IN UINTN  Pages,
> +  IN UINTN  Alignment
> +  )
> +{
> +  return InternalAllocateAlignedPages (EfiRuntimeServicesData, Pages,
> Alignment);
> +}
> +
> +/**
> +  Allocates one or more 4KB pages of type EfiReservedMemoryType at a
> specified alignment.
> +
> +  Allocates the number of 4KB pages specified by Pages of type
> EfiReservedMemoryType with an
> +  alignment specified by Alignment.  The allocated buffer is returned.  If
> Pages is 0, then NULL is
> +  returned.  If there is not enough memory at the specified alignment
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> +  If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> +  @param  Pages                 The number of 4 KB pages to
> allocate.
> +  @param  Alignment             The requested alignment of the
> allocation.  Must be a power of two.
> +                                If Alignment is zero, then byte
> alignment is used.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateAlignedReservedPages (
> +  IN UINTN  Pages,
> +  IN UINTN  Alignment
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Frees one or more 4KB pages that were previously allocated with one of
> the aligned page
> +  allocation functions in the Memory Allocation Library.
> +
> +  Frees the number of 4KB pages specified by Pages from the buffer
> specified by Buffer.  Buffer
> +  must have been allocated on a previous call to the aligned page
> allocation services of the Memory
> +  Allocation Library.  If it is not possible to free allocated pages, then this
> function will
> +  perform no actions.
> +
> +  If Buffer was not allocated with an aligned page allocation function in
> the Memory Allocation
> +  Library, then ASSERT().
> +  If Pages is zero, then ASSERT().
> +
> +  @param  Buffer                Pointer to the buffer of pages to
> free.
> +  @param  Pages                 The number of 4 KB pages to free.
> +
> +**/
> +VOID
> +EFIAPI
> +FreeAlignedPages (
> +  IN VOID   *Buffer,
> +  IN UINTN  Pages
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  ASSERT (Pages != 0);
> +  Status = gMmst->MmFreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
> Pages);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +/**
> +  Allocates a buffer of a certain pool type.
> +
> +  Allocates the number bytes specified by AllocationSize of a certain pool
> type and returns a
> +  pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer
> of 0 size is
> +  returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is returned.
> +
> +  @param  MemoryType            The type of memory to allocate.
> +  @param  AllocationSize        The number of bytes to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalAllocatePool (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            AllocationSize
> +  )
> +{
> +  EFI_STATUS  Status;
> +  VOID        *Memory;
> +
> +  Memory = NULL;
> +
> +  Status = gMmst->MmAllocatePool (MemoryType, AllocationSize,
> &Memory);
> +  if (EFI_ERROR (Status)) {
> +    Memory = NULL;
> +  }
> +  return Memory;
> +}
> +
> +/**
> +  Allocates a buffer of type EfiBootServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiBootServicesData and returns a
> +  pointer to the allocated buffer.  If AllocationSize is 0, then a valid buffer
> of 0 size is
> +  returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
> +}
> +
> +/**
> +  Allocates a buffer of type EfiRuntimeServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiRuntimeServicesData and returns
> +  a pointer to the allocated buffer.  If AllocationSize is 0, then a valid
> buffer of 0 size is
> +  returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateRuntimePool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
> +}
> +
> +/**
> +  Allocates a buffer of type EfiReservedMemoryType.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiReservedMemoryType and returns
> +  a pointer to the allocated buffer.  If AllocationSize is 0, then a valid
> buffer of 0 size is
> +  returned.  If there is not enough memory remaining to satisfy the
> request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateReservedPool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Allocates and zeros a buffer of a certain pool type.
> +
> +  Allocates the number bytes specified by AllocationSize of a certain pool
> type, clears the buffer
> +  with zeros, and returns a pointer to the allocated buffer.  If
> AllocationSize is 0, then a valid
> +  buffer of 0 size is returned.  If there is not enough memory remaining to
> satisfy the request,
> +  then NULL is returned.
> +
> +  @param  PoolType              The type of memory to allocate.
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalAllocateZeroPool (
> +  IN EFI_MEMORY_TYPE  PoolType,
> +  IN UINTN            AllocationSize
> +  )
> +{
> +  VOID  *Memory;
> +
> +  Memory = InternalAllocatePool (PoolType, AllocationSize);
> +  if (Memory != NULL) {
> +    Memory = ZeroMem (Memory, AllocationSize);
> +  }
> +  return Memory;
> +}
> +
> +/**
> +  Allocates and zeros a buffer of type EfiBootServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiBootServicesData, clears the
> +  buffer with zeros, and returns a pointer to the allocated buffer.  If
> AllocationSize is 0, then a
> +  valid buffer of 0 size is returned.  If there is not enough memory
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateZeroPool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return InternalAllocateZeroPool (EfiRuntimeServicesData,
> AllocationSize);
> +}
> +
> +/**
> +  Allocates and zeros a buffer of type EfiRuntimeServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiRuntimeServicesData, clears the
> +  buffer with zeros, and returns a pointer to the allocated buffer.  If
> AllocationSize is 0, then a
> +  valid buffer of 0 size is returned.  If there is not enough memory
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateRuntimeZeroPool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return InternalAllocateZeroPool (EfiRuntimeServicesData,
> AllocationSize);
> +}
> +
> +/**
> +  Allocates and zeros a buffer of type EfiReservedMemoryType.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiReservedMemoryType, clears the
> +  buffer with zeros, and returns a pointer to the allocated buffer.  If
> AllocationSize is 0, then a
> +  valid buffer of 0 size is returned.  If there is not enough memory
> remaining to satisfy the
> +  request, then NULL is returned.
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateReservedZeroPool (
> +  IN UINTN  AllocationSize
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Copies a buffer to an allocated buffer of a certain pool type.
> +
> +  Allocates the number bytes specified by AllocationSize of a certain pool
> type, copies
> +  AllocationSize bytes from Buffer to the newly allocated buffer, and
> returns a pointer to the
> +  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
> returned.  If there
> +  is not enough memory remaining to satisfy the request, then NULL is
> returned.
> +  If Buffer is NULL, then ASSERT().
> +  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then
> ASSERT().
> +
> +  @param  PoolType              The type of pool to allocate.
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +  @param  Buffer                The buffer to copy to the allocated
> buffer.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalAllocateCopyPool (
> +  IN EFI_MEMORY_TYPE  PoolType,
> +  IN UINTN            AllocationSize,
> +  IN CONST VOID       *Buffer
> +  )
> +{
> +  VOID  *Memory;
> +
> +  ASSERT (Buffer != NULL);
> +  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
> +
> +  Memory = InternalAllocatePool (PoolType, AllocationSize);
> +  if (Memory != NULL) {
> +     Memory = CopyMem (Memory, Buffer, AllocationSize);
> +  }
> +  return Memory;
> +}
> +
> +/**
> +  Copies a buffer to an allocated buffer of type EfiBootServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiBootServicesData, copies
> +  AllocationSize bytes from Buffer to the newly allocated buffer, and
> returns a pointer to the
> +  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
> returned.  If there
> +  is not enough memory remaining to satisfy the request, then NULL is
> returned.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then
> ASSERT().
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +  @param  Buffer                The buffer to copy to the allocated
> buffer.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateCopyPool (
> +  IN UINTN       AllocationSize,
> +  IN CONST VOID  *Buffer
> +  )
> +{
> +  return InternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize,
> Buffer);
> +}
> +
> +/**
> +  Copies a buffer to an allocated buffer of type EfiRuntimeServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiRuntimeServicesData, copies
> +  AllocationSize bytes from Buffer to the newly allocated buffer, and
> returns a pointer to the
> +  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
> returned.  If there
> +  is not enough memory remaining to satisfy the request, then NULL is
> returned.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then
> ASSERT().
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +  @param  Buffer                The buffer to copy to the allocated
> buffer.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateRuntimeCopyPool (
> +  IN UINTN       AllocationSize,
> +  IN CONST VOID  *Buffer
> +  )
> +{
> +  return InternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize,
> Buffer);
> +}
> +
> +/**
> +  Copies a buffer to an allocated buffer of type EfiReservedMemoryType.
> +
> +  Allocates the number bytes specified by AllocationSize of type
> EfiReservedMemoryType, copies
> +  AllocationSize bytes from Buffer to the newly allocated buffer, and
> returns a pointer to the
> +  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is
> returned.  If there
> +  is not enough memory remaining to satisfy the request, then NULL is
> returned.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then
> ASSERT().
> +
> +  @param  AllocationSize        The number of bytes to allocate and
> zero.
> +  @param  Buffer                The buffer to copy to the allocated
> buffer.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateReservedCopyPool (
> +  IN UINTN       AllocationSize,
> +  IN CONST VOID  *Buffer
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Reallocates a buffer of a specified memory type.
> +
> +  Allocates and zeros the number bytes specified by NewSize from memory
> of the type
> +  specified by PoolType.  If OldBuffer is not NULL, then the smaller of
> OldSize and
> +  NewSize bytes are copied from OldBuffer to the newly allocated buffer,
> and
> +  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
> +  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
> +  enough memory remaining to satisfy the request, then NULL is returned.
> +
> +  If the allocation of the new buffer is successful and the smaller of
> NewSize and OldSize
> +  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
> +
> +  @param  PoolType       The type of pool to allocate.
> +  @param  OldSize        The size, in bytes, of OldBuffer.
> +  @param  NewSize        The size, in bytes, of the buffer to
> reallocate.
> +  @param  OldBuffer      The buffer to copy to the allocated buffer.
> This is an optional
> +                         parameter that may be NULL.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +InternalReallocatePool (
> +  IN EFI_MEMORY_TYPE  PoolType,
> +  IN UINTN            OldSize,
> +  IN UINTN            NewSize,
> +  IN VOID             *OldBuffer  OPTIONAL
> +  )
> +{
> +  VOID  *NewBuffer;
> +
> +  NewBuffer = InternalAllocateZeroPool (PoolType, NewSize);
> +  if (NewBuffer != NULL && OldBuffer != NULL) {
> +    CopyMem (NewBuffer, OldBuffer, MIN (OldSize, NewSize));
> +    FreePool (OldBuffer);
> +  }
> +  return NewBuffer;
> +}
> +
> +/**
> +  Reallocates a buffer of type EfiBootServicesData.
> +
> +  Allocates and zeros the number bytes specified by NewSize from memory
> of type
> +  EfiBootServicesData.  If OldBuffer is not NULL, then the smaller of
> OldSize and
> +  NewSize bytes are copied from OldBuffer to the newly allocated buffer,
> and
> +  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
> +  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
> +  enough memory remaining to satisfy the request, then NULL is returned.
> +
> +  If the allocation of the new buffer is successful and the smaller of
> NewSize and OldSize
> +  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
> +
> +  @param  OldSize        The size, in bytes, of OldBuffer.
> +  @param  NewSize        The size, in bytes, of the buffer to
> reallocate.
> +  @param  OldBuffer      The buffer to copy to the allocated buffer.
> This is an optional
> +                         parameter that may be NULL.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +ReallocatePool (
> +  IN UINTN  OldSize,
> +  IN UINTN  NewSize,
> +  IN VOID   *OldBuffer  OPTIONAL
> +  )
> +{
> +  return InternalReallocatePool (EfiRuntimeServicesData, OldSize, NewSize,
> OldBuffer);
> +}
> +
> +/**
> +  Reallocates a buffer of type EfiRuntimeServicesData.
> +
> +  Allocates and zeros the number bytes specified by NewSize from memory
> of type
> +  EfiRuntimeServicesData.  If OldBuffer is not NULL, then the smaller of
> OldSize and
> +  NewSize bytes are copied from OldBuffer to the newly allocated buffer,
> and
> +  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
> +  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
> +  enough memory remaining to satisfy the request, then NULL is returned.
> +
> +  If the allocation of the new buffer is successful and the smaller of
> NewSize and OldSize
> +  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
> +
> +  @param  OldSize        The size, in bytes, of OldBuffer.
> +  @param  NewSize        The size, in bytes, of the buffer to
> reallocate.
> +  @param  OldBuffer      The buffer to copy to the allocated buffer.
> This is an optional
> +                         parameter that may be NULL.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +ReallocateRuntimePool (
> +  IN UINTN  OldSize,
> +  IN UINTN  NewSize,
> +  IN VOID   *OldBuffer  OPTIONAL
> +  )
> +{
> +  return InternalReallocatePool (EfiRuntimeServicesData, OldSize, NewSize,
> OldBuffer);
> +}
> +
> +/**
> +  Reallocates a buffer of type EfiReservedMemoryType.
> +
> +  Allocates and zeros the number bytes specified by NewSize from memory
> of type
> +  EfiReservedMemoryType.  If OldBuffer is not NULL, then the smaller of
> OldSize and
> +  NewSize bytes are copied from OldBuffer to the newly allocated buffer,
> and
> +  OldBuffer is freed.  A pointer to the newly allocated buffer is returned.
> +  If NewSize is 0, then a valid buffer of 0 size is  returned.  If there is not
> +  enough memory remaining to satisfy the request, then NULL is returned.
> +
> +  If the allocation of the new buffer is successful and the smaller of
> NewSize and OldSize
> +  is greater than (MAX_ADDRESS - OldBuffer + 1), then ASSERT().
> +
> +  @param  OldSize        The size, in bytes, of OldBuffer.
> +  @param  NewSize        The size, in bytes, of the buffer to
> reallocate.
> +  @param  OldBuffer      The buffer to copy to the allocated buffer.
> This is an optional
> +                         parameter that may be NULL.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +ReallocateReservedPool (
> +  IN UINTN  OldSize,
> +  IN UINTN  NewSize,
> +  IN VOID   *OldBuffer  OPTIONAL
> +  )
> +{
> +  return NULL;
> +}
> +
> +/**
> +  Frees a buffer that was previously allocated with one of the pool
> allocation functions in the
> +  Memory Allocation Library.
> +
> +  Frees the buffer specified by Buffer.  Buffer must have been allocated
> on a previous call to the
> +  pool allocation services of the Memory Allocation Library.  If it is not
> possible to free pool
> +  resources, then this function will perform no actions.
> +
> +  If Buffer was not allocated with a pool allocation function in the Memory
> Allocation Library,
> +  then ASSERT().
> +
> +  @param  Buffer                Pointer to the buffer to free.
> +
> +**/
> +VOID
> +EFIAPI
> +FreePool (
> +  IN VOID   *Buffer
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  Status = gMmst->MmFreePool (Buffer);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.inf
> b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.inf
> new file mode 100644
> index 000000000000..8776e34e7c06
> --- /dev/null
> +++
> b/StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo
> neMmMemoryAllocationLib.inf
> @@ -0,0 +1,42 @@
> +## @file
> +# Memory Allocation Library instance standalone MM modules.
> +#
> +# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +# Copyright (c) 2018, 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                      = MemoryAllocationLib
> +  FILE_GUID                      =
> 54646378-A9DC-473F-9BE1-BD027C4C76DE
> +  MODULE_TYPE                    = MM_STANDALONE
> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032
> +  LIBRARY_CLASS                  =
> MemoryAllocationLib|MM_STANDALONE
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> +#
> +
> +[Sources]
> +  StandaloneMmMemoryAllocationLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  StandaloneMmPkg/StandaloneMmPkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  MmServicesTableLib
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules
  2019-01-16 20:22 ` [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules Ard Biesheuvel
@ 2019-01-18 15:24   ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:24 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation
> for MM_STANDALONE modules
> 
> This HobLib code is based on the staging implementation of
> StandaloneMmPkg, with the following changes:
> - drop the unused AArch64/StandaloneMmCoreHobLibInternal.c source file
> - remove hack from HobLibConstructor()
> - update code comments referring the MM core
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> | 649 ++++++++++++++++++++
> 
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> |  45 ++
>  2 files changed, 694 insertions(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> new file mode 100644
> index 000000000000..cc1a08166470
> --- /dev/null
> +++
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> @@ -0,0 +1,649 @@
> +/** @file
> +  HOB Library implementation for Standalone MM Core.
> +
> +Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.<BR>
> +Copyright (c) 2018, 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 <PiMm.h>
> +
> +#include <Library/HobLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MmServicesTableLib.h>
> +
> +//
> +// Cache copy of HobList pointer.
> +//
> +STATIC VOID *gHobList = NULL;
> +
> +/**
> +  The constructor function caches the pointer to HOB list.
> +
> +  The constructor function gets the start address of HOB list from system
> configuration table.
> +  It will ASSERT() if that operation fails and it will always return
> EFI_SUCCESS.
> +
> +  @param  ImageHandle     The firmware allocated handle for the
> image.
> +  @param  MmSystemTable   A pointer to the MM System Table.
> +
> +  @retval EFI_SUCCESS     The constructor successfully gets HobList.
> +  @retval Other value     The constructor can't get HobList.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HobLibConstructor (
> +  IN EFI_HANDLE             ImageHandle,
> +  IN EFI_MM_SYSTEM_TABLE    *MmSystemTable
> +  )
> +{
> +  UINTN       Index;
> +
> +  for (Index = 0; Index < gMmst->NumberOfTableEntries; Index++) {
> +    if (CompareGuid (&gEfiHobListGuid,
> &gMmst->MmConfigurationTable[Index].VendorGuid)) {
> +      gHobList = gMmst->MmConfigurationTable[Index].VendorTable;
> +      break;
> +    }
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Returns the pointer to the HOB list.
> +
> +  This function returns the pointer to first HOB in the list.
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @return The pointer to the HOB list.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetHobList (
> +  VOID
> +  )
> +{
> +  UINTN       Index;
> +
> +  if (gHobList == NULL) {
> +    for (Index = 0; Index < gMmst->NumberOfTableEntries; Index++) {
> +      if (CompareGuid (&gEfiHobListGuid,
> &gMmst->MmConfigurationTable[Index].VendorGuid)) {
> +        gHobList = gMmst->MmConfigurationTable[Index].VendorTable;
> +        break;
> +      }
> +    }
> +  }
> +  ASSERT (gHobList != NULL);
> +  return gHobList;
> +}
> +
> +/**
> +  Returns the next instance of a HOB type from the starting HOB.
> +
> +  This function searches the first instance of a HOB type from the starting
> HOB pointer.
> +  If there does not exist such HOB type from the starting HOB pointer, it
> will return NULL.
> +  In contrast with macro GET_NEXT_HOB(), this function does not skip the
> starting HOB pointer
> +  unconditionally: it returns HobStart back if HobStart itself meets the
> requirement;
> +  caller is required to use GET_NEXT_HOB() if it wishes to skip current
> HobStart.
> +
> +  If HobStart is NULL, then ASSERT().
> +
> +  @param  Type          The HOB type to return.
> +  @param  HobStart      The starting HOB pointer to search from.
> +
> +  @return The next instance of a HOB type from the starting HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetNextHob (
> +  IN UINT16                 Type,
> +  IN CONST VOID             *HobStart
> +  )
> +{
> +  EFI_PEI_HOB_POINTERS  Hob;
> +
> +  ASSERT (HobStart != NULL);
> +
> +  Hob.Raw = (UINT8 *) HobStart;
> +  //
> +  // Parse the HOB list until end of list or matching type is found.
> +  //
> +  while (!END_OF_HOB_LIST (Hob)) {
> +    if (Hob.Header->HobType == Type) {
> +      return Hob.Raw;
> +    }
> +    Hob.Raw = GET_NEXT_HOB (Hob);
> +  }
> +  return NULL;
> +}
> +
> +/**
> +  Returns the first instance of a HOB type among the whole HOB list.
> +
> +  This function searches the first instance of a HOB type among the whole
> HOB list.
> +  If there does not exist such HOB type in the HOB list, it will return NULL.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @param  Type          The HOB type to return.
> +
> +  @return The next instance of a HOB type from the starting HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetFirstHob (
> +  IN UINT16                 Type
> +  )
> +{
> +  VOID      *HobList;
> +
> +  HobList = GetHobList ();
> +  return GetNextHob (Type, HobList);
> +}
> +
> +/**
> +  Returns the next instance of the matched GUID HOB from the starting
> HOB.
> +
> +  This function searches the first instance of a HOB from the starting HOB
> pointer.
> +  Such HOB should satisfy two conditions:
> +  its HOB type is EFI_HOB_TYPE_GUID_EXTENSION, and its GUID Name
> equals to the input Guid.
> +  If such a HOB from the starting HOB pointer does not exist, it will return
> NULL.
> +  Caller is required to apply GET_GUID_HOB_DATA () and
> GET_GUID_HOB_DATA_SIZE ()
> +  to extract the data section and its size information, respectively.
> +  In contrast with macro GET_NEXT_HOB(), this function does not skip the
> starting HOB pointer
> +  unconditionally: it returns HobStart back if HobStart itself meets the
> requirement;
> +  caller is required to use GET_NEXT_HOB() if it wishes to skip current
> HobStart.
> +
> +  If Guid is NULL, then ASSERT().
> +  If HobStart is NULL, then ASSERT().
> +
> +  @param  Guid          The GUID to match with in the HOB list.
> +  @param  HobStart      A pointer to a Guid.
> +
> +  @return The next instance of the matched GUID HOB from the starting
> HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetNextGuidHob (
> +  IN CONST EFI_GUID         *Guid,
> +  IN CONST VOID             *HobStart
> +  )
> +{
> +  EFI_PEI_HOB_POINTERS  GuidHob;
> +
> +  GuidHob.Raw = (UINT8 *) HobStart;
> +  while ((GuidHob.Raw = GetNextHob (EFI_HOB_TYPE_GUID_EXTENSION,
> GuidHob.Raw)) != NULL) {
> +    if (CompareGuid (Guid, &GuidHob.Guid->Name)) {
> +      break;
> +    }
> +    GuidHob.Raw = GET_NEXT_HOB (GuidHob);
> +  }
> +  return GuidHob.Raw;
> +}
> +
> +/**
> +  Returns the first instance of the matched GUID HOB among the whole
> HOB list.
> +
> +  This function searches the first instance of a HOB among the whole HOB
> list.
> +  Such HOB should satisfy two conditions:
> +  its HOB type is EFI_HOB_TYPE_GUID_EXTENSION and its GUID Name
> equals to the input Guid.
> +  If such a HOB from the starting HOB pointer does not exist, it will return
> NULL.
> +  Caller is required to apply GET_GUID_HOB_DATA () and
> GET_GUID_HOB_DATA_SIZE ()
> +  to extract the data section and its size information, respectively.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +  If Guid is NULL, then ASSERT().
> +
> +  @param  Guid          The GUID to match with in the HOB list.
> +
> +  @return The first instance of the matched GUID HOB among the whole
> HOB list.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetFirstGuidHob (
> +  IN CONST EFI_GUID         *Guid
> +  )
> +{
> +  VOID      *HobList;
> +
> +  HobList = GetHobList ();
> +  return GetNextGuidHob (Guid, HobList);
> +}
> +
> +/**
> +  Get the system boot mode from the HOB list.
> +
> +  This function returns the system boot mode information from the
> +  PHIT HOB in HOB list.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @param  VOID
> +
> +  @return The Boot Mode.
> +
> +**/
> +EFI_BOOT_MODE
> +EFIAPI
> +GetBootModeHob (
> +  VOID
> +  )
> +{
> +  EFI_HOB_HANDOFF_INFO_TABLE    *HandOffHob;
> +
> +  HandOffHob = (EFI_HOB_HANDOFF_INFO_TABLE *) GetHobList ();
> +
> +  return  HandOffHob->BootMode;
> +}
> +
> +VOID *
> +CreateHob (
> +  IN  UINT16    HobType,
> +  IN  UINT16    HobLength
> +  )
> +{
> +  EFI_HOB_HANDOFF_INFO_TABLE  *HandOffHob;
> +  EFI_HOB_GENERIC_HEADER      *HobEnd;
> +  EFI_PHYSICAL_ADDRESS        FreeMemory;
> +  VOID                        *Hob;
> +
> +  HandOffHob = GetHobList ();
> +
> +  HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> +
> +  FreeMemory = HandOffHob->EfiFreeMemoryTop -
> HandOffHob->EfiFreeMemoryBottom;
> +
> +  if (FreeMemory < HobLength) {
> +      return NULL;
> +  }
> +
> +  Hob = (VOID*) (UINTN) HandOffHob->EfiEndOfHobList;
> +  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobType = HobType;
> +  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobLength = HobLength;
> +  ((EFI_HOB_GENERIC_HEADER*) Hob)->Reserved = 0;
> +
> +  HobEnd = (EFI_HOB_GENERIC_HEADER*) ((UINTN)Hob + HobLength);
> +  HandOffHob->EfiEndOfHobList = (EFI_PHYSICAL_ADDRESS) (UINTN)
> HobEnd;
> +
> +  HobEnd->HobType   = EFI_HOB_TYPE_END_OF_HOB_LIST;
> +  HobEnd->HobLength = sizeof(EFI_HOB_GENERIC_HEADER);
> +  HobEnd->Reserved  = 0;
> +  HobEnd++;
> +  HandOffHob->EfiFreeMemoryBottom = (EFI_PHYSICAL_ADDRESS)
> (UINTN) HobEnd;
> +
> +  return Hob;
> +}
> +
> +/**
> +  Builds a HOB for a loaded PE32 module.
> +
> +  This function builds a HOB for a loaded PE32 module.
> +  If ModuleName is NULL, then ASSERT().
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  ModuleName              The GUID File Name of the
> module.
> +  @param  MemoryAllocationModule  The 64 bit physical address of the
> module.
> +  @param  ModuleLength            The length of the module in
> bytes.
> +  @param  EntryPoint              The 64 bit physical address of the
> module entry point.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildModuleHob (
> +  IN CONST EFI_GUID         *ModuleName,
> +  IN EFI_PHYSICAL_ADDRESS   MemoryAllocationModule,
> +  IN UINT64                 ModuleLength,
> +  IN EFI_PHYSICAL_ADDRESS   EntryPoint
> +  )
> +{
> +  EFI_HOB_MEMORY_ALLOCATION_MODULE  *Hob;
> +
> +  ASSERT (((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
> +          ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0));
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION_MODULE));
> +
> +  CopyGuid (&(Hob->MemoryAllocationHeader.Name),
> &gEfiHobMemoryAllocModuleGuid);
> +  Hob->MemoryAllocationHeader.MemoryBaseAddress =
> MemoryAllocationModule;
> +  Hob->MemoryAllocationHeader.MemoryLength      = ModuleLength;
> +  Hob->MemoryAllocationHeader.MemoryType        =
> EfiBootServicesCode;
> +
> +  //
> +  // Zero the reserved space to match HOB spec
> +  //
> +  ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof
> (Hob->MemoryAllocationHeader.Reserved));
> +
> +  CopyGuid (&Hob->ModuleName, ModuleName);
> +  Hob->EntryPoint = EntryPoint;
> +}
> +
> +/**
> +  Builds a HOB that describes a chunk of system memory.
> +
> +  This function builds a HOB that describes a chunk of system memory.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  ResourceType        The type of resource described by this
> HOB.
> +  @param  ResourceAttribute   The resource attributes of the memory
> described by this HOB.
> +  @param  PhysicalStart       The 64 bit physical address of memory
> described by this HOB.
> +  @param  NumberOfBytes       The length of the memory described
> by this HOB in bytes.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildResourceDescriptorHob (
> +  IN EFI_RESOURCE_TYPE            ResourceType,
> +  IN EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttribute,
> +  IN EFI_PHYSICAL_ADDRESS         PhysicalStart,
> +  IN UINT64                       NumberOfBytes
> +  )
> +{
> +  EFI_HOB_RESOURCE_DESCRIPTOR  *Hob;
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof
> (EFI_HOB_RESOURCE_DESCRIPTOR));
> +  ASSERT(Hob != NULL);
> +
> +  Hob->ResourceType      = ResourceType;
> +  Hob->ResourceAttribute = ResourceAttribute;
> +  Hob->PhysicalStart     = PhysicalStart;
> +  Hob->ResourceLength    = NumberOfBytes;
> +}
> +
> +/**
> +  Builds a GUID HOB with a certain data length.
> +
> +  This function builds a customized HOB tagged with a GUID for
> identification
> +  and returns the start address of GUID HOB data so that caller can fill the
> customized data.
> +  The HOB Header and Name field is already stripped.
> +  If Guid is NULL, then ASSERT().
> +  If there is no additional space for HOB creation, then ASSERT().
> +  If DataLength >= (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then
> ASSERT().
> +
> +  @param  Guid          The GUID to tag the customized HOB.
> +  @param  DataLength    The size of the data payload for the GUID
> HOB.
> +
> +  @return The start address of GUID HOB data.
> +
> +**/
> +VOID *
> +EFIAPI
> +BuildGuidHob (
> +  IN CONST EFI_GUID              *Guid,
> +  IN UINTN                       DataLength
> +  )
> +{
> +  EFI_HOB_GUID_TYPE *Hob;
> +
> +  //
> +  // Make sure that data length is not too long.
> +  //
> +  ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16) (sizeof
> (EFI_HOB_GUID_TYPE) + DataLength));
> +  CopyGuid (&Hob->Name, Guid);
> +  return Hob + 1;
> +}
> +
> +
> +/**
> +  Copies a data buffer to a newly-built HOB.
> +
> +  This function builds a customized HOB tagged with a GUID for
> identification,
> +  copies the input data to the HOB data field and returns the start address
> of the GUID HOB data.
> +  The HOB Header and Name field is already stripped.
> +  If Guid is NULL, then ASSERT().
> +  If Data is NULL and DataLength > 0, then ASSERT().
> +  If there is no additional space for HOB creation, then ASSERT().
> +  If DataLength >= (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then
> ASSERT().
> +
> +  @param  Guid          The GUID to tag the customized HOB.
> +  @param  Data          The data to be copied into the data field of
> the GUID HOB.
> +  @param  DataLength    The size of the data payload for the GUID
> HOB.
> +
> +  @return The start address of GUID HOB data.
> +
> +**/
> +VOID *
> +EFIAPI
> +BuildGuidDataHob (
> +  IN CONST EFI_GUID              *Guid,
> +  IN VOID                        *Data,
> +  IN UINTN                       DataLength
> +  )
> +{
> +  VOID  *HobData;
> +
> +  ASSERT (Data != NULL || DataLength == 0);
> +
> +  HobData = BuildGuidHob (Guid, DataLength);
> +
> +  return CopyMem (HobData, Data, DataLength);
> +}
> +
> +/**
> +  Builds a Firmware Volume HOB.
> +
> +  This function builds a Firmware Volume HOB.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The base address of the Firmware Volume.
> +  @param  Length        The size of the Firmware Volume in bytes.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildFvHob (
> +  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN UINT64                      Length
> +  )
> +{
> +  EFI_HOB_FIRMWARE_VOLUME  *Hob;
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof
> (EFI_HOB_FIRMWARE_VOLUME));
> +
> +  Hob->BaseAddress = BaseAddress;
> +  Hob->Length      = Length;
> +}
> +
> +
> +/**
> +  Builds a EFI_HOB_TYPE_FV2 HOB.
> +
> +  This function builds a EFI_HOB_TYPE_FV2 HOB.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The base address of the Firmware Volume.
> +  @param  Length        The size of the Firmware Volume in bytes.
> +  @param  FvName       The name of the Firmware Volume.
> +  @param  FileName      The name of the file.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildFv2Hob (
> +  IN          EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN          UINT64                      Length,
> +  IN CONST    EFI_GUID                    *FvName,
> +  IN CONST    EFI_GUID                    *FileName
> +  )
> +{
> +  EFI_HOB_FIRMWARE_VOLUME2  *Hob;
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof
> (EFI_HOB_FIRMWARE_VOLUME2));
> +
> +  Hob->BaseAddress = BaseAddress;
> +  Hob->Length      = Length;
> +  CopyGuid (&Hob->FvName, FvName);
> +  CopyGuid (&Hob->FileName, FileName);
> +}
> +
> +
> +/**
> +  Builds a HOB for the CPU.
> +
> +  This function builds a HOB for the CPU.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  SizeOfMemorySpace   The maximum physical memory
> addressability of the processor.
> +  @param  SizeOfIoSpace       The maximum physical I/O
> addressability of the processor.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildCpuHob (
> +  IN UINT8                       SizeOfMemorySpace,
> +  IN UINT8                       SizeOfIoSpace
> +  )
> +{
> +  EFI_HOB_CPU  *Hob;
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
> +
> +  Hob->SizeOfMemorySpace = SizeOfMemorySpace;
> +  Hob->SizeOfIoSpace     = SizeOfIoSpace;
> +
> +  //
> +  // Zero the reserved space to match HOB spec
> +  //
> +  ZeroMem (Hob->Reserved, sizeof (Hob->Reserved));
> +}
> +
> +/**
> +  Builds a HOB for the memory allocation.
> +
> +  This function builds a HOB for the memory allocation.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The 64 bit physical address of the memory.
> +  @param  Length        The length of the memory allocation in
> bytes.
> +  @param  MemoryType    Type of memory allocated by this HOB.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildMemoryAllocationHob (
> +  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN UINT64                      Length,
> +  IN EFI_MEMORY_TYPE             MemoryType
> +  )
> +{
> +  EFI_HOB_MEMORY_ALLOCATION  *Hob;
> +
> +  ASSERT (((BaseAddress & (EFI_PAGE_SIZE - 1)) == 0) &&
> +          ((Length & (EFI_PAGE_SIZE - 1)) == 0));
> +
> +  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION));
> +
> +  ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
> +  Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
> +  Hob->AllocDescriptor.MemoryLength      = Length;
> +  Hob->AllocDescriptor.MemoryType        = MemoryType;
> +  //
> +  // Zero the reserved space to match HOB spec
> +  //
> +  ZeroMem (Hob->AllocDescriptor.Reserved, sizeof
> (Hob->AllocDescriptor.Reserved));
> +}
> +
> +/**
> +  Builds a HOB that describes a chunk of system memory with Owner
> GUID.
> +
> +  This function builds a HOB that describes a chunk of system memory.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  ResourceType        The type of resource described by this
> HOB.
> +  @param  ResourceAttribute   The resource attributes of the memory
> described by this HOB.
> +  @param  PhysicalStart       The 64 bit physical address of memory
> described by this HOB.
> +  @param  NumberOfBytes       The length of the memory described
> by this HOB in bytes.
> +  @param  OwnerGUID           GUID for the owner of this
> resource.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildResourceDescriptorWithOwnerHob (
> +  IN EFI_RESOURCE_TYPE            ResourceType,
> +  IN EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttribute,
> +  IN EFI_PHYSICAL_ADDRESS         PhysicalStart,
> +  IN UINT64                       NumberOfBytes,
> +  IN EFI_GUID                     *OwnerGUID
> +  )
> +{
> +  ASSERT (FALSE);
> +}
> +
> +/**
> +  Builds a Capsule Volume HOB.
> +
> +  This function builds a Capsule Volume HOB.
> +  If the platform does not support Capsule Volume HOBs, then ASSERT().
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The base address of the Capsule Volume.
> +  @param  Length        The size of the Capsule Volume in bytes.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildCvHob (
> +  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN UINT64                      Length
> +  )
> +{
> +  ASSERT (FALSE);
> +}
> +
> +
> +/**
> +  Builds a HOB for the BSP store.
> +
> +  This function builds a HOB for BSP store.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The 64 bit physical address of the BSP.
> +  @param  Length        The length of the BSP store in bytes.
> +  @param  MemoryType    Type of memory allocated by this HOB.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildBspStoreHob (
> +  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN UINT64                      Length,
> +  IN EFI_MEMORY_TYPE             MemoryType
> +  )
> +{
> +  ASSERT (FALSE);
> +}
> +
> +/**
> +  Builds a HOB for the Stack.
> +
> +  This function builds a HOB for the stack.
> +  If there is no additional space for HOB creation, then ASSERT().
> +
> +  @param  BaseAddress   The 64 bit physical address of the Stack.
> +  @param  Length        The length of the stack in bytes.
> +
> +**/
> +VOID
> +EFIAPI
> +BuildStackHob (
> +  IN EFI_PHYSICAL_ADDRESS        BaseAddress,
> +  IN UINT64                      Length
> +  )
> +{
> +  ASSERT (FALSE);
> +}
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.i
> nf
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.i
> nf
> new file mode 100644
> index 000000000000..542a19cc4bec
> --- /dev/null
> +++
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.i
> nf
> @@ -0,0 +1,45 @@
> +## @file
> +# Instance of HOB Library for Standalone MM modules.
> +#
> +# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +# Copyright (c) 2018, 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                      = HobLib
> +  FILE_GUID                      =
> 8262551B-AB2D-4E76-99FC-5EBB83F4988E
> +  MODULE_TYPE                    = MM_STANDALONE
> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032
> +  LIBRARY_CLASS                  = HobLib|MM_STANDALONE
> +  CONSTRUCTOR                    = HobLibConstructor
> +
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> +#
> +
> +[Sources]
> +  StandaloneMmHobLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  MmServicesTableLib
> +
> +[Guids]
> +  gEfiHobListGuid                               ## CONSUMES
> ## SystemTable
> +  gEfiHobMemoryAllocModuleGuid                  ## CONSUMES
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE
  2019-01-16 20:22 ` [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE Ard Biesheuvel
@ 2019-01-18 15:24   ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:24 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib:
> restrict to MM_CORE_STANDALONE
> 
> Remove MM_STANDALONE from the list of permitted modules for this
> library.
> It should only be used by the standalone MM core.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCore
> HobLib.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCor
> eHobLib.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCor
> eHobLib.inf
> index db19d3c926e8..ac036e31cf5e 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCor
> eHobLib.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCor
> eHobLib.inf
> @@ -24,7 +24,7 @@ [Defines]
>    MODULE_TYPE                    = MM_CORE_STANDALONE
>    VERSION_STRING                 = 1.0
>    PI_SPECIFICATION_VERSION       = 0x00010032
> -  LIBRARY_CLASS                  =
> HobLib|MM_CORE_STANDALONE MM_STANDALONE
> +  LIBRARY_CLASS                  =
> HobLib|MM_CORE_STANDALONE
> 
>  #
>  #  VALID_ARCHITECTURES           = AARCH64
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref
  2019-01-16 20:22 ` [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref Ard Biesheuvel
@ 2019-01-18 15:27   ` Yao, Jiewen
  2019-01-18 15:31     ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:27 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

Usually, we do not encourage to use SerialPortLib directly in a hardware independent environment.
I do not suggest we bring an architecture dependency on the existence of SerialPort in a common code.

However, if ARCH64 has some specific code that must use SerialPortLib explicitly, I am OK.
Can we move SerialPortLib under [LibraryClasses.AARCH64] ?




> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:23 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> add missing SerialPortLib ref
> 
> StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
> so add SerialPortLib to its list of LibraryClasses.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> reEntryPoint.inf | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> index 3222cd359f3e..769eaeeefbea 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> @@ -43,6 +43,7 @@ [Packages.AARCH64]
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> +  SerialPortLib
> 
>  [LibraryClasses.AARCH64]
>    StandaloneMmMmuLib
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref
  2019-01-18 15:27   ` Yao, Jiewen
@ 2019-01-18 15:31     ` Ard Biesheuvel
  2019-01-18 15:33       ` Yao, Jiewen
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-18 15:31 UTC (permalink / raw)
  To: Yao, Jiewen
  Cc: edk2-devel@lists.01.org, Achin Gupta, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

On Fri, 18 Jan 2019 at 16:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Usually, we do not encourage to use SerialPortLib directly in a hardware independent environment.
> I do not suggest we bring an architecture dependency on the existence of SerialPort in a common code.
>
> However, if ARCH64 has some specific code that must use SerialPortLib explicitly, I am OK.
> Can we move SerialPortLib under [LibraryClasses.AARCH64] ?
>

I am happy to remove the SerialPortInitialize call, and instead rely
on DebugLib to pull in SerialPortLIb if it wants to (and rely on the
constructor to be invoked implicitly)



>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Wednesday, January 16, 2019 12:23 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> > Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> > Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> > Mujawar <Sami.Mujawar@arm.com>
> > Subject: [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> > add missing SerialPortLib ref
> >
> > StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
> > so add SerialPortLib to its list of LibraryClasses.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > ---
> >
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> > reEntryPoint.inf | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > index 3222cd359f3e..769eaeeefbea 100644
> > ---
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > +++
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > @@ -43,6 +43,7 @@ [Packages.AARCH64]
> >  [LibraryClasses]
> >    BaseLib
> >    DebugLib
> > +  SerialPortLib
> >
> >  [LibraryClasses.AARCH64]
> >    StandaloneMmMmuLib
> > --
> > 2.17.1
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref
  2019-01-18 15:31     ` Ard Biesheuvel
@ 2019-01-18 15:33       ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:33 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Achin Gupta, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

That would be even better. Thanks Ard.

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, January 18, 2019 7:32 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; Achin Gupta <achin.gupta@arm.com>;
> Supreeth Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: Re: [PATCH v2 05/11]
> StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib
> ref
> 
> On Fri, 18 Jan 2019 at 16:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > Usually, we do not encourage to use SerialPortLib directly in a hardware
> independent environment.
> > I do not suggest we bring an architecture dependency on the existence of
> SerialPort in a common code.
> >
> > However, if ARCH64 has some specific code that must use SerialPortLib
> explicitly, I am OK.
> > Can we move SerialPortLib under [LibraryClasses.AARCH64] ?
> >
> 
> I am happy to remove the SerialPortInitialize call, and instead rely
> on DebugLib to pull in SerialPortLIb if it wants to (and rely on the
> constructor to be invoked implicitly)
> 
> 
> 
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > Sent: Wednesday, January 16, 2019 12:23 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Supreeth
> > > Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> > > <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> > > Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> > > Mujawar <Sami.Mujawar@arm.com>
> > > Subject: [PATCH v2 05/11]
> StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> > > add missing SerialPortLib ref
> > >
> > > StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
> > > so add SerialPortLib to its list of LibraryClasses.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > > ---
> > >
> > >
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> > > reEntryPoint.inf | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > index 3222cd359f3e..769eaeeefbea 100644
> > > ---
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > +++
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > @@ -43,6 +43,7 @@ [Packages.AARCH64]
> > >  [LibraryClasses]
> > >    BaseLib
> > >    DebugLib
> > > +  SerialPortLib
> > >
> > >  [LibraryClasses.AARCH64]
> > >    StandaloneMmMmuLib
> > > --
> > > 2.17.1
> >

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
  2019-01-16 20:22 ` [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice Ard Biesheuvel
@ 2019-01-18 15:34   ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:34 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:23 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy
> dispatched image twice
> 
> The dispatcher uses the PE/COFF loader to load images into the heap,
> but only does so after copying the entire image first, leading to
> two copies being made for no good reason.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  StandaloneMmPkg/Core/Dispatcher.c | 30 +-------------------
>  1 file changed, 1 insertion(+), 29 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 8d009b4f80c1..8a2ad5118d92 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -294,7 +294,6 @@ MmLoadImage (
>    IN OUT EFI_MM_DRIVER_ENTRY  *DriverEntry
>    )
>  {
> -  VOID                           *Buffer;
>    UINTN                          PageCount;
>    EFI_STATUS                     Status;
>    EFI_PHYSICAL_ADDRESS           DstBuffer;
> @@ -302,17 +301,12 @@ MmLoadImage (
> 
>    DEBUG ((DEBUG_INFO, "MmLoadImage - %g\n",
> &DriverEntry->FileName));
> 
> -  Buffer = AllocateCopyPool (DriverEntry->Pe32DataSize,
> DriverEntry->Pe32Data);
> -  if (Buffer == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
>    Status               = EFI_SUCCESS;
> 
>    //
>    // Initialize ImageContext
>    //
> -  ImageContext.Handle = Buffer;
> +  ImageContext.Handle = DriverEntry->Pe32Data;
>    ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
> 
>    //
> @@ -320,9 +314,6 @@ MmLoadImage (
>    //
>    Status = PeCoffLoaderGetImageInfo (&ImageContext);
>    if (EFI_ERROR (Status)) {
> -    if (Buffer != NULL) {
> -      MmFreePool (Buffer);
> -    }
>      return Status;
>    }
> 
> @@ -336,9 +327,6 @@ MmLoadImage (
>               &DstBuffer
>               );
>    if (EFI_ERROR (Status)) {
> -    if (Buffer != NULL) {
> -      MmFreePool (Buffer);
> -    }
>      return Status;
>    }
> 
> @@ -355,9 +343,6 @@ MmLoadImage (
>    //
>    Status = PeCoffLoaderLoadImage (&ImageContext);
>    if (EFI_ERROR (Status)) {
> -    if (Buffer != NULL) {
> -      MmFreePool (Buffer);
> -    }
>      MmFreePages (DstBuffer, PageCount);
>      return Status;
>    }
> @@ -367,9 +352,6 @@ MmLoadImage (
>    //
>    Status = PeCoffLoaderRelocateImage (&ImageContext);
>    if (EFI_ERROR (Status)) {
> -    if (Buffer != NULL) {
> -      MmFreePool (Buffer);
> -    }
>      MmFreePages (DstBuffer, PageCount);
>      return Status;
>    }
> @@ -393,9 +375,6 @@ MmLoadImage (
>                                                (VOID
> **)&DriverEntry->LoadedImage
>                                                );
>      if (EFI_ERROR (Status)) {
> -      if (Buffer != NULL) {
> -        MmFreePool (Buffer);
> -      }
>        MmFreePages (DstBuffer, PageCount);
>        return Status;
>      }
> @@ -482,13 +461,6 @@ MmLoadImage (
> 
>    DEBUG_CODE_END ();
> 
> -  //
> -  // Free buffer allocated by Fv->ReadSection.
> -  //
> -  // The UEFI Boot Services FreePool() function must be used because
> Fv->ReadSection
> -  // used the UEFI Boot Services AllocatePool() function
> -  //
> -  MmFreePool (Buffer);
>    return Status;
>  }
> 
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes
  2019-01-16 20:22 ` [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
@ 2019-01-18 15:39   ` Yao, Jiewen
  2019-01-18 15:41     ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:39 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org

The idea seems good.

May I know if there is any restriction on 64 handlers?

+STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];

If a system is configured with more handlers, what is expected behavior?


Thank you
Yao Jiewen


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 16, 2019 12:23 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated
> firmware volumes
> 
> Standalone MM requires 4 KB section alignment for all images, so that
> strict permissions can be applied. Unfortunately, this results in a
> lot of wasted space, which is usually costly in the secure world
> environment that standalone MM is expected to operate in.
> 
> So let's permit the standalone MM drivers (but not the core) to be
> delivered in a compressed firmware volume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  StandaloneMmPkg/Core/FwVol.c
> | 99 ++++++++++++++++++--
>  StandaloneMmPkg/Core/StandaloneMmCore.inf
> |  1 +
> 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> oneMmCoreEntryPoint.c |  5 +
> 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> reEntryPoint.inf       |  3 +
>  4 files changed, 99 insertions(+), 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/FwVol.c
> b/StandaloneMmPkg/Core/FwVol.c
> index 5abf98c24797..8eb827dda5c4 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include "StandaloneMmCore.h"
>  #include <Library/FvLib.h>
> +#include <Library/ExtractGuidedSectionLib.h>
> 
>  //
>  // List of file types supported by dispatcher
> @@ -65,15 +66,25 @@ Returns:
> 
>  --*/
>  {
> -  EFI_STATUS          Status;
> -  EFI_STATUS          DepexStatus;
> -  EFI_FFS_FILE_HEADER *FileHeader;
> -  EFI_FV_FILETYPE     FileType;
> -  VOID                *Pe32Data;
> -  UINTN               Pe32DataSize;
> -  VOID                *Depex;
> -  UINTN               DepexSize;
> -  UINTN               Index;
> +  EFI_STATUS                              Status;
> +  EFI_STATUS                              DepexStatus;
> +  EFI_FFS_FILE_HEADER                     *FileHeader;
> +  EFI_FV_FILETYPE                         FileType;
> +  VOID                                    *Pe32Data;
> +  UINTN                                   Pe32DataSize;
> +  VOID                                    *Depex;
> +  UINTN                                   DepexSize;
> +  UINTN                                   Index;
> +  EFI_COMMON_SECTION_HEADER               *Section;
> +  VOID                                    *SectionData;
> +  UINTN                                   SectionDataSize;
> +  UINT32                                  DstBufferSize;
> +  VOID                                    *ScratchBuffer;
> +  UINT32                                  ScratchBufferSize;
> +  VOID                                    *DstBuffer;
> +  UINT16                                  SectionAttribute;
> +  UINT32                                  AuthenticationStatus;
> +  EFI_FIRMWARE_VOLUME_HEADER              *InnerFvHeader;
> 
>    DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n",
> FwVolHeader));
> 
> @@ -83,6 +94,71 @@ Returns:
> 
>    FvIsBeingProcesssed (FwVolHeader);
> 
> +  //
> +  // First check for encapsulated compressed firmware volumes
> +  //
> +  FileHeader = NULL;
> +  do {
> +    Status = FfsFindNextFile
> (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> +               FwVolHeader, &FileHeader);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +    Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> FileHeader,
> +               &SectionData, &SectionDataSize);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +    Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> +    Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
> +               &ScratchBufferSize, &SectionAttribute);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +
> +    //
> +    // Allocate scratch buffer
> +    //
> +    ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (ScratchBufferSize));
> +    if (ScratchBuffer == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    //
> +    // Allocate destination buffer
> +    //
> +    DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (DstBufferSize));
> +    if (DstBuffer == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    //
> +    // Call decompress function
> +    //
> +    Status = ExtractGuidedSectionDecode (Section, &DstBuffer,
> ScratchBuffer,
> +               &AuthenticationStatus);
> +    FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> +    if (EFI_ERROR (Status)) {
> +      goto FreeDstBuffer;
> +    }
> +
> +    DEBUG ((DEBUG_INFO,
> +      "Processing compressed firmware volume (AuthenticationStatus
> == %x)\n",
> +      AuthenticationStatus));
> +
> +    Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
> +               EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
> +    if (EFI_ERROR (Status)) {
> +      goto FreeDstBuffer;
> +    }
> +
> +    InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(Section + 1);
> +    Status = MmCoreFfsFindMmDriver (InnerFvHeader);
> +    if (EFI_ERROR (Status)) {
> +      goto FreeDstBuffer;
> +    }
> +  } while (TRUE);
> +
>    for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]);
> Index++) {
>      DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n",
> mMmFileTypes[Index]));
>      FileType = mMmFileTypes[Index];
> @@ -100,5 +176,10 @@ Returns:
>      } while (!EFI_ERROR (Status));
>    }
> 
> +  return EFI_SUCCESS;
> +
> +FreeDstBuffer:
> +  FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
> +
>    return Status;
>  }
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index ff2b8b9cef03..83d31e2d92c5 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -49,6 +49,7 @@ [LibraryClasses]
>    BaseMemoryLib
>    CacheMaintenanceLib
>    DebugLib
> +  ExtractGuidedSectionLib
>    FvLib
>    HobLib
>    MemoryAllocationLib
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> index 5cca532456fd..67ff9112d5c0 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> @@ -205,6 +205,8 @@ GetSpmVersion (VOID)
>    return Status;
>  }
> 
> +STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
> +
>  /**
>    The entry point of Standalone MM Foundation.
> 
> @@ -285,6 +287,9 @@ _ModuleEntryPoint (
>      goto finish;
>    }
> 
> +  PcdSet64 (PcdGuidedExtractHandlerTableAddress,
> +    (UINT64)mExtractGuidedSectionHandlerInfo);
> +
>    //
>    // Create Hoblist based upon boot information passed by privileged
> software
>    //
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> index 769eaeeefbea..55d769fa77e4 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> @@ -54,3 +54,6 @@ [Guids]
>    gEfiMmPeiMmramMemoryReserveGuid
>    gEfiStandaloneMmNonSecureBufferGuid
>    gEfiArmTfCpuDriverEpDescriptorGuid
> +
> +[PatchPcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes
  2019-01-18 15:39   ` Yao, Jiewen
@ 2019-01-18 15:41     ` Ard Biesheuvel
  2019-01-18 15:49       ` Yao, Jiewen
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-18 15:41 UTC (permalink / raw)
  To: Yao, Jiewen
  Cc: edk2-devel@lists.01.org, Achin Gupta, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

On Fri, 18 Jan 2019 at 16:40, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> The idea seems good.
>
> May I know if there is any restriction on 64 handlers?
>
> +STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
>
> If a system is configured with more handlers, what is expected behavior?
>

Good question. I wasn't really sure how to implement this. For any
given platform configuration, I don't think you will ever need more
than one handler, unless you are encapsulating a compressed FV inside
a signed FV perhaps?

Do you have any suggestions how to improve this code?

>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Wednesday, January 16, 2019 12:23 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> > Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> > Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> > Mujawar <Sami.Mujawar@arm.com>
> > Subject: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated
> > firmware volumes
> >
> > Standalone MM requires 4 KB section alignment for all images, so that
> > strict permissions can be applied. Unfortunately, this results in a
> > lot of wasted space, which is usually costly in the secure world
> > environment that standalone MM is expected to operate in.
> >
> > So let's permit the standalone MM drivers (but not the core) to be
> > delivered in a compressed firmware volume.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  StandaloneMmPkg/Core/FwVol.c
> > | 99 ++++++++++++++++++--
> >  StandaloneMmPkg/Core/StandaloneMmCore.inf
> > |  1 +
> >
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> > oneMmCoreEntryPoint.c |  5 +
> >
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> > reEntryPoint.inf       |  3 +
> >  4 files changed, 99 insertions(+), 9 deletions(-)
> >
> > diff --git a/StandaloneMmPkg/Core/FwVol.c
> > b/StandaloneMmPkg/Core/FwVol.c
> > index 5abf98c24797..8eb827dda5c4 100644
> > --- a/StandaloneMmPkg/Core/FwVol.c
> > +++ b/StandaloneMmPkg/Core/FwVol.c
> > @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >
> >  #include "StandaloneMmCore.h"
> >  #include <Library/FvLib.h>
> > +#include <Library/ExtractGuidedSectionLib.h>
> >
> >  //
> >  // List of file types supported by dispatcher
> > @@ -65,15 +66,25 @@ Returns:
> >
> >  --*/
> >  {
> > -  EFI_STATUS          Status;
> > -  EFI_STATUS          DepexStatus;
> > -  EFI_FFS_FILE_HEADER *FileHeader;
> > -  EFI_FV_FILETYPE     FileType;
> > -  VOID                *Pe32Data;
> > -  UINTN               Pe32DataSize;
> > -  VOID                *Depex;
> > -  UINTN               DepexSize;
> > -  UINTN               Index;
> > +  EFI_STATUS                              Status;
> > +  EFI_STATUS                              DepexStatus;
> > +  EFI_FFS_FILE_HEADER                     *FileHeader;
> > +  EFI_FV_FILETYPE                         FileType;
> > +  VOID                                    *Pe32Data;
> > +  UINTN                                   Pe32DataSize;
> > +  VOID                                    *Depex;
> > +  UINTN                                   DepexSize;
> > +  UINTN                                   Index;
> > +  EFI_COMMON_SECTION_HEADER               *Section;
> > +  VOID                                    *SectionData;
> > +  UINTN                                   SectionDataSize;
> > +  UINT32                                  DstBufferSize;
> > +  VOID                                    *ScratchBuffer;
> > +  UINT32                                  ScratchBufferSize;
> > +  VOID                                    *DstBuffer;
> > +  UINT16                                  SectionAttribute;
> > +  UINT32                                  AuthenticationStatus;
> > +  EFI_FIRMWARE_VOLUME_HEADER              *InnerFvHeader;
> >
> >    DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n",
> > FwVolHeader));
> >
> > @@ -83,6 +94,71 @@ Returns:
> >
> >    FvIsBeingProcesssed (FwVolHeader);
> >
> > +  //
> > +  // First check for encapsulated compressed firmware volumes
> > +  //
> > +  FileHeader = NULL;
> > +  do {
> > +    Status = FfsFindNextFile
> > (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> > +               FwVolHeader, &FileHeader);
> > +    if (EFI_ERROR (Status)) {
> > +      break;
> > +    }
> > +    Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> > FileHeader,
> > +               &SectionData, &SectionDataSize);
> > +    if (EFI_ERROR (Status)) {
> > +      break;
> > +    }
> > +    Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> > +    Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
> > +               &ScratchBufferSize, &SectionAttribute);
> > +    if (EFI_ERROR (Status)) {
> > +      break;
> > +    }
> > +
> > +    //
> > +    // Allocate scratch buffer
> > +    //
> > +    ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> > (ScratchBufferSize));
> > +    if (ScratchBuffer == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    //
> > +    // Allocate destination buffer
> > +    //
> > +    DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> > (DstBufferSize));
> > +    if (DstBuffer == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    //
> > +    // Call decompress function
> > +    //
> > +    Status = ExtractGuidedSectionDecode (Section, &DstBuffer,
> > ScratchBuffer,
> > +               &AuthenticationStatus);
> > +    FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> > +    if (EFI_ERROR (Status)) {
> > +      goto FreeDstBuffer;
> > +    }
> > +
> > +    DEBUG ((DEBUG_INFO,
> > +      "Processing compressed firmware volume (AuthenticationStatus
> > == %x)\n",
> > +      AuthenticationStatus));
> > +
> > +    Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
> > +               EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
> > +    if (EFI_ERROR (Status)) {
> > +      goto FreeDstBuffer;
> > +    }
> > +
> > +    InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(Section + 1);
> > +    Status = MmCoreFfsFindMmDriver (InnerFvHeader);
> > +    if (EFI_ERROR (Status)) {
> > +      goto FreeDstBuffer;
> > +    }
> > +  } while (TRUE);
> > +
> >    for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]);
> > Index++) {
> >      DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n",
> > mMmFileTypes[Index]));
> >      FileType = mMmFileTypes[Index];
> > @@ -100,5 +176,10 @@ Returns:
> >      } while (!EFI_ERROR (Status));
> >    }
> >
> > +  return EFI_SUCCESS;
> > +
> > +FreeDstBuffer:
> > +  FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
> > +
> >    return Status;
> >  }
> > diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > index ff2b8b9cef03..83d31e2d92c5 100644
> > --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > @@ -49,6 +49,7 @@ [LibraryClasses]
> >    BaseMemoryLib
> >    CacheMaintenanceLib
> >    DebugLib
> > +  ExtractGuidedSectionLib
> >    FvLib
> >    HobLib
> >    MemoryAllocationLib
> > diff --git
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > aloneMmCoreEntryPoint.c
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > aloneMmCoreEntryPoint.c
> > index 5cca532456fd..67ff9112d5c0 100644
> > ---
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > aloneMmCoreEntryPoint.c
> > +++
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > aloneMmCoreEntryPoint.c
> > @@ -205,6 +205,8 @@ GetSpmVersion (VOID)
> >    return Status;
> >  }
> >
> > +STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
> > +
> >  /**
> >    The entry point of Standalone MM Foundation.
> >
> > @@ -285,6 +287,9 @@ _ModuleEntryPoint (
> >      goto finish;
> >    }
> >
> > +  PcdSet64 (PcdGuidedExtractHandlerTableAddress,
> > +    (UINT64)mExtractGuidedSectionHandlerInfo);
> > +
> >    //
> >    // Create Hoblist based upon boot information passed by privileged
> > software
> >    //
> > diff --git
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > index 769eaeeefbea..55d769fa77e4 100644
> > ---
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > +++
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > CoreEntryPoint.inf
> > @@ -54,3 +54,6 @@ [Guids]
> >    gEfiMmPeiMmramMemoryReserveGuid
> >    gEfiStandaloneMmNonSecureBufferGuid
> >    gEfiArmTfCpuDriverEpDescriptorGuid
> > +
> > +[PatchPcd]
> > +  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
> > --
> > 2.17.1
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements
  2019-01-18  9:26 ` [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Achin Gupta
@ 2019-01-18 15:41   ` Yao, Jiewen
  2019-01-21 14:39     ` Ard Biesheuvel
  0 siblings, 1 reply; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:41 UTC (permalink / raw)
  To: Achin Gupta, Ard Biesheuvel, edk2-devel@lists.01.org
  Cc: Supreeth Venkatesh, Leif Lindholm, Jagadeesh Ujja, Thomas Abraham,
	Sami Mujawar, nd

Thanks to remind me.
I took a look at all patches.

I skipped all ARM specific patches, and reviewed rest of them.
All my feedback is provided in the individual patch.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Achin Gupta [mailto:Achin.Gupta@arm.com]
> Sent: Friday, January 18, 2019 1:27 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org;
> Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <Jagadeesh.Ujja@arm.com>;
> Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar
> <Sami.Mujawar@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and
> improvements
> 
> Hi Ard,
> 
> For all the patches...
> 
> Reviewed-by: Achin Gupta <achin.gupta@arm.com>
> 
> Jiewen. There are changes to the generic Standalone MM code in this series.
> Do you want to have a look as well?
> 
> cheers,
> Achin
> ________________________________________
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 16 January 2019 20:22
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel; Achin Gupta; Jiewen Yao; Supreeth Venkatesh; Leif
> Lindholm; Jagadeesh Ujja; Thomas Abraham; Sami Mujawar
> Subject: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and
> improvements
> 
> This series addresses a number of issues I ran into while bringing up
> the standalone MM based authenticated variable store on the SynQuacer
> (AArch64) platform.
> 
> Patches #1 - #3 are based on Jagadeesh's patch that imports some staging
> code into StandaloneMmPkg, with the following changes:
> - drop unused source files, GUID references are other unused bit,
> - clean up comments referring to the MM core implementation.
> 
> Patches #4 - #9 are obvious fixes/improvements.
> 
> Patch #10 adds support for TE formatted MM_CORE_STANDALONE binaries.
> This is useful given that the 4 KB section alignment we require in
> AArch64 implementations of standalone MM (due to the strict separation
> between code and date) results in 8 KB of wasted space at the start of
> the firmware volume. This can be reduced to 4 KB when using a TE image
> and the FIXED attribute in the associated [Rule] section, by leveraging
> an existing optimization in the FFS generation code that aligns TE images
> by reducing FFS padding rather than adding more.
> 
> Patch #11 is another space optimization: it reuses the existing support
> for encapsulated compressed firmware volumes in FFS files to shrink the
> size of the primary standalone MM FV considerably. Again, due to
> alignment requirements, there is significant bloat in the uncompressed
> images (4 KB for the PE/COFF header, and up to 4 KB per section for the
> .text, .data and .reloc sections), making the absolute minimum size of
> any trivial MM_STANDALONE module 16 KB.
> 
> Changes since v1:
> - add patches #1 - #3
> - add Supreeth's ack to patches #4 - #7
> 
> Cc: Achin Gupta <achin.gupta@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> 
> Ard Biesheuvel (11):
>   StandaloneMmPkg: add HobLib implementation for MM_STANDALONE
> modules
>   StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib
> implementation
>   StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to
> MM_CORE_STANDALONE
>   StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing
> SerialPortLib
>     ref
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for
> ASCII
>     strings
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
>     ASSERT_EFI_ERROR()s
>   StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime
>     attribute
>   StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE
>     images
>   StandaloneMmPkg/Core: permit encapsulated firmware volumes
> 
>  StandaloneMmPkg/Core/Dispatcher.c             |  30 +-
>  StandaloneMmPkg/Core/FwVol.c                  |  99 ++-
>  StandaloneMmPkg/Core/StandaloneMmCore.inf     |   1 +
>  .../StandaloneMmCpu/AArch64/EventHandle.c     |   2 +-
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c |   6 +-
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h |   8 +-
>  .../AArch64/StandaloneMmCpu.inf               |   4 +-
>  .../AArch64/SetPermissions.c                  | 109 +--
>  .../AArch64/StandaloneMmCoreEntryPoint.c      |   7 +-
>  .../StandaloneMmCoreEntryPoint.inf            |   4 +
>  .../StandaloneMmCoreHobLib.inf                |   2 +-
>  .../StandaloneMmHobLib/StandaloneMmHobLib.c   | 649
> ++++++++++++++
>  .../StandaloneMmHobLib/StandaloneMmHobLib.inf |  45 +
>  .../StandaloneMmMemoryAllocationLib.c         | 822
> ++++++++++++++++++
>  .../StandaloneMmMemoryAllocationLib.inf       |  42 +
>  .../StandaloneMmPeCoffExtraActionLib.c        |   9 +-
>  16 files changed, 1716 insertions(+), 123 deletions(-)
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.c
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.inf
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes
  2019-01-18 15:41     ` Ard Biesheuvel
@ 2019-01-18 15:49       ` Yao, Jiewen
  0 siblings, 0 replies; 25+ messages in thread
From: Yao, Jiewen @ 2019-01-18 15:49 UTC (permalink / raw)
  To: Ard Biesheuvel, Kinney, Michael D
  Cc: edk2-devel@lists.01.org, Achin Gupta, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Panakamattam Abraham,
	Sami Mujawar

Add Mike Kinney.

Hi Mike
Do you have any suggestion on how to implement this in a self-contained environment for gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress?

 STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
 PcdSet64 (PcdGuidedExtractHandlerTableAddress, (UINT64)mExtractGuidedSectionHandlerInfo);

Maybe, we can consider below from secure coding perspective.
1) Define a MACRO for 64.
2) Add a check (somewhere) to see if there is overflow on 64, then stop and report error.



Thank you
Yao Jiewen

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, January 18, 2019 7:42 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; Achin Gupta <achin.gupta@arm.com>;
> Supreeth Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>
> Subject: Re: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated
> firmware volumes
> 
> On Fri, 18 Jan 2019 at 16:40, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > The idea seems good.
> >
> > May I know if there is any restriction on 64 handlers?
> >
> > +STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
> >
> > If a system is configured with more handlers, what is expected behavior?
> >
> 
> Good question. I wasn't really sure how to implement this. For any
> given platform configuration, I don't think you will ever need more
> than one handler, unless you are encapsulating a compressed FV inside
> a signed FV perhaps?
> 
> Do you have any suggestions how to improve this code?
> 
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > Sent: Wednesday, January 16, 2019 12:23 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Supreeth
> > > Venkatesh <supreeth.venkatesh@arm.com>; Leif Lindholm
> > > <leif.lindholm@linaro.org>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>;
> > > Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Sami
> > > Mujawar <Sami.Mujawar@arm.com>
> > > Subject: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated
> > > firmware volumes
> > >
> > > Standalone MM requires 4 KB section alignment for all images, so that
> > > strict permissions can be applied. Unfortunately, this results in a
> > > lot of wasted space, which is usually costly in the secure world
> > > environment that standalone MM is expected to operate in.
> > >
> > > So let's permit the standalone MM drivers (but not the core) to be
> > > delivered in a compressed firmware volume.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  StandaloneMmPkg/Core/FwVol.c
> > > | 99 ++++++++++++++++++--
> > >  StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > |  1 +
> > >
> > >
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> > > oneMmCoreEntryPoint.c |  5 +
> > >
> > >
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
> > > reEntryPoint.inf       |  3 +
> > >  4 files changed, 99 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/StandaloneMmPkg/Core/FwVol.c
> > > b/StandaloneMmPkg/Core/FwVol.c
> > > index 5abf98c24797..8eb827dda5c4 100644
> > > --- a/StandaloneMmPkg/Core/FwVol.c
> > > +++ b/StandaloneMmPkg/Core/FwVol.c
> > > @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> > > ANY KIND, EITHER EXPRESS OR IMPLIED.
> > >
> > >  #include "StandaloneMmCore.h"
> > >  #include <Library/FvLib.h>
> > > +#include <Library/ExtractGuidedSectionLib.h>
> > >
> > >  //
> > >  // List of file types supported by dispatcher
> > > @@ -65,15 +66,25 @@ Returns:
> > >
> > >  --*/
> > >  {
> > > -  EFI_STATUS          Status;
> > > -  EFI_STATUS          DepexStatus;
> > > -  EFI_FFS_FILE_HEADER *FileHeader;
> > > -  EFI_FV_FILETYPE     FileType;
> > > -  VOID                *Pe32Data;
> > > -  UINTN               Pe32DataSize;
> > > -  VOID                *Depex;
> > > -  UINTN               DepexSize;
> > > -  UINTN               Index;
> > > +  EFI_STATUS                              Status;
> > > +  EFI_STATUS                              DepexStatus;
> > > +  EFI_FFS_FILE_HEADER                     *FileHeader;
> > > +  EFI_FV_FILETYPE                         FileType;
> > > +  VOID                                    *Pe32Data;
> > > +  UINTN                                   Pe32DataSize;
> > > +  VOID                                    *Depex;
> > > +  UINTN                                   DepexSize;
> > > +  UINTN                                   Index;
> > > +  EFI_COMMON_SECTION_HEADER               *Section;
> > > +  VOID                                    *SectionData;
> > > +  UINTN                                   SectionDataSize;
> > > +  UINT32                                  DstBufferSize;
> > > +  VOID                                    *ScratchBuffer;
> > > +  UINT32                                  ScratchBufferSize;
> > > +  VOID                                    *DstBuffer;
> > > +  UINT16                                  SectionAttribute;
> > > +  UINT32
> AuthenticationStatus;
> > > +  EFI_FIRMWARE_VOLUME_HEADER              *InnerFvHeader;
> > >
> > >    DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n",
> > > FwVolHeader));
> > >
> > > @@ -83,6 +94,71 @@ Returns:
> > >
> > >    FvIsBeingProcesssed (FwVolHeader);
> > >
> > > +  //
> > > +  // First check for encapsulated compressed firmware volumes
> > > +  //
> > > +  FileHeader = NULL;
> > > +  do {
> > > +    Status = FfsFindNextFile
> > > (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> > > +               FwVolHeader, &FileHeader);
> > > +    if (EFI_ERROR (Status)) {
> > > +      break;
> > > +    }
> > > +    Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> > > FileHeader,
> > > +               &SectionData, &SectionDataSize);
> > > +    if (EFI_ERROR (Status)) {
> > > +      break;
> > > +    }
> > > +    Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> > > +    Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
> > > +               &ScratchBufferSize, &SectionAttribute);
> > > +    if (EFI_ERROR (Status)) {
> > > +      break;
> > > +    }
> > > +
> > > +    //
> > > +    // Allocate scratch buffer
> > > +    //
> > > +    ScratchBuffer = (VOID *)(UINTN)AllocatePages
> (EFI_SIZE_TO_PAGES
> > > (ScratchBufferSize));
> > > +    if (ScratchBuffer == NULL) {
> > > +      return EFI_OUT_OF_RESOURCES;
> > > +    }
> > > +
> > > +    //
> > > +    // Allocate destination buffer
> > > +    //
> > > +    DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> > > (DstBufferSize));
> > > +    if (DstBuffer == NULL) {
> > > +      return EFI_OUT_OF_RESOURCES;
> > > +    }
> > > +
> > > +    //
> > > +    // Call decompress function
> > > +    //
> > > +    Status = ExtractGuidedSectionDecode (Section, &DstBuffer,
> > > ScratchBuffer,
> > > +               &AuthenticationStatus);
> > > +    FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES
> (ScratchBufferSize));
> > > +    if (EFI_ERROR (Status)) {
> > > +      goto FreeDstBuffer;
> > > +    }
> > > +
> > > +    DEBUG ((DEBUG_INFO,
> > > +      "Processing compressed firmware volume (AuthenticationStatus
> > > == %x)\n",
> > > +      AuthenticationStatus));
> > > +
> > > +    Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
> > > +               EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
> &Section);
> > > +    if (EFI_ERROR (Status)) {
> > > +      goto FreeDstBuffer;
> > > +    }
> > > +
> > > +    InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(Section +
> 1);
> > > +    Status = MmCoreFfsFindMmDriver (InnerFvHeader);
> > > +    if (EFI_ERROR (Status)) {
> > > +      goto FreeDstBuffer;
> > > +    }
> > > +  } while (TRUE);
> > > +
> > >    for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof
> (mMmFileTypes[0]);
> > > Index++) {
> > >      DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n",
> > > mMmFileTypes[Index]));
> > >      FileType = mMmFileTypes[Index];
> > > @@ -100,5 +176,10 @@ Returns:
> > >      } while (!EFI_ERROR (Status));
> > >    }
> > >
> > > +  return EFI_SUCCESS;
> > > +
> > > +FreeDstBuffer:
> > > +  FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
> > > +
> > >    return Status;
> > >  }
> > > diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > index ff2b8b9cef03..83d31e2d92c5 100644
> > > --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > @@ -49,6 +49,7 @@ [LibraryClasses]
> > >    BaseMemoryLib
> > >    CacheMaintenanceLib
> > >    DebugLib
> > > +  ExtractGuidedSectionLib
> > >    FvLib
> > >    HobLib
> > >    MemoryAllocationLib
> > > diff --git
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > > aloneMmCoreEntryPoint.c
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > > aloneMmCoreEntryPoint.c
> > > index 5cca532456fd..67ff9112d5c0 100644
> > > ---
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > > aloneMmCoreEntryPoint.c
> > > +++
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> > > aloneMmCoreEntryPoint.c
> > > @@ -205,6 +205,8 @@ GetSpmVersion (VOID)
> > >    return Status;
> > >  }
> > >
> > > +STATIC UINT64 mExtractGuidedSectionHandlerInfo[64];
> > > +
> > >  /**
> > >    The entry point of Standalone MM Foundation.
> > >
> > > @@ -285,6 +287,9 @@ _ModuleEntryPoint (
> > >      goto finish;
> > >    }
> > >
> > > +  PcdSet64 (PcdGuidedExtractHandlerTableAddress,
> > > +    (UINT64)mExtractGuidedSectionHandlerInfo);
> > > +
> > >    //
> > >    // Create Hoblist based upon boot information passed by privileged
> > > software
> > >    //
> > > diff --git
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > index 769eaeeefbea..55d769fa77e4 100644
> > > ---
> > >
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > +++
> > >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> > > CoreEntryPoint.inf
> > > @@ -54,3 +54,6 @@ [Guids]
> > >    gEfiMmPeiMmramMemoryReserveGuid
> > >    gEfiStandaloneMmNonSecureBufferGuid
> > >    gEfiArmTfCpuDriverEpDescriptorGuid
> > > +
> > > +[PatchPcd]
> > > +  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
> > > --
> > > 2.17.1
> >

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements
  2019-01-18 15:41   ` Yao, Jiewen
@ 2019-01-21 14:39     ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2019-01-21 14:39 UTC (permalink / raw)
  To: Yao, Jiewen
  Cc: Achin Gupta, edk2-devel@lists.01.org, Supreeth Venkatesh,
	Leif Lindholm, Jagadeesh Ujja, Thomas Abraham, Sami Mujawar, nd

On Fri, 18 Jan 2019 at 16:42, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Thanks to remind me.
> I took a look at all patches.
>
> I skipped all ARM specific patches, and reviewed rest of them.
> All my feedback is provided in the individual patch.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: Achin Gupta [mailto:Achin.Gupta@arm.com]
> > Sent: Friday, January 18, 2019 1:27 AM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org;
> > Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Jagadeesh Ujja <Jagadeesh.Ujja@arm.com>;
> > Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar
> > <Sami.Mujawar@arm.com>; nd <nd@arm.com>
> > Subject: Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and
> > improvements
> >
> > Hi Ard,
> >
> > For all the patches...
> >
> > Reviewed-by: Achin Gupta <achin.gupta@arm.com>
> >
> > Jiewen. There are changes to the generic Standalone MM code in this series.
> > Do you want to have a look as well?
> >

Thanks all

I pushed most of this series

380148b685b7 StandaloneMmPkg: add HobLib implementation for
MM_STANDALONE modules
66dde0c7516b StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib
implementation
2cc186178bfd StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to
MM_CORE_STANDALONE
c8102727edb0 StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
41915a19a772 StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a
modifier for ASCII strings
d2f438bf6a51 StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
ASSERT_EFI_ERROR()s
77746e70807d StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore
runtime attribute
877013d0a58f StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
4b28452d9863 StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the
use of TE images

The remaining two patches will be included in the next batch of
StandaloneMmPkg work.


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-01-21 14:39 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-16 20:22 [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 01/11] StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules Ard Biesheuvel
2019-01-18 15:24   ` Yao, Jiewen
2019-01-16 20:22 ` [PATCH v2 02/11] StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation Ard Biesheuvel
2019-01-18 15:23   ` Yao, Jiewen
2019-01-16 20:22 ` [PATCH v2 03/11] StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE Ard Biesheuvel
2019-01-18 15:24   ` Yao, Jiewen
2019-01-16 20:22 ` [PATCH v2 04/11] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 05/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref Ard Biesheuvel
2019-01-18 15:27   ` Yao, Jiewen
2019-01-18 15:31     ` Ard Biesheuvel
2019-01-18 15:33       ` Yao, Jiewen
2019-01-16 20:22 ` [PATCH v2 06/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 07/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 08/11] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 09/11] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice Ard Biesheuvel
2019-01-18 15:34   ` Yao, Jiewen
2019-01-16 20:22 ` [PATCH v2 10/11] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images Ard Biesheuvel
2019-01-16 20:22 ` [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
2019-01-18 15:39   ` Yao, Jiewen
2019-01-18 15:41     ` Ard Biesheuvel
2019-01-18 15:49       ` Yao, Jiewen
2019-01-18  9:26 ` [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements Achin Gupta
2019-01-18 15:41   ` Yao, Jiewen
2019-01-21 14:39     ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox