[PATCH 0/8] StandaloneMmPkg: assorted fixes and improvements

[PATCH 0/8] StandaloneMmPkg: assorted fixes and improvements

[Ard Biesheuvel]

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.

The first six patches are obvious fixes/improvements.

Patch #7 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 #8 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.

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 (8):
  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 +
 .../StandaloneMmPeCoffExtraActionLib.c        |   9 +-
 11 files changed, 157 insertions(+), 122 deletions(-)

-- 
2.17.1

[PATCH 1/8] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone

[Ard Biesheuvel]

Fix a couple of occurrences of typo Standlone -> Standalone. Since
_PiMmStandloneArmTfCpuDriverEntry() is never referenced, drop it
altogether.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 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

[PATCH 2/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref

[Ard Biesheuvel]

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>
---
 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

[PATCH 3/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings

[Ard Biesheuvel]

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>
---
 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

[PATCH 4/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s

[Ard Biesheuvel]

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>
---
 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

[PATCH 5/8] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute

[Ard Biesheuvel]

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

[PATCH 6/8] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice

[Ard Biesheuvel]

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

[PATCH 7/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images

[Ard Biesheuvel]

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

[PATCH 8/8] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Ard Biesheuvel]

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..d95491f252f9 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, extra one page for adjustment
+    //
+    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 = (VOID *)(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

Re: [PATCH 1/8] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone

[Supreeth Venkatesh]

On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> Fix a couple of occurrences of typo Standlone -> Standalone. Since
> _PiMmStandloneArmTfCpuDriverEntry() is never referenced, drop it
> altogether.
> 
> 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

Re: [PATCH 2/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref

[Supreeth Venkatesh]

On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> 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/StandaloneMmCoreE
> ntryPoint.inf | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> index 3222cd359f3e..769eaeeefbea 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> @@ -43,6 +43,7 @@ [Packages.AARCH64]
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> +  SerialPortLib
>  
>  [LibraryClasses.AARCH64]
>    StandaloneMmMmuLib

Re: [PATCH 3/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings

[Supreeth Venkatesh]

On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> 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/SetPermis
> sions.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> index 60c1f66b83fa..3ca7f6660f47 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.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",

Re: [PATCH 4/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s

[Supreeth Venkatesh]

On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> 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/Standalon
> eMmCoreEntryPoint.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> index 05ed6c8dd0b5..5cca532456fd 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.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);
>  }