Re: [edk2] [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

2019-03-06 Thread Achin Gupta
Reviewed-by: achin.gu...@arm.com

On Tue, Mar 05, 2019 at 02:32:44PM +0100, Ard Biesheuvel wrote:
> 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 
> ---
>  StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
>  StandaloneMmPkg/Core/FwVol.c  | 99 ++--
>  2 files changed, 91 insertions(+), 9 deletions(-)
>
> 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/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 
> +#include 
>
>  //
>  // 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, );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
> +   , );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> +Status = ExtractGuidedSectionGetInfo (Section, ,
> +   , );
> +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, , ScratchBuffer,
> +);
> +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, );
> +if (EFI_ERROR (Status)) {
> +  goto FreeDstBuffer;
> +}
> +
> +

Re: [edk2] [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

2019-03-05 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 06/10] 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 
> ---
>  StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
>  StandaloneMmPkg/Core/FwVol.c  | 99
> ++--
>  2 files changed, 91 insertions(+), 9 deletions(-)
> 
> 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/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 
> +#include 
> 
>  //
>  // 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, );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> FileHeader,
> +   , );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> +Status = ExtractGuidedSectionGetInfo (Section, ,
> +   , );
> +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, ,
> ScratchBuffer,
> +);
> +FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> +if (EFI_ERROR (Status)) {
> +  goto FreeDstBuffer;
> +}
> +
> +DEBUG ((DEBUG_INFO,
> +  

[edk2] [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

2019-03-05 Thread 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 
---
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
 StandaloneMmPkg/Core/FwVol.c  | 99 ++--
 2 files changed, 91 insertions(+), 9 deletions(-)

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/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 
+#include 
 
 //
 // 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, );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
+   , );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
+Status = ExtractGuidedSectionGetInfo (Section, ,
+   , );
+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, , ScratchBuffer,
+);
+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, );
+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 =