On 10/30/23 08:49, Wei6 Xu wrote:
> The MmCoreFfsFindMmDriver only checks for encapsulated compressed FVs.
> When an inner FV is uncompressed, StandaloneMmCore will miss the FV and
> all the MM drivers in the FV will not be dispatched.
> Add checks for uncompressed inner FV to fix this issue.
>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Sami Mujawar <sami.muja...@arm.com>
> Cc: Ray Ni <ray...@intel.com>
> Signed-off-by: Wei6 Xu <wei6...@intel.com>
> ---
> StandaloneMmPkg/Core/FwVol.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index fa335d62c252..783dbaf9b048 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -116,6 +116,25 @@ MmCoreFfsFindMmDriver (
> break;
> }
>
> + //
> + // Check uncompressed firmware volumes
> + //
> + Status = FfsFindSectionData (
> + EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
> + FileHeader,
> + &SectionData,
> + &SectionDataSize
> + );
> + if (!EFI_ERROR (Status)) {
> + if (SectionDataSize > sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
> + InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)SectionData;
> + MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
> + }
> + }
> +
> + //
> + // Check compressed firmware volumes
> + //
> Status = FfsFindSection (
> EFI_SECTION_GUID_DEFINED,
> FileHeader,
(1) In case we find an EFI_SECTION_FIRMWARE_VOLUME_IMAGE, do we still want to
look for an EFI_SECTION_GUID_DEFINED?
I think that's quite unlikely. If you agree, can you add a "continue" right
after the new MmCoreFfsFindMmDriver() call?
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
(2) We have a separate "InnerFvHeader" assignment, near the bottom of this
(first) loop in the function:
Status = FindFfsSectionInSections (
DstBuffer,
DstBufferSize,
EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
&Section
);
if (EFI_ERROR (Status)) {
goto FreeDstBuffer;
}
InnerFvHeader = (VOID *)(Section + 1);
Status = MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
That doesn't look right to me; what if Section in fact points to an
EFI_COMMON_SECTION_HEADER2?
FfsFindSectionData() handles this internally; it checks IS_SECTION2(), and then
adjusts SectionData accordingly:
if (IS_SECTION2 (Section)) {
*SectionData = (VOID *)((EFI_COMMON_SECTION_HEADER2 *)Section + 1);
...
} else {
*SectionData = (VOID *)(Section + 1);
...
}
I think we need to set InnerFvHeader similarly, here. Can you please append a
separate patch for fixing that?
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110316): https://edk2.groups.io/g/devel/message/110316
Mute This Topic: https://groups.io/mt/102270549/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
