REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614

When we allocate pool to save the rebased PEIMs, the address will change
randomly, therefore the hash will change and result PCR0 change as well.
To avoid this, we save the raw PEIMs and use it to calculate hash.

The MigratedFvInfo HOB will never produce when
PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD control
the total feature.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec                 |  3 ++
 MdeModulePkg/Core/Pei/PeiMain.inf             |  1 +
 MdeModulePkg/Core/Pei/PeiMain.h               |  1 +
 MdeModulePkg/Include/Guid/MigratedFvInfo.h    | 22 +++++++++++++++
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 28 +++++++++++++++++++
 5 files changed, 55 insertions(+)
 create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 16db17d0a873..40197dba862c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -389,6 +389,9 @@ [Guids]
   ## GUID indicates the capsule is to store Capsule On Disk file names.
   gEdkiiCapsuleOnDiskNameGuid = { 0x98c80a4f, 0xe16b, 0x4d11, { 0x93, 0x9a, 0xab, 0xe5, 0x61, 0x26, 0x3, 0x30 } }
 
+  ## Include/Guid/MigratedFvInfo.h
+  gEdkiiMigratedFvInfoGuid = { 0xc1ab12f7, 0x74aa, 0x408d, { 0xa2, 0xf4, 0xc6, 0xce, 0xfd, 0x17, 0x98, 0x71 } }
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid       = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
index 5b36d516b3fa..0cf357371a16 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -77,6 +77,7 @@ [Guids]
   ## CONSUMES   ## GUID      # Used to compare with FV's file system GUID and get the FV's file system format
   gEfiFirmwareFileSystem3Guid
   gStatusCodeCallbackGuid
+  gEdkiiMigratedFvInfoGuid                      ## SOMETIMES_PRODUCES     ## HOB
 
 [Ppis]
   gEfiPeiStatusCodePpiGuid                      ## SOMETIMES_CONSUMES # PeiReportStatusService is not ready if this PPI doesn't exist
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index b0101dba5e30..cbf74d5b9d9a 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/FirmwareFileSystem2.h>
 #include <Guid/FirmwareFileSystem3.h>
 #include <Guid/AprioriFileName.h>
+#include <Guid/MigratedFvInfo.h>
 
 ///
 /// It is an FFS type extension used for PeiFindFileEx. It indicates current
diff --git a/MdeModulePkg/Include/Guid/MigratedFvInfo.h b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
new file mode 100644
index 000000000000..061c17ed0e48
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
@@ -0,0 +1,22 @@
+/** @file
+  Migrated FV information
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
+#define __EDKII_MIGRATED_FV_INFO_GUID_H__
+
+typedef struct {
+  UINT32           FvOrgBase;  // original FV address
+  UINT32           FvNewBase;  // new FV address
+  UINT32           FvDataBase; // original FV data
+  UINT32           FvLength;   // Fv Length
+} EDKII_MIGRATED_FV_INFO;
+
+extern EFI_GUID gEdkiiMigratedFvInfoGuid;
+
+#endif // #ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
+
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index ef88b3423376..f654cea15c59 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1223,10 +1223,12 @@ EvacuateTempRam (
   EFI_FIRMWARE_VOLUME_HEADER    *FvHeader;
   EFI_FIRMWARE_VOLUME_HEADER    *ChildFvHeader;
   EFI_FIRMWARE_VOLUME_HEADER    *MigratedFvHeader;
+  EFI_FIRMWARE_VOLUME_HEADER    *RawDataFvHeader;
   EFI_FIRMWARE_VOLUME_HEADER    *MigratedChildFvHeader;
 
   PEI_CORE_FV_HANDLE            PeiCoreFvHandle;
   EFI_PEI_CORE_FV_LOCATION_PPI  *PeiCoreFvLocationPpi;
+  EDKII_MIGRATED_FV_INFO        MigratedFvInfo;
 
   ASSERT (Private->PeiMemoryInstalled);
 
@@ -1263,6 +1265,9 @@ EvacuateTempRam (
         (((EFI_PHYSICAL_ADDRESS)(UINTN) FvHeader + (FvHeader->FvLength - 1)) < Private->FreePhysicalMemoryTop)
         )
       ) {
+      //
+      // Allocate page to save the rebased PEIMs, the PEIMs will get control later
+      //
       Status =  PeiServicesAllocatePages (
                   EfiBootServicesCode,
                   EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength),
@@ -1270,6 +1275,17 @@ EvacuateTempRam (
                   );
       ASSERT_EFI_ERROR (Status);
 
+      //
+      // Allocate pool to save the raw PEIMs, it used to keep consistent context across
+      // multiple boot and PCR0 will keep same no matter if allocate random page address.
+      //
+      Status =  PeiServicesAllocatePages (
+                  EfiBootServicesCode,
+                  EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength),
+                  (EFI_PHYSICAL_ADDRESS *) &RawDataFvHeader
+                  );
+      ASSERT_EFI_ERROR (Status);
+
       DEBUG ((
         DEBUG_VERBOSE,
         "  Migrating FV[%d] from 0x%08X to 0x%08X\n",
@@ -1278,7 +1294,19 @@ EvacuateTempRam (
         (UINTN) MigratedFvHeader
         ));
 
+      //
+      // Copy the context to the rebased pages and raw pages, and create hob to save the
+      // information. the MigratedFvInfo HOB will never produce when
+      // PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD control the
+      // feature.
+      //
       CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength);
+      CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN) FvHeader->FvLength);
+      MigratedFvInfo.FvOrgBase  = (UINT32) (UINTN) FvHeader;
+      MigratedFvInfo.FvNewBase  = (UINT32) (UINTN) MigratedFvHeader;
+      MigratedFvInfo.FvDataBase = (UINT32) (UINTN) RawDataFvHeader;
+      MigratedFvInfo.FvLength   = (UINT32) (UINTN) FvHeader->FvLength;
+      BuildGuidDataHob (&gEdkiiMigratedFvInfoGuid, &MigratedFvInfo, sizeof (MigratedFvInfo));
 
       //
       // Migrate any children for this FV now
-- 
2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62223): https://edk2.groups.io/g/devel/message/62223
Mute This Topic: https://groups.io/mt/75372253/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 07/08/20 10:10, Guomin Jiang wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> When we allocate pool to save the rebased PEIMs, the address will change
> randomly, therefore the hash will change and result PCR0 change as well.
> To avoid this, we save the raw PEIMs and use it to calculate hash.
> 
> The MigratedFvInfo HOB will never produce when
> PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD control
> the total feature.

[...]

> @@ -1278,7 +1294,19 @@ EvacuateTempRam (
>          (UINTN) MigratedFvHeader
>          ));
>  
> +      //
> +      // Copy the context to the rebased pages and raw pages, and create hob to save the
> +      // information. the MigratedFvInfo HOB will never produce when
> +      // PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD control the
> +      // feature.
> +      //
>        CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength);
> +      CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN) FvHeader->FvLength);
> +      MigratedFvInfo.FvOrgBase  = (UINT32) (UINTN) FvHeader;
> +      MigratedFvInfo.FvNewBase  = (UINT32) (UINTN) MigratedFvHeader;
> +      MigratedFvInfo.FvDataBase = (UINT32) (UINTN) RawDataFvHeader;
> +      MigratedFvInfo.FvLength   = (UINT32) (UINTN) FvHeader->FvLength;
> +      BuildGuidDataHob (&gEdkiiMigratedFvInfoGuid, &MigratedFvInfo, sizeof (MigratedFvInfo));
>  
>        //
>        // Migrate any children for this FV now
> 

Thank you for addressing my requests!

This patch should definitely be reviewed by PeiCore experts, but from my
perspective, I'm happy with the updates.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62254): https://edk2.groups.io/g/devel/message/62254
Mute This Topic: https://groups.io/mt/75372253/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-