Michael, I need Jiewen's input on why MmAccess and MmCommunication PPIs were not installed in normal boot path. Without understanding the reason, I don't have confidence to approve the change.
Sai, Do you see other impacts to Intel platforms with this behavior change? Thanks, Ray -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki Sent: Tuesday, August 10, 2021 11:36 PM To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V <rangasai.v.chaga...@intel.com> Cc: Yao, Jiewen <jiewen....@intel.com> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg/PeiSmmAccessLib: Remove S3 requirement Installation is a platform decision. The buried dependency on boot mode in this particular function is just a roadblock platforms have to work around. The role of this API is to install the PPI. Thanks, Michael On 8/9/2021 9:47 PM, Ni, Ray wrote: > Michael, > Allowing the gPeiSmmAccessPpiGuid PPI installation in normal boot will > further allow gEfiPeiSmmCommunicationPpiGuid installation in normal path, > while without your change neither of the PPIs is installed in normal boot. > > + Jiewen for potential security concern. > > Thanks, > Ray > >> -----Original Message----- >> From: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com> >> Sent: Tuesday, August 10, 2021 6:46 AM >> To: mikub...@linux.microsoft.com; devel@edk2.groups.io >> Cc: Ni, Ray <ray...@intel.com> >> Subject: RE: [edk2-platforms][PATCH v1 1/1] >> IntelSiliconPkg/PeiSmmAccessLib: Remove S3 requirement >> >> Reviewed-by: Sai Chaganty <rangasai.v.chaga...@intel.com> >> >> -----Original Message----- >> From: mikub...@linux.microsoft.com <mikub...@linux.microsoft.com> >> Sent: Monday, August 09, 2021 6:40 AM >> To: devel@edk2.groups.io >> Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V >> <rangasai.v.chaga...@intel.com> >> Subject: [edk2-platforms][PATCH v1 1/1] >> IntelSiliconPkg/PeiSmmAccessLib: Remove S3 requirement >> >> From: Michael Kubacki <michael.kuba...@microsoft.com> >> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3539 >> >> PeiInstallSmmAccessPpi() currently requires the boot mode be set to S3 to >> actually install gEfiPeiMmAccessPpiGuid. >> >> This change removes this requirement in the function implementation for two >> reasons: >> >> 1. Practical use cases exist to require this PPI in cases other than >> the boot mode being set to BOOT_ON_S3_RESUME. >> >> 2. It is poor API design to implicitly bury this requirement within >> a function whose responsibility is to install the PPI. The caller >> can easily place arbitrary constraints around whether to call >> based on conditions such as the boot mode being >> BOOT_ON_S3_RESUME. >> >> Cc: Ray Ni <ray...@intel.com> >> Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com> >> Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> >> --- >> >> Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.c >> | 12 ------------ >> 1 file changed, 12 deletions(-) >> >> diff --git >> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcces >> sLib/PeiSmmAccessLib.c >> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcces >> sLib/PeiSmmAccessLib.c index d9bf4fba983e..4df0d695fdaf 100644 >> --- >> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcces >> sLib/PeiSmmAccessLib.c >> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmA >> +++ cce >> +++ ssLib/PeiSmmAccessLib.c >> @@ -252,19 +252,7 @@ PeiInstallSmmAccessPpi ( >> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *DescriptorBlock; >> SMM_ACCESS_PRIVATE_DATA *SmmAccessPrivate; >> VOID *HobList; >> - EFI_BOOT_MODE BootMode; >> >> - Status = PeiServicesGetBootMode (&BootMode); >> - if (EFI_ERROR (Status)) { >> - // >> - // If not in S3 boot path. do nothing >> - // >> - return EFI_SUCCESS; >> - } >> - >> - if (BootMode != BOOT_ON_S3_RESUME) { >> - return EFI_SUCCESS; >> - } >> // >> // Initialize private data >> // >> -- >> 2.28.0.windows.1 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79145): https://edk2.groups.io/g/devel/message/79145 Mute This Topic: https://groups.io/mt/84768258/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
