From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "edk2-devel@lists.01.org" <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
Date: Fri, 18 Jan 2019 16:41:53 +0100 [thread overview]
Message-ID: <CAKv+Gu8moQLHVSzS735sjyw4Q3cbL7gszpkSrj7HuHCFqGzetA@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F4B1A3C@shsmsx102.ccr.corp.intel.com>
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
>
next prev parent reply other threads:[~2019-01-18 15:42 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu8moQLHVSzS735sjyw4Q3cbL7gszpkSrj7HuHCFqGzetA@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox