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

When PcdMigrateTemporaryRamFirmwareVolumes is TRUE, it will shadow the
PEIMs, when it is disabled, PEIMs marked REGISTER_FOR_SHADOW will be
shadowed as well and it is controled by PcdShadowPeimOnBoot and
PcdShadowPeimOnS3Boot.

To cover the shadow behavior, the change will always shadow PEIMs when
enable PcdMigrateTemporaryRamFirmwareVolumes.

When PcdMigrateTemporaryRamFirmwareVolumes is true, if enable
PcdShadowPeimOnBoot or PcdShadowPeimOnS3Boot, it will shadow some PEIMs
twice and occupy more memory and waste more boot time, it is unnecessary,
so the only valid choice is to enable PcdMigrateTemporaryRamFirmwareVolumes
and disable PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot.

Signed-off-by: Guomin Jiang <guomin.ji...@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Hao A Wu <hao.a...@intel.com>
Cc: Dandan Bi <dandan...@intel.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Debkumar De <debkumar...@intel.com>
Cc: Harry Han <harry....@intel.com>
Cc: Catharine West <catharine.w...@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec                 | 11 +++++---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 14 +++++++---
 MdeModulePkg/Core/Pei/Image/Image.c           |  6 ++---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c       | 26 +++++++++++++++----
 4 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index e0ad9373e62f..5220202b233b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1223,11 +1223,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt Shadow Peim and PeiCore on boot
   gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|TRUE|BOOLEAN|0x30001029
 
-  ## Enable the feature that evacuate temporary memory to permanent memory or 
not
+  ## Enable the feature that evacuate temporary memory to permanent memory or 
not<BR><BR>
   #  Set FALSE as default, if the developer need this feature to avoid this 
vulnerability, please
-  #  enable it in dsc file.
-  #  TRUE - Evacuate temporary memory, the actions include copy memory, 
convert PPI pointers and so on.
-  #  FALSE - Do nothing, for example, no copy memory, no convert PPI pointers 
and so on.
+  #  enable it and disable PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot in 
dsc file at the same time.<BR>
+  #  The reason is that PcdMigrateTemporaryRamFirmwareVolumes will make all 
PEIMs be shadowed and
+  #  it is unnecessary that shadow PEIMs which is controled by 
PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot
+  #  again, it will occupy more memory and waste more time if you enable 
it.<BR>
+  #  TRUE - Evacuate temporary memory, the actions include copy memory, 
convert PPI pointers and so on.<BR>
+  #  FALSE - Do nothing, for example, no copy memory, no convert PPI pointers 
and so on.<BR>
   # @Prompt Evacuate temporary memory to permanent memory
   
gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes|FALSE|BOOLEAN|0x3000102A
 
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 667d9273bb91..11ff5e693304 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1408,7 +1408,11 @@ PeiDispatcher (
   PeimFileHandle = NULL;
   EntryPoint     = 0;
 
-  if ((Private->PeiMemoryInstalled) && 
(Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || 
PcdGetBool (PcdShadowPeimOnS3Boot))) {
+  if ((Private->PeiMemoryInstalled) &&
+      (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)
+       || (Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME)
+       || PcdGetBool (PcdShadowPeimOnS3Boot))
+    ) {
     //
     // Once real memory is available, shadow the RegisterForShadow modules. 
And meanwhile
     // update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW to 
PEIM_STATE_DONE.
@@ -1607,13 +1611,17 @@ PeiDispatcher (
             PeiCheckAndSwitchStack (SecCoreData, Private);
 
             if ((Private->PeiMemoryInstalled) && 
(Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW) 
&&   \
-                (Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
+                (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)
+                 || (Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME)
+                 || PcdGetBool (PcdShadowPeimOnS3Boot))
+              ) {
               //
               // If memory is available we shadow images by default for 
performance reasons.
               // We call the entry point a 2nd time so the module knows it's 
shadowed.
               //
               //PERF_START (PeiServices, L"PEIM", PeimFileHandle, 0);
-              if ((Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME) && !PcdGetBool (PcdShadowPeimOnBoot)) {
+              if ((Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME) && !PcdGetBool (PcdShadowPeimOnBoot)
+                  && !PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
                 //
                 // Load PEIM into Memory for Register for shadow PEIM.
                 //
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c 
b/MdeModulePkg/Core/Pei/Image/Image.c
index 0caeb63e26b4..f9b570ba1f47 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -299,7 +299,7 @@ LoadAndRelocatePeCoffImage (
   IsRegisterForShadow = FALSE;
   if ((Private->CurrentFileHandle == FileHandle)
     && 
(Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] 
== PEIM_STATE_REGISTER_FOR_SHADOW)) {
-    IsRegisterForShadow = TRUE;
+    IsRegisterForShadow = TRUE && !PcdGetBool 
(PcdMigrateTemporaryRamFirmwareVolumes);
   }
 
   //
@@ -319,8 +319,7 @@ LoadAndRelocatePeCoffImage (
   // Check whether the file type is PEI module.
   //
   IsPeiModule = FALSE;
-  if (FileInfo.FileType == EFI_FV_FILETYPE_PEI_CORE ||
-      FileInfo.FileType == EFI_FV_FILETYPE_PEIM ||
+  if (FileInfo.FileType == EFI_FV_FILETYPE_PEIM ||
       FileInfo.FileType == EFI_FV_FILETYPE_COMBINED_PEIM_DRIVER) {
     IsPeiModule = TRUE;
   }
@@ -342,6 +341,7 @@ LoadAndRelocatePeCoffImage (
   // Allocate Memory for the image when memory is ready, and image is 
relocatable.
   // On normal boot, PcdShadowPeimOnBoot decides whether load PEIM or PeiCore 
into memory.
   // On S3 boot, PcdShadowPeimOnS3Boot decides whether load PEIM or PeiCore 
into memory.
+  // PeiCore is specificial case, it will separate from IsPeiModule.
   //
   if ((!ImageContext.RelocationsStripped) && (Private->PeiMemoryInstalled) && 
((!IsPeiModule) ||
       (!IsS3Boot && (PcdGetBool (PcdShadowPeimOnBoot) || IsRegisterForShadow)) 
|| (IsS3Boot && PcdGetBool (PcdShadowPeimOnS3Boot)))) {
diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c 
b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 48605eeada86..3f168bb56893 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -322,7 +322,8 @@ PeiCore (
       // PEI Core and PEIMs to get high performance.
       //
       OldCoreData->ShadowedPeiCore = (PEICORE_FUNCTION_POINTER) (UINTN) 
PeiCore;
-      if ((HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME && 
PcdGetBool (PcdShadowPeimOnS3Boot))
+      if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)
+          || (HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME && 
PcdGetBool (PcdShadowPeimOnS3Boot))
           || (HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME && 
PcdGetBool (PcdShadowPeimOnBoot))) {
         OldCoreData->ShadowedPeiCore = ShadowPeiCore (OldCoreData);
       }
@@ -422,10 +423,25 @@ PeiCore (
     }
   } else {
     if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
-      if (PrivateData.HobList.HandoffInformationTable->BootMode == 
BOOT_ON_S3_RESUME) {
-        TempRamEvacuation = PcdGetBool (PcdShadowPeimOnS3Boot);
-      } else {
-        TempRamEvacuation = PcdGetBool (PcdShadowPeimOnBoot);
+      TempRamEvacuation = TRUE;
+
+      //
+      // When PcdMigrateTemporaryRamFirmwareVolumes is TRUE, it makes sense 
only
+      // when both PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot is FALSE.
+      // The reason is that PcdMigrateTemporaryRamFirmwareVolumes will make 
all PEIMs
+      // be shadowed and it is unnecessary that shadow PEIMs which is 
controled by
+      // PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot again, it will occupy 
more
+      // memory and waste more time if you enable it.
+      //
+      if (PcdGetBool (PcdShadowPeimOnBoot) || PcdGetBool 
(PcdShadowPeimOnS3Boot)) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "!!!IMPORTANT NOTICE!!!\n"
+          "When you see the message, it mean that you enable the 
PcdShadowPeimOnBoot or PcdShadowPeimOnS3Boot when enable 
PcdMigrateTemporaryRamFirmwareVolumes\n"
+          "It make no sense because it will occupy more memory and waste more 
time.\n"
+          "You must disable PcdShadowPeimOnBoot and PcdShadowPeimOnS3Boot when 
enable PcdMigrateTemporaryRamFirmwareVolumes for performance reason.\n\n"));
+        ASSERT ((PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes) == TRUE) &&
+                (PcdGetBool (PcdShadowPeimOnBoot) == FALSE) && (PcdGetBool 
(PcdShadowPeimOnS3Boot) == FALSE));
       }
     }
 
-- 
2.25.1.windows.1


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

View/Reply Online (#63102): https://edk2.groups.io/g/devel/message/63102
Mute This Topic: https://groups.io/mt/75720856/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to