From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 438CB21B02822 for ; Fri, 18 Jan 2019 07:42:04 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id i145so7255770ita.4 for ; Fri, 18 Jan 2019 07:42:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xhVBWPAz0HcoxbDj/nLdDcJqmu1URn88kV3Vi2YRCdg=; b=Or0S3fFtu/5aj8p9oAAnghzEe1aK4NUndaTvNK5hZKRbMEGQM3mQJ2l6pxzywnxxWE AYdFqCLHGcq2PbjlnHXq74ov9pmZvuCOKCwd6kWXAhXkveXaxoEAw9NueAZRPeXR4Axb /67bHmthhWA9Z6MoNNoZlZ5Xz7rCozlL/peNA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xhVBWPAz0HcoxbDj/nLdDcJqmu1URn88kV3Vi2YRCdg=; b=ZQxgA9KeTr2LwjxE7zVF5AJfLpGNJf8jRl9U4lP0p2WYXdUvaMpLqlTr5y1h0dEiq6 FKVELaq5JLKGlBc7jhuqhaeLZBzaQJ7XU8Ot9+PbYjkn5KQI2FZ2IZH1SWdbj3ysq0k5 dWgRZrfMwR1kBHl6733K3mBlzpvtnI5icICKSgmaMVXHyS+x45JaTtOg9B64vffvA2bX XgeaaC5Cl7fbjpq41Ho/WQBsSlAl9Eb9vjsqT2heKddQRck7cKQyeJwKsp80CHBb9wfF 9/z0S6oUCsnxxBKVLOnP3cfFu5WUsXEsgmOX10HjrYfqef9Vz5SjcaO6ym2X7Wli+YWm K+AA== X-Gm-Message-State: AJcUukcEL70/8RQXti7Ks7Fbe9Od06UP6/7YyeoNCvICBU/Wstn5W5L2 Gm8IgAl7soGcJCpVOx2FZZUzO/tuGZ/TU8ZIoFKitA== X-Google-Smtp-Source: ALg8bN6vt/s7S1x83Md1i4+dCQ6xpYXIJq3HiUpU7Q4QNL/DvoS06YFyk0OTumBYU5RvW1901GJKT1sfHiePqM0NV1s= X-Received: by 2002:a02:183:: with SMTP id 3mr11159833jak.130.1547826124244; Fri, 18 Jan 2019 07:42:04 -0800 (PST) MIME-Version: 1.0 References: <20190116202236.6977-1-ard.biesheuvel@linaro.org> <20190116202236.6977-12-ard.biesheuvel@linaro.org> <74D8A39837DF1E4DA445A8C0B3885C503F4B1A3C@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F4B1A3C@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 18 Jan 2019 16:41:53 +0100 Message-ID: To: "Yao, Jiewen" Cc: "edk2-devel@lists.01.org" , Achin Gupta , Supreeth Venkatesh , Leif Lindholm , Jagadeesh Ujja , Thomas Panakamattam Abraham , Sami Mujawar Subject: Re: [PATCH v2 11/11] StandaloneMmPkg/Core: permit encapsulated firmware volumes X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jan 2019 15:42:05 -0000 Content-Type: text/plain; charset="UTF-8" On Fri, 18 Jan 2019 at 16:40, Yao, Jiewen 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 ; Achin Gupta > > ; Yao, Jiewen ; Supreeth > > Venkatesh ; Leif Lindholm > > ; Jagadeesh Ujja ; > > Thomas Panakamattam Abraham ; Sami > > Mujawar > > 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 > > --- > > 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 > > +#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, &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 >