Re: [PATCH 2/2] MdeModulePkg DxeCore: Handle multiple FV images in one FV file

["Gao, Liming" <liming.gao@intel.com>]

Star:
  You change the parameter name in the function. Please also update the function description to match the function implementation. 

Thanks
Liming
>-----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 2/2] MdeModulePkg DxeCore: 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 DxeCore 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/Dxe/Dispatcher/Dispatcher.c | 241 ++++++++++++++-
>-----------
> 1 file changed, 128 insertions(+), 113 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c
>b/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c
>index f72c47a9e54c..086a590ef4e3 100644
>--- a/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c
>+++ b/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c
>@@ -184,14 +184,13 @@ CoreAddToDriverList (
>   );
>
> /**
>-  Get the driver from the FV through driver name, and produce a FVB
>protocol on FvHandle.
>+  Get Fv image(s) from the FV through file name, and produce FVB protocol
>for every Fv image(s).
>
>   @param  Fv                    The FIRMWARE_VOLUME protocol installed on the FV.
>   @param  FvHandle              The handle which FVB protocol installed on.
>   @param  DriverName            The driver guid specified.
>
>   @retval EFI_OUT_OF_RESOURCES  No enough memory or other resource.
>-  @retval EFI_VOLUME_CORRUPTED  Corrupted volume.
>   @retval EFI_SUCCESS           Function successfully returned.
>
> **/
>@@ -199,7 +198,7 @@ EFI_STATUS
> CoreProcessFvImageFile (
>   IN  EFI_FIRMWARE_VOLUME2_PROTOCOL   *Fv,
>   IN  EFI_HANDLE                      FvHandle,
>-  IN  EFI_GUID                        *DriverName
>+  IN  EFI_GUID                        *FileName
>   );
>
>
>@@ -1004,14 +1003,13 @@ GetFvUsedSize (
> }
>
> /**
>-  Get the driver from the FV through driver name, and produce a FVB
>protocol on FvHandle.
>+  Get Fv image(s) from the FV through file name, and produce FVB protocol
>for every Fv image(s).
>
>   @param  Fv                    The FIRMWARE_VOLUME protocol installed on the FV.
>   @param  FvHandle              The handle which FVB protocol installed on.
>   @param  DriverName            The driver guid specified.
>
>   @retval EFI_OUT_OF_RESOURCES  No enough memory or other resource.
>-  @retval EFI_VOLUME_CORRUPTED  Corrupted volume.
>   @retval EFI_SUCCESS           Function successfully returned.
>
> **/
>@@ -1019,7 +1017,7 @@ EFI_STATUS
> CoreProcessFvImageFile (
>   IN  EFI_FIRMWARE_VOLUME2_PROTOCOL   *Fv,
>   IN  EFI_HANDLE                      FvHandle,
>-  IN  EFI_GUID                        *DriverName
>+  IN  EFI_GUID                        *FileName
>   )
> {
>   EFI_STATUS                          Status;
>@@ -1033,141 +1031,158 @@ CoreProcessFvImageFile (
>   EFI_DEVICE_PATH_PROTOCOL            *FvFileDevicePath;
>   UINT32                              FvUsedSize;
>   UINT8                               EraseByte;
>+  UINTN                               Index;
>
>   //
>-  // Read the first (and only the first) firmware volume section
>+  // Read firmware volume section(s)
>   //
>   SectionType   = EFI_SECTION_FIRMWARE_VOLUME_IMAGE;
>-  FvHeader      = NULL;
>-  FvAlignment   = 0;
>-  Buffer        = NULL;
>-  BufferSize    = 0;
>-  AlignedBuffer = NULL;
>-  Status = Fv->ReadSection (
>-                 Fv,
>-                 DriverName,
>-                 SectionType,
>-                 0,
>-                 &Buffer,
>-                 &BufferSize,
>-                 &AuthenticationStatus
>-                 );
>-  if (!EFI_ERROR (Status)) {
>-     //
>-    // Evaluate the authentication status of the Firmware Volume through
>-    // Security Architectural Protocol
>-    //
>-    if (gSecurity != NULL) {
>-      FvFileDevicePath = CoreFvToDevicePath (Fv, FvHandle, DriverName);
>-      Status = gSecurity->FileAuthenticationState (
>-                            gSecurity,
>-                            AuthenticationStatus,
>-                            FvFileDevicePath
>-                            );
>-      if (FvFileDevicePath != NULL) {
>-        FreePool (FvFileDevicePath);
>-      }
>
>-      if (Status != EFI_SUCCESS) {
>-        //
>-        // Security check failed. The firmware volume should not be used for any
>purpose.
>-        //
>-        if (Buffer != NULL) {
>-          FreePool (Buffer);
>+  Index = 0;
>+  do {
>+    FvHeader      = NULL;
>+    FvAlignment   = 0;
>+    Buffer        = NULL;
>+    BufferSize    = 0;
>+    AlignedBuffer = NULL;
>+    Status = Fv->ReadSection (
>+                   Fv,
>+                   FileName,
>+                   SectionType,
>+                   Index,
>+                   &Buffer,
>+                   &BufferSize,
>+                   &AuthenticationStatus
>+                   );
>+    if (!EFI_ERROR (Status)) {
>+       //
>+      // Evaluate the authentication status of the Firmware Volume through
>+      // Security Architectural Protocol
>+      //
>+      if (gSecurity != NULL) {
>+        FvFileDevicePath = CoreFvToDevicePath (Fv, FvHandle, FileName);
>+        Status = gSecurity->FileAuthenticationState (
>+                              gSecurity,
>+                              AuthenticationStatus,
>+                              FvFileDevicePath
>+                              );
>+        if (FvFileDevicePath != NULL) {
>+          FreePool (FvFileDevicePath);
>+        }
>+
>+        if (Status != EFI_SUCCESS) {
>+          //
>+          // Security check failed. The firmware volume should not be used for
>any purpose.
>+          //
>+          if (Buffer != NULL) {
>+            FreePool (Buffer);
>+          }
>+          break;
>         }
>-        return Status;
>       }
>-    }
>
>-    //
>-    // FvImage should be at its required alignment.
>-    //
>-    FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) Buffer;
>-    //
>-    // 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) {
>       //
>-      // Get FvHeader alignment
>+      // FvImage should be at its required alignment.
>       //
>-      FvAlignment = 1 << ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_ALIGNMENT) >> 16);
>+      FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) Buffer;
>       //
>-      // FvAlignment must be greater than or equal to 8 bytes of the minimum
>FFS alignment value.
>+      // 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 (FvAlignment < 8) {
>-        FvAlignment = 8;
>-      }
>+      if ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_WEAK_ALIGNMENT) != EFI_FVB2_WEAK_ALIGNMENT) {
>+        //
>+        // Get FvHeader alignment
>+        //
>+        FvAlignment = 1 << ((ReadUnaligned32 (&FvHeader->Attributes) &
>EFI_FVB2_ALIGNMENT) >> 16);
>+        //
>+        // FvAlignment must be greater than or equal to 8 bytes of the minimum
>FFS alignment value.
>+        //
>+        if (FvAlignment < 8) {
>+          FvAlignment = 8;
>+        }
>
>-      DEBUG ((
>-        DEBUG_INFO,
>-        "%a() FV at 0x%x, FvAlignment required is 0x%x\n",
>-        __FUNCTION__,
>-        FvHeader,
>-        FvAlignment
>-        ));
>+        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) {
>         //
>-        // Allocate the aligned buffer for the FvImage.
>+        // Check FvImage alignment.
>         //
>-        AlignedBuffer = AllocateAlignedPages (EFI_SIZE_TO_PAGES (BufferSize),
>(UINTN) FvAlignment);
>-        if (AlignedBuffer == NULL) {
>-          FreePool (Buffer);
>-          return EFI_OUT_OF_RESOURCES;
>-        } else {
>+        if ((UINTN) FvHeader % FvAlignment != 0) {
>           //
>-          // Move FvImage into the aligned buffer and release the original buffer.
>+          // Allocate the aligned buffer for the FvImage.
>           //
>-          if (GetFvUsedSize (FvHeader, &FvUsedSize, &EraseByte)) {
>+          AlignedBuffer = AllocateAlignedPages (EFI_SIZE_TO_PAGES
>(BufferSize), (UINTN) FvAlignment);
>+          if (AlignedBuffer == NULL) {
>+            FreePool (Buffer);
>+            Status = EFI_OUT_OF_RESOURCES;
>+            break;
>+          } else {
>             //
>-            // Copy the used bytes and fill the rest with the erase value.
>+            // Move FvImage into the aligned buffer and release the original
>buffer.
>             //
>-            CopyMem (AlignedBuffer, FvHeader, (UINTN) FvUsedSize);
>-            SetMem (
>-              (UINT8 *) AlignedBuffer + FvUsedSize,
>-              (UINTN) (BufferSize - FvUsedSize),
>-              EraseByte
>-              );
>-          } else {
>-            CopyMem (AlignedBuffer, Buffer, BufferSize);
>+            if (GetFvUsedSize (FvHeader, &FvUsedSize, &EraseByte)) {
>+              //
>+              // Copy the used bytes and fill the rest with the erase value.
>+              //
>+              CopyMem (AlignedBuffer, FvHeader, (UINTN) FvUsedSize);
>+              SetMem (
>+                (UINT8 *) AlignedBuffer + FvUsedSize,
>+                (UINTN) (BufferSize - FvUsedSize),
>+                EraseByte
>+                );
>+            } else {
>+              CopyMem (AlignedBuffer, Buffer, BufferSize);
>+            }
>+            FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) AlignedBuffer;
>+            FreePool (Buffer);
>+            Buffer = NULL;
>           }
>-          FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) AlignedBuffer;
>-          CoreFreePool (Buffer);
>-          Buffer = NULL;
>         }
>       }
>+      //
>+      // Produce a FVB protocol for the file
>+      //
>+      Status = ProduceFVBProtocolOnBuffer (
>+                (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>+                (UINT64)BufferSize,
>+                FvHandle,
>+                AuthenticationStatus,
>+                NULL
>+                );
>     }
>-    //
>-    // Produce a FVB protocol for the file
>-    //
>-    Status = ProduceFVBProtocolOnBuffer (
>-              (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
>-              (UINT64)BufferSize,
>-              FvHandle,
>-              AuthenticationStatus,
>-              NULL
>-              );
>-  }
>
>-  if (EFI_ERROR (Status)) {
>-    //
>-    // ReadSection or Produce FVB failed, Free data buffer
>-    //
>-    if (Buffer != NULL) {
>-      FreePool (Buffer);
>-    }
>+    if (EFI_ERROR (Status)) {
>+      //
>+      // ReadSection or Produce FVB failed, Free data buffer
>+      //
>+      if (Buffer != NULL) {
>+        FreePool (Buffer);
>+      }
>+
>+      if (AlignedBuffer != NULL) {
>+        FreeAlignedPages (AlignedBuffer, EFI_SIZE_TO_PAGES (BufferSize));
>+      }
>
>-    if (AlignedBuffer != NULL) {
>-      FreeAlignedPages (AlignedBuffer, EFI_SIZE_TO_PAGES (BufferSize));
>+      break;
>+    } else {
>+      Index++;
>     }
>-  }
>+  } while (TRUE);
>
>-  return Status;
>+  if (Index > 0) {
>+    //
>+    // At least one FvImage has been processed successfully.
>+    //
>+    return EFI_SUCCESS;
>+  } else {
>+    return Status;
>+  }
> }
>
>
>--
>2.7.0.windows.1