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.biesheu...@linaro.org] > Sent: Friday, January 18, 2019 7:42 AM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: edk2-devel@lists.01.org; Achin Gupta <achin.gu...@arm.com>; > Supreeth Venkatesh <supreeth.venkat...@arm.com>; Leif Lindholm > <leif.lindh...@linaro.org>; Jagadeesh Ujja <jagadeesh.u...@arm.com>; > Thomas Panakamattam Abraham <thomas.abra...@arm.com>; Sami > Mujawar <sami.muja...@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....@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.biesheu...@linaro.org] > > > Sent: Wednesday, January 16, 2019 12:23 PM > > > To: edk2-devel@lists.01.org > > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Achin Gupta > > > <achin.gu...@arm.com>; Yao, Jiewen <jiewen....@intel.com>; > Supreeth > > > Venkatesh <supreeth.venkat...@arm.com>; Leif Lindholm > > > <leif.lindh...@linaro.org>; Jagadeesh Ujja <jagadeesh.u...@arm.com>; > > > Thomas Panakamattam Abraham <thomas.abra...@arm.com>; Sami > > > Mujawar <sami.muja...@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.biesheu...@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 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
