On 07/03/20 16:00, Laszlo Ersek wrote: > On 07/02/20 07:15, Guomin Jiang wrote: >> From: Jian J Wang <jian.j.w...@intel.com> >> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614 >> >> 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> >> Signed-off-by: Jian J Wang <jian.j.w...@intel.com> >> --- >> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +++ >> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 2 +- >> 2 files changed, 4 insertions(+), 1 deletion(-) > > (1) The commit message is empty, and therefore useless. Please explain > why this change is being made. > >> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >> index 3f1702854660..4ab54594ed66 100644 >> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >> @@ -121,6 +121,9 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] >> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## >> SOMETIMES_CONSUMES >> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## >> SOMETIMES_CONSUMES >> >> +[Pcd] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES >> + >> [Depex] >> gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid >> >> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c >> b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c >> index d48028cea0dd..9e1831c69819 100644 >> --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c >> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c >> @@ -77,7 +77,7 @@ PeimInitializeDxeIpl ( >> >> BootMode = GetBootModeHob (); >> >> - if (BootMode != BOOT_ON_S3_RESUME) { >> + if (BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot)) { >> Status = PeiServicesRegisterForShadow (FileHandle); >> if (Status == EFI_SUCCESS) { >> // >> > > (2) The above check does not seem complete. I think it should consider > "PcdMigrateTemporaryRamFirmwareVolumes". > > I don't exactly understand the impact of the change, but it seems to > potentially affect even such platforms that set > "PcdMigrateTemporaryRamFirmwareVolumes" to FALSE; and that seems wrong.
... On further consideration, this patch seems to be fixing a preexistent bug that is not related to the CVE at all. I think this issue was simply exposed when testing the new feature. Is that right? If that's correct, then please explain this very clearly in the commit message. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62039): https://edk2.groups.io/g/devel/message/62039 Mute This Topic: https://groups.io/mt/75252663/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
