From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.131084.1671105800038965632 for ; Thu, 15 Dec 2022 04:03:20 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=FSdRfqof; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A21D9B81B8E for ; Thu, 15 Dec 2022 12:03:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 206F0C43396 for ; Thu, 15 Dec 2022 12:03:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1671105795; bh=lg33Ot9VmQY9TzR9wmpKUg5ashKwMMsd0Mpw14e9d1k=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=FSdRfqoffu3iJEeWddOEMCKguBIft6uFnryTVrqlDdlY33nsbAIBxSn+99v0OKel9 18xuDk6PvwD27ZPfMVCKmqiUAtJjIHg1OtK29xautHY+TQdBgA6p6w8bEQI/Ce1Gu4 /Lf3zDZlb60cS+XOf34dunPjq+PXuLCBe9+cjlklgbJBfgqT2Mp7OCCes2v+8caPmX wXi/6XribVk4UpyVWhB+Y5Z4+2/0FYOp9DlpCSKpUAFMnoaxfdnIw32cQtlyQinuUL 54itk6fWlBEK4rZSroRxo9FlrkaTRGoyIWsDybQ3F2I2njsGk2cndwug/OAtLjppJe yW4RypcRIjCfw== Received: by mail-lf1-f45.google.com with SMTP id c1so15309337lfi.7 for ; Thu, 15 Dec 2022 04:03:14 -0800 (PST) X-Gm-Message-State: AFqh2kqSAGa2rJeGVPbU8LoPm/M7MCncwSrwkIjHEUAhStiRaTLgzaAF 5/4sZtXECAfzCRnKg1NadidBXHObisYkn7fN2iI= X-Google-Smtp-Source: AMrXdXtaQq8VvastynVVVoqZwL9HbiaLXjbuAKj+kjoi0fkjt7zP13ajFs4YfIvLcZe4W2M8bxCjsVhvY9yRx08bSvM= X-Received: by 2002:a05:6512:3718:b0:4b8:9001:a694 with SMTP id z24-20020a056512371800b004b89001a694mr379102lfr.426.1671105792990; Thu, 15 Dec 2022 04:03:12 -0800 (PST) MIME-Version: 1.0 References: <20221213062409.932-1-min.m.xu@intel.com> <20221213062409.932-2-min.m.xu@intel.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 15 Dec 2022 13:03:01 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH V2 1/4] EmbeddedPkg/PrePiLib: Add FFS_CHECK_SECTION_HOOK when finding section To: "Xu, Min M" Cc: "devel@edk2.groups.io" , Leif Lindholm , Ard Biesheuvel , Abner Chang , Daniel Schaefer , Gerd Hoffmann , "Aktas, Erdem" , James Bottomley , "Yao, Jiewen" , Tom Lendacky Content-Type: text/plain; charset="UTF-8" On Thu, 15 Dec 2022 at 04:26, Xu, Min M wrote: > > Hi, Leif/Ard/Abner/Daniel > Since you're the maintainer/reviewer of EmbeddedPkg, would you please help to review this patch? Any comments is welcome. > I have no objections to this Acked-by: Ard Biesheuvel > > > -----Original Message----- > > From: Xu, Min M > > Sent: Tuesday, December 13, 2022 2:24 PM > > To: devel@edk2.groups.io > > Cc: Xu, Min M ; Leif Lindholm > > ; Ard Biesheuvel ; > > Abner Chang ; Daniel Schaefer > > ; Gerd Hoffmann ; Aktas, > > Erdem ; James Bottomley ; > > Yao, Jiewen ; Tom Lendacky > > > > Subject: [PATCH V2 1/4] EmbeddedPkg/PrePiLib: Add > > FFS_CHECK_SECTION_HOOK when finding section > > > > From: Min M Xu > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4152 > > > > EmbeddedPkg/PrePiLib provides the service of finding sections based on the > > input SectionType. But sometimes there maybe multiple sections with the > > same SectionType. FFS_CHECK_SECTION_HOOK is a hook which can be called > > to do additional check. > > > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Cc: Abner Chang > > Cc: Daniel Schaefer > > Cc: Gerd Hoffmann > > Cc: Erdem Aktas > > Cc: James Bottomley > > Cc: Jiewen Yao > > Cc: Tom Lendacky > > Signed-off-by: Min Xu > > > > EmbeddedPkg > > --- > > EmbeddedPkg/Include/Library/PrePiLib.h | 23 +++++++++++--- > > EmbeddedPkg/Library/PrePiLib/FwVol.c | 42 ++++++++++++++++++------- > > EmbeddedPkg/Library/PrePiLib/PrePiLib.c | 2 +- > > 3 files changed, 49 insertions(+), 18 deletions(-) > > > > diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h > > b/EmbeddedPkg/Include/Library/PrePiLib.h > > index 3741b08c4478..f60b6678185a 100644 > > --- a/EmbeddedPkg/Include/Library/PrePiLib.h > > +++ b/EmbeddedPkg/Include/Library/PrePiLib.h > > @@ -52,11 +52,23 @@ FfsFindNextFile ( > > IN OUT EFI_PEI_FILE_HANDLE *FileHandle > > ); > > > > +/** > > + * This is a hook which is used to check if the section is the target one. > > + * > > + */ > > +typedef > > +EFI_STATUS > > +(EFIAPI *FFS_CHECK_SECTION_HOOK)( > > + IN EFI_COMMON_SECTION_HEADER *Section > > + ); > > + > > /** > > This service enables discovery sections of a given type within a valid FFS file. > > + Caller also can provide a SectionCheckHook to do additional checking. > > > > - @param SearchType The value of the section type to find. > > - @param FfsFileHeader A pointer to the file header that contains the > > set of sections to > > + @param SectionType The value of the section type to find. > > + @param SectionCheckHook A hook which can check if the section is the > > target one. > > + @param FileHeader A pointer to the file header that contains the set > > of sections to > > be searched. > > @param SectionData A pointer to the discovered section, if successful. > > > > @@ -67,9 +79,10 @@ FfsFindNextFile ( > > EFI_STATUS > > EFIAPI > > FfsFindSectionData ( > > - IN EFI_SECTION_TYPE SectionType, > > - IN EFI_PEI_FILE_HANDLE FileHandle, > > - OUT VOID **SectionData > > + IN EFI_SECTION_TYPE SectionType, > > + IN FFS_CHECK_SECTION_HOOK SectionCheckHook, > > + IN EFI_PEI_FILE_HANDLE FileHandle, > > + OUT VOID **SectionData > > ); > > > > /** > > diff --git a/EmbeddedPkg/Library/PrePiLib/FwVol.c > > b/EmbeddedPkg/Library/PrePiLib/FwVol.c > > index 0a6d6925b7ea..778d8b13c33b 100644 > > --- a/EmbeddedPkg/Library/PrePiLib/FwVol.c > > +++ b/EmbeddedPkg/Library/PrePiLib/FwVol.c > > @@ -264,16 +264,18 @@ FindFileEx ( > > Go through the file to search SectionType section, > > when meeting an encapsuled section. > > > > - @param SectionType - Filter to find only section of this type. > > - @param Section - From where to search. > > - @param SectionSize - The file size to search. > > - @param OutputBuffer - Pointer to the section to search. > > + @param SectionType - Filter to find only section of this type. > > + @param SectionCheckHook - A hook which can check if the section is the > > target one. > > + @param Section - From where to search. > > + @param SectionSize - The file size to search. > > + @param OutputBuffer - Pointer to the section to search. > > > > @retval EFI_SUCCESS > > **/ > > EFI_STATUS > > FfsProcessSection ( > > IN EFI_SECTION_TYPE SectionType, > > + IN FFS_CHECK_SECTION_HOOK SectionCheckHook, > > IN EFI_COMMON_SECTION_HEADER *Section, > > IN UINTN SectionSize, > > OUT VOID **OutputBuffer > > @@ -292,7 +294,9 @@ FfsProcessSection ( > > UINT32 AuthenticationStatus; > > CHAR8 *CompressedData; > > UINT32 CompressedDataLength; > > + BOOLEAN Found; > > > > + Found = FALSE; > > *OutputBuffer = NULL; > > ParsedLength = 0; > > Status = EFI_NOT_FOUND; > > @@ -302,13 +306,23 @@ FfsProcessSection ( > > } > > > > if (Section->Type == SectionType) { > > - if (IS_SECTION2 (Section)) { > > - *OutputBuffer = (VOID *)((UINT8 *)Section + sizeof > > (EFI_COMMON_SECTION_HEADER2)); > > + if (SectionCheckHook != NULL) { > > + Found = SectionCheckHook (Section) == EFI_SUCCESS; > > } else { > > - *OutputBuffer = (VOID *)((UINT8 *)Section + sizeof > > (EFI_COMMON_SECTION_HEADER)); > > + Found = TRUE; > > } > > > > - return EFI_SUCCESS; > > + if (Found) { > > + if (IS_SECTION2 (Section)) { > > + *OutputBuffer = (VOID *)((UINT8 *)Section + sizeof > > (EFI_COMMON_SECTION_HEADER2)); > > + } else { > > + *OutputBuffer = (VOID *)((UINT8 *)Section + sizeof > > (EFI_COMMON_SECTION_HEADER)); > > + } > > + > > + return EFI_SUCCESS; > > + } else { > > + goto CheckNextSection; > > + } > > } else if ((Section->Type == EFI_SECTION_COMPRESSION) || (Section- > > >Type == EFI_SECTION_GUID_DEFINED)) { > > if (Section->Type == EFI_SECTION_COMPRESSION) { > > if (IS_SECTION2 (Section)) { > > @@ -415,6 +429,7 @@ FfsProcessSection ( > > } else { > > return FfsProcessSection ( > > SectionType, > > + SectionCheckHook, > > DstBuffer, > > DstBufferSize, > > OutputBuffer > > @@ -422,6 +437,7 @@ FfsProcessSection ( > > } > > } > > > > +CheckNextSection: > > if (IS_SECTION2 (Section)) { > > SectionLength = SECTION2_SIZE (Section); > > } else { > > @@ -456,9 +472,10 @@ FfsProcessSection ( EFI_STATUS EFIAPI > > FfsFindSectionData ( > > - IN EFI_SECTION_TYPE SectionType, > > - IN EFI_PEI_FILE_HANDLE FileHandle, > > - OUT VOID **SectionData > > + IN EFI_SECTION_TYPE SectionType, > > + IN FFS_CHECK_SECTION_HOOK SectionCheckHook, > > + IN EFI_PEI_FILE_HANDLE FileHandle, > > + OUT VOID **SectionData > > ) > > { > > EFI_FFS_FILE_HEADER *FfsFileHeader; > > @@ -478,6 +495,7 @@ FfsFindSectionData ( > > > > return FfsProcessSection ( > > SectionType, > > + SectionCheckHook, > > Section, > > FileSize, > > SectionData > > @@ -799,7 +817,7 @@ FfsProcessFvFile ( > > // > > // Find FvImage in FvFile > > // > > - Status = FfsFindSectionData (EFI_SECTION_FIRMWARE_VOLUME_IMAGE, > > FvFileHandle, (VOID **)&FvImageHandle); > > + Status = FfsFindSectionData (EFI_SECTION_FIRMWARE_VOLUME_IMAGE, > > NULL, > > + FvFileHandle, (VOID **)&FvImageHandle); > > if (EFI_ERROR (Status)) { > > return Status; > > } > > diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c > > b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c > > index a0c5d02debd0..3b6fc4f0eba8 100644 > > --- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c > > +++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c > > @@ -131,7 +131,7 @@ LoadDxeCoreFromFfsFile ( > > VOID *Hob; > > EFI_FV_FILE_INFO FvFileInfo; > > > > - Status = FfsFindSectionData (EFI_SECTION_PE32, FileHandle, > > &PeCoffImage); > > + Status = FfsFindSectionData (EFI_SECTION_PE32, NULL, FileHandle, > > + &PeCoffImage); > > if (EFI_ERROR (Status)) { > > return Status; > > } > > -- > > 2.29.2.windows.2 >