public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [PATCH 1/2] MdeModulePkg PeiCore: Handle multiple FV images in one FV file
Date: Thu, 30 Aug 2018 10:02:14 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E2EB61E@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <1535535466-62348-2-git-send-email-star.zeng@intel.com>

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Zeng, Star
>Sent: Wednesday, August 29, 2018 5:38 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>;
>Yao, Jiewen <jiewen.yao@intel.com>
>Subject: [PATCH 1/2] MdeModulePkg PeiCore: Handle multiple FV images in
>one FV file
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1131
>
>PI spec and BaseTools supports to generate multiple FV images
>in one FV file.
>This patch is to update PeiCore to handle the case.
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Jiewen Yao <jiewen.yao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> MdeModulePkg/Core/Pei/FwVol/FwVol.c | 267 +++++++++++++++++++-----
>------------
> MdeModulePkg/Core/Pei/PeiMain.h     |   2 +-
> 2 files changed, 145 insertions(+), 124 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
>b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
>index 65c485549718..7ea503a10fb5 100644
>--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
>+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
>@@ -1358,7 +1358,7 @@ GetFvUsedSize (
> }
>
> /**
>-  Get Fv image from the FV type file, then install FV INFO(2) ppi, Build FV hob.
>+  Get Fv image(s) from the FV type file, then install FV INFO(2) ppi, Build FV(2,
>3) hob.
>
>   @param PrivateData          PeiCore's private data structure
>   @param ParentFvCoreHandle   Pointer of EFI_CORE_FV_HANDLE to parent
>Fv image that contain this Fv image.
>@@ -1391,6 +1391,7 @@ ProcessFvFile (
>   UINT32                        AuthenticationStatus;
>   UINT32                        FvUsedSize;
>   UINT8                         EraseByte;
>+  UINTN                         Index;
>
>   //
>   // Check if this EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE file has
>already
>@@ -1412,144 +1413,164 @@ ProcessFvFile (
>   ParentFvPpi    = ParentFvCoreHandle->FvPpi;
>
>   //
>-  // Find FvImage in FvFile
>+  // Find FvImage(s) in FvFile
>   //
>-  AuthenticationStatus = 0;
>-  if ((ParentFvPpi->Signature ==
>EFI_PEI_FIRMWARE_VOLUME_PPI_SIGNATURE) &&
>-      (ParentFvPpi->Revision == EFI_PEI_FIRMWARE_VOLUME_PPI_REVISION))
>{
>-    Status = ParentFvPpi->FindSectionByType2 (
>-                            ParentFvPpi,
>-                            EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>-                            0,
>-                            ParentFvFileHandle,
>-                            (VOID **)&FvHeader,
>-                            &AuthenticationStatus
>-                            );
>-  } else {
>-    Status = ParentFvPpi->FindSectionByType (
>-                            ParentFvPpi,
>-                            EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>-                            ParentFvFileHandle,
>-                            (VOID **)&FvHeader
>-                            );
>-  }
>-  if (EFI_ERROR (Status)) {
>-    return Status;
>-  }
>-
>-  Status = VerifyPeim (PrivateData, ParentFvHandle, ParentFvFileHandle,
>AuthenticationStatus);
>-  if (Status == EFI_SECURITY_VIOLATION) {
>-    return Status;
>-  }
>-
>-  //
>-  // If EFI_FVB2_WEAK_ALIGNMENT is set in the volume header then the first
>byte of the volume
>-  // can be aligned on any power-of-two boundary. A weakly aligned volume
>can not be moved from
>-  // its initial linked location and maintain its alignment.
>-  //
>-  if ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_WEAK_ALIGNMENT) != EFI_FVB2_WEAK_ALIGNMENT) {
>-    //
>-    // FvAlignment must be greater than or equal to 8 bytes of the minimum
>FFS alignment value.
>-    //
>-    FvAlignment = 1 << ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_ALIGNMENT) >> 16);
>-    if (FvAlignment < 8) {
>-      FvAlignment = 8;
>+  Index = 0;
>+  do {
>+    AuthenticationStatus = 0;
>+    if ((ParentFvPpi->Signature ==
>EFI_PEI_FIRMWARE_VOLUME_PPI_SIGNATURE) &&
>+        (ParentFvPpi->Revision ==
>EFI_PEI_FIRMWARE_VOLUME_PPI_REVISION)) {
>+      Status = ParentFvPpi->FindSectionByType2 (
>+                              ParentFvPpi,
>+                              EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>+                              Index,
>+                              ParentFvFileHandle,
>+                              (VOID **)&FvHeader,
>+                              &AuthenticationStatus
>+                              );
>+    } else {
>+      //
>+      // Old FvPpi has no parameter to input SearchInstance,
>+      // only one instance is supported.
>+      //
>+      if (Index > 0) {
>+        break;
>+      }
>+      Status = ParentFvPpi->FindSectionByType (
>+                              ParentFvPpi,
>+                              EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>+                              ParentFvFileHandle,
>+                              (VOID **)&FvHeader
>+                              );
>+    }
>+    if (EFI_ERROR (Status)) {
>+      break;
>     }
>
>-    DEBUG ((
>-      DEBUG_INFO,
>-      "%a() FV at 0x%x, FvAlignment required is 0x%x\n",
>-      __FUNCTION__,
>-      FvHeader,
>-      FvAlignment
>-      ));
>+    Status = VerifyPeim (PrivateData, ParentFvHandle, ParentFvFileHandle,
>AuthenticationStatus);
>+    if (Status == EFI_SECURITY_VIOLATION) {
>+      break;
>+    }
>
>     //
>-    // Check FvImage alignment.
>+    // If EFI_FVB2_WEAK_ALIGNMENT is set in the volume header then the
>first byte of the volume
>+    // can be aligned on any power-of-two boundary. A weakly aligned volume
>can not be moved from
>+    // its initial linked location and maintain its alignment.
>     //
>-    if ((UINTN) FvHeader % FvAlignment != 0) {
>-      FvLength    = ReadUnaligned64 (&FvHeader->FvLength);
>-      NewFvBuffer = AllocateAlignedPages (EFI_SIZE_TO_PAGES ((UINT32)
>FvLength), FvAlignment);
>-      if (NewFvBuffer == NULL) {
>-        return EFI_OUT_OF_RESOURCES;
>+    if ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_WEAK_ALIGNMENT) != EFI_FVB2_WEAK_ALIGNMENT) {
>+      //
>+      // FvAlignment must be greater than or equal to 8 bytes of the minimum
>FFS alignment value.
>+      //
>+      FvAlignment = 1 << ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_ALIGNMENT) >> 16);
>+      if (FvAlignment < 8) {
>+        FvAlignment = 8;
>       }
>-      if (GetFvUsedSize (FvHeader, &FvUsedSize, &EraseByte)) {
>-        //
>-        // Copy the used bytes and fill the rest with the erase value.
>-        //
>-        CopyMem (NewFvBuffer, FvHeader, (UINTN) FvUsedSize);
>-        SetMem (
>-          (UINT8 *) NewFvBuffer + FvUsedSize,
>-          (UINTN) (FvLength - FvUsedSize),
>-          EraseByte
>-          );
>-      } else {
>-        CopyMem (NewFvBuffer, FvHeader, (UINTN) FvLength);
>+
>+      DEBUG ((
>+        DEBUG_INFO,
>+        "%a() FV at 0x%x, FvAlignment required is 0x%x\n",
>+        __FUNCTION__,
>+        FvHeader,
>+        FvAlignment
>+        ));
>+
>+      //
>+      // Check FvImage alignment.
>+      //
>+      if ((UINTN) FvHeader % FvAlignment != 0) {
>+        FvLength    = ReadUnaligned64 (&FvHeader->FvLength);
>+        NewFvBuffer = AllocateAlignedPages (EFI_SIZE_TO_PAGES ((UINT32)
>FvLength), FvAlignment);
>+        if (NewFvBuffer == NULL) {
>+          Status = EFI_OUT_OF_RESOURCES;
>+          break;
>+        }
>+        if (GetFvUsedSize (FvHeader, &FvUsedSize, &EraseByte)) {
>+          //
>+          // Copy the used bytes and fill the rest with the erase value.
>+          //
>+          CopyMem (NewFvBuffer, FvHeader, (UINTN) FvUsedSize);
>+          SetMem (
>+            (UINT8 *) NewFvBuffer + FvUsedSize,
>+            (UINTN) (FvLength - FvUsedSize),
>+            EraseByte
>+            );
>+        } else {
>+          CopyMem (NewFvBuffer, FvHeader, (UINTN) FvLength);
>+        }
>+        FvHeader = (EFI_FIRMWARE_VOLUME_HEADER*) NewFvBuffer;
>       }
>-      FvHeader = (EFI_FIRMWARE_VOLUME_HEADER*) NewFvBuffer;
>     }
>-  }
>
>-  Status = ParentFvPpi->GetVolumeInfo (ParentFvPpi, ParentFvHandle,
>&ParentFvImageInfo);
>-  ASSERT_EFI_ERROR (Status);
>+    Status = ParentFvPpi->GetVolumeInfo (ParentFvPpi, ParentFvHandle,
>&ParentFvImageInfo);
>+    ASSERT_EFI_ERROR (Status);
>
>-  Status = ParentFvPpi->GetFileInfo (ParentFvPpi, ParentFvFileHandle,
>&FileInfo);
>-  ASSERT_EFI_ERROR (Status);
>-
>-  //
>-  // Install FvInfo(2) Ppi
>-  // NOTE: FvInfo2 must be installed before FvInfo so that recursive
>processing of encapsulated
>-  // FVs inherit the proper AuthenticationStatus.
>-  //
>-  PeiServicesInstallFvInfo2Ppi(
>-    &FvHeader->FileSystemGuid,
>-    (VOID**)FvHeader,
>-    (UINT32)FvHeader->FvLength,
>-    &ParentFvImageInfo.FvName,
>-    &FileInfo.FileName,
>-    AuthenticationStatus
>-    );
>+    Status = ParentFvPpi->GetFileInfo (ParentFvPpi, ParentFvFileHandle,
>&FileInfo);
>+    ASSERT_EFI_ERROR (Status);
>
>-  PeiServicesInstallFvInfoPpi (
>-    &FvHeader->FileSystemGuid,
>-    (VOID**) FvHeader,
>-    (UINT32) FvHeader->FvLength,
>-    &ParentFvImageInfo.FvName,
>-    &FileInfo.FileName
>-    );
>+    //
>+    // Install FvInfo(2) Ppi
>+    // NOTE: FvInfo2 must be installed before FvInfo so that recursive
>processing of encapsulated
>+    // FVs inherit the proper AuthenticationStatus.
>+    //
>+    PeiServicesInstallFvInfo2Ppi(
>+      &FvHeader->FileSystemGuid,
>+      (VOID**)FvHeader,
>+      (UINT32)FvHeader->FvLength,
>+      &ParentFvImageInfo.FvName,
>+      &FileInfo.FileName,
>+      AuthenticationStatus
>+      );
>+
>+    PeiServicesInstallFvInfoPpi (
>+      &FvHeader->FileSystemGuid,
>+      (VOID**) FvHeader,
>+      (UINT32) FvHeader->FvLength,
>+      &ParentFvImageInfo.FvName,
>+      &FileInfo.FileName
>+      );
>
>-  //
>-  // Inform the extracted FvImage to Fv HOB consumer phase, i.e. DXE phase
>-  //
>-  BuildFvHob (
>-    (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>-    FvHeader->FvLength
>-    );
>+    //
>+    // Inform the extracted FvImage to Fv HOB consumer phase, i.e. DXE
>phase
>+    //
>+    BuildFvHob (
>+      (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>+      FvHeader->FvLength
>+      );
>
>-  //
>-  // Makes the encapsulated volume show up in DXE phase to skip processing
>of
>-  // encapsulated file again.
>-  //
>-  BuildFv2Hob (
>-    (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>-    FvHeader->FvLength,
>-    &ParentFvImageInfo.FvName,
>-    &FileInfo.FileName
>-    );
>+    //
>+    // Makes the encapsulated volume show up in DXE phase to skip
>processing of
>+    // encapsulated file again.
>+    //
>+    BuildFv2Hob (
>+      (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>+      FvHeader->FvLength,
>+      &ParentFvImageInfo.FvName,
>+      &FileInfo.FileName
>+      );
>
>-  //
>-  // Build FV3 HOB with authentication status to be propagated to DXE.
>-  //
>-  BuildFv3Hob (
>-    (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>-    FvHeader->FvLength,
>-    AuthenticationStatus,
>-    TRUE,
>-    &ParentFvImageInfo.FvName,
>-    &FileInfo.FileName
>-    );
>+    //
>+    // Build FV3 HOB with authentication status to be propagated to DXE.
>+    //
>+    BuildFv3Hob (
>+      (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>+      FvHeader->FvLength,
>+      AuthenticationStatus,
>+      TRUE,
>+      &ParentFvImageInfo.FvName,
>+      &FileInfo.FileName
>+      );
>+
>+    Index++;
>+  } while (TRUE);
>
>-  return EFI_SUCCESS;
>+  if (Index > 0) {
>+    //
>+    // At least one FvImage has been processed successfully.
>+    //
>+    return EFI_SUCCESS;
>+  } else {
>+    return Status;
>+  }
> }
>
> /**
>diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
>b/MdeModulePkg/Core/Pei/PeiMain.h
>index e2f8cd9c7758..6469436b8f79 100644
>--- a/MdeModulePkg/Core/Pei/PeiMain.h
>+++ b/MdeModulePkg/Core/Pei/PeiMain.h
>@@ -1258,7 +1258,7 @@ SecurityPpiNotifyCallback (
>   );
>
> /**
>-  Get Fv image from the FV type file, then install FV INFO(2) ppi, Build FV hob.
>+  Get Fv image(s) from the FV type file, then install FV INFO(2) ppi, Build FV(2,
>3) hob.
>
>   @param PrivateData          PeiCore's private data structure
>   @param ParentFvCoreHandle   Pointer of EFI_CORE_FV_HANDLE to parent
>Fv image that contain this Fv image.
>--
>2.7.0.windows.1



  reply	other threads:[~2018-08-30 10:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29  9:37 [PATCH 0/2] Core: Handle multiple FV images in one FV file Star Zeng
2018-08-29  9:37 ` [PATCH 1/2] MdeModulePkg PeiCore: " Star Zeng
2018-08-30 10:02   ` Gao, Liming [this message]
2018-08-29  9:37 ` [PATCH 2/2] MdeModulePkg DxeCore: " Star Zeng
2018-08-30  9:54   ` Gao, Liming
2018-08-30  9:59     ` Zeng, Star

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=4A89E2EF3DFEDB4C8BFDE51014F606A14E2EB61E@SHSMSX104.ccr.corp.intel.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