Re: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install MigrateTempRamPpi

["Li, Zhihao" <zhihao.li@intel.com>]

Hi

The change in MdeModulePkg have been merged now. Could you please help to review the patch in IntelFsp2WrapperPkg.
I had sent the patch on Apr 30. I can send it again if you need.
This is the PR link for IntelFsp2WrapperPkg: https://github.com/tianocore/edk2/pull/5601/commits/290d3347c45d359da3ff1ccd50f290a45767d43a

Thanks a lot.

BR,
Zhihao

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Saturday, June 15, 2024 11:16 AM
To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
Subject: 回复: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install MigrateTempRamPpi

Zhihao:
  I have no other comment for the change in MdeModulePkg. Please create pull request for it. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Li, Zhihao <zhihao.li@intel.com>
> 发送时间: 2024年6月11日 15:36
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
> 主题: RE: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install 
> MigrateTempRamPpi
> 
> Hi Liming
> 
> If there are no concerns about it, could you please help to review the 
> patch in MdeModulePkg scope and check in?
> And then, I contact with the maintainers of IntelFsp2WrapperPkg for 
> another patch review.
> 
> BR,
> Zhihao
> 
> -----Original Message-----
> From: Li, Zhihao
> Sent: Thursday, May 30, 2024 2:32 PM
> To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: 
> Install MigrateTempRamPpi
> 
> Yes, they are used.
> Refer to https://bugzilla.tianocore.org/show_bug.cgi?id=2376 , Fsp 
> binary measurement has been implemented and controlled by 
> PcdFspMeasurementConfig.
> Current defect:
> 1. FSP-T/FSP-M may not be migrated.
> 2. Even if FSP-M has been migrated, its measurement still used the 
> original address.
> Corresponding modifications:
> In MdeModulePkg scope:
> 1. Add the gEdkiiPeiMigrateTempRamPpiGuid and install it after 
> EvacuateTempRam is called.
> In IntelFsp2WrapperPkg scope:
> 1. Add MigrateTempRamPpi notification which will check the migration 
> of FSP-T/M and migrate them if they are not migrated but need to be measured.
> 2. Fix Tcg notification to use migrated address if the binaries had been migrated.
> 
> BR,
> Zhihao
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Thursday, May 30, 2024 1:12 PM
> To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: 
> Install MigrateTempRamPpi
> 
> Zhihao:
>   If Fsp-T/M is not installed, are they still used in PEI boot? If 
> they are used, I agree they should be measured.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> > 发送时间: 2024年5月29日 11:36
> > 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > 抄送: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> > <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B 
> > <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
> > 主题: Re: [edk2-devel] [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install 
> > MigrateTempRamPpi
> >
> > Issue description:
> > 1. PeiCore only migrates Fsp-M in dispatch mode and doesn't migrate 
> > Fsp-T and Fsp-M in Api mode.
> > 2. Fsp-T and Fsp-M will be measured in post-mem PEI and the 
> > measurement uses original addresses.
> > RootCause:
> > PeiCore only migrates installed FVs and Fsp-T/M may not be installed.
> >
> > Defect in implementation:
> > In MdeModulePkg/Core/Pei/PeiMain/PeiMain.c line 450:
> > EvacuateTempRam will migrate installed content from Temporary RAM to 
> > Permanent RAM because of BootGuard TOCTOU 
> > vulnerability(https://bugzilla.tianocore.org/show_bug.cgi?id=1614).
> > In IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c line 220:
> > FspmWrapperInit will install Fspm in dispatch mode or directly call 
> > PeiFspMemoryInit function in api mode.
> > ==>
> > Api mode: Fsp-T and Fsp-M are not migrated because they are not installed.
> > Dispatch mode: Fsp-T is not migrated because it is not installed.
> >
> > In IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c line 291,
> 300:
> > TcgPpiNotify transmits original addresses(PcdFsptBaseAddress,
> > PcdFspmBaseAddress) to MeasureFspFirmwareBlob which will trigger 
> > HashLogExtendEvent.
> > In SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c line 966:
> > TcgPpi will be installed in PeimEntryMP which will be called when 
> > the PEI Foundation discovers permanent memory(line 1059 
> > mImageInMemory =
> TRUE).
> > ==>
> > Original addresses of Fsp-T and Fsp-M will be used for measurement 
> > after permanent memory is ready and installed FVs are migrated.
> >
> >
> > Solution:
> > MdeModulePkg: PeiCore Installs MigrateTempRamPpi if 
> > PcdMigrateTemporaryRamFirmwareVolumes is True.
> > IntelFsp2WrapperPkg : 1. MigrateTempRamPpi nitification in 
> > FspmWrapperPeim migrates FspT/M binary to permanent memory and build
> MigatedFvInfoHob.
> >                                           2. TCG notification checks 
> > MigatedFvInfoHob and transmits DRAM address for measurement.
> >
> > BR,
> > Zhihao
> >
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Tuesday, May 28, 2024 5:44 PM
> > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io
> > Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> > <nathaniel.l.desimone@intel.com>; Duggapu, Chinni B 
> > <chinni.b.duggapu@intel.com>; Chen, Gang C <gang.c.chen@intel.com>
> > Subject: 回复: [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install 
> > MigrateTempRamPpi
> >
> > Zhihao:
> >   Could you explain the situation that FSP-T/M is not migrated by PeiCore?
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: Zhihao Li <zhihao.li@intel.com>
> > > 发送时间: 2024年4月29日 11:20
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Chasel Chiu <chasel.chiu@intel.com>; Nate DeSimone 
> > > <nathaniel.l.desimone@intel.com>; Duggapu Chinni B 
> > > <chinni.b.duggapu@intel.com>; Chen Gang C <gang.c.chen@intel.com>; 
> > > Liming Gao <gaoliming@byosoft.com.cn>
> > > 主题: [PATCH v1 1/2] MdeModulePkg/Core/Pei: Install 
> > > MigrateTempRamPpi
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4716
> > >
> > > Migrate FSP-T/M binary from temporary RAM to permanent RAM before 
> > > NEM tear down. Tcg module will use permanent address of FSP-T/M 
> > > for measurement.
> > > 1. PeiCore installs mMigrateTempRamPpi if 
> > > PcdMigrateTemporaryRamFirmwareVolumes is True 2. FspmWrapperPeim 
> > > migrate FspT/M binary to permanent memory and build 
> > > MigatedFvInfoHob 3. TCG notification checks MigatedFvInfoHob and 
> > > transmits DRAM address for measurement
> > >
> > > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > > Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> > > Cc: Chen Gang C <gang.c.chen@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > >
> > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > ---
> > >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c    | 10 ++++++++-
> > >  MdeModulePkg/Core/Pei/PeiMain.h            |  3 ++-
> > >  MdeModulePkg/Core/Pei/PeiMain.inf          |  3 ++-
> > >  MdeModulePkg/Include/Guid/MigratedFvInfo.h |  4 ++-- 
> > > MdeModulePkg/Include/Ppi/MigrateTempRam.h  | 23
> > > ++++++++++++++++++++
> > >  MdeModulePkg/MdeModulePkg.dec              |  5 ++++-
> > >  6 files changed, 42 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > > index bf1719d7941a..0e3d9a843816 100644
> > > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    Pei Core Main Entry Point
> > >
> > > -Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > > reserved.<BR>
> > > +Copyright (c) 2006 - 2024, Intel Corporation. All rights 
> > > +reserved.<BR>
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -13,6 +13,11 @@ EFI_PEI_PPI_DESCRIPTOR  mMemoryDiscoveredPpi
> = {
> > >    &gEfiPeiMemoryDiscoveredPpiGuid,
> > >    NULL
> > >  };
> > > +EFI_PEI_PPI_DESCRIPTOR  mMigrateTempRamPpi = {
> > > +  (EFI_PEI_PPI_DESCRIPTOR_PPI |
> > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> > > +  &gEdkiiPeiMigrateTempRamPpiGuid,
> > > +  NULL
> > > +};
> > >
> > >  ///
> > >  /// Pei service instance
> > > @@ -449,6 +454,9 @@ PeiCore (
> > >        //
> > >        EvacuateTempRam (&PrivateData, SecCoreData);
> > >
> > > +      Status = PeiServicesInstallPpi (&mMigrateTempRamPpi);
> > > +      ASSERT_EFI_ERROR (Status);
> > > +
> > >        DEBUG ((DEBUG_VERBOSE, "PPI lists after temporary RAM 
> > > evacuation:\n"));
> > >        DumpPpiList (&PrivateData);
> > >      }
> > > diff --git a/MdeModulePkg/Core/Pei/PeiMain.h 
> > > b/MdeModulePkg/Core/Pei/PeiMain.h index 46b6c23014a3..8df0c2d561f7
> > > 100644
> > > --- a/MdeModulePkg/Core/Pei/PeiMain.h
> > > +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    Definition of Pei Core Structures and Services
> > >
> > > -Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > > reserved.<BR>
> > > +Copyright (c) 2006 - 2024, Intel Corporation. All rights 
> > > +reserved.<BR>
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -26,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent 
> > > #include <Ppi/TemporaryRamDone.h>  #include <Ppi/SecHobData.h> 
> > > #include <Ppi/PeiCoreFvLocation.h>
> > > +#include <Ppi/MigrateTempRam.h>
> > >  #include <Library/DebugLib.h>
> > >  #include <Library/PeiCoreEntryPoint.h>  #include 
> > > <Library/BaseLib.h> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> > > b/MdeModulePkg/Core/Pei/PeiMain.inf
> > > index 893bdc052798..4e545ddab2ab 100644
> > > --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> > > +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> > > @@ -6,7 +6,7 @@
> > >  # 2) Dispatch PEIM from discovered FV.
> > >  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
> > >  #
> > > -# Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > > reserved.<BR>
> > > +# Copyright (c) 2006 - 2024, Intel Corporation. All rights 
> > > +reserved.<BR>
> > >  #
> > >  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -101,6 
> > > +101,7 @@
> > >    gEfiPeiReset2PpiGuid                          ##
> > > SOMETIMES_CONSUMES
> > >    gEfiSecHobDataPpiGuid                         ##
> > > SOMETIMES_CONSUMES
> > >    gEfiPeiCoreFvLocationPpiGuid                  ##
> > > SOMETIMES_CONSUMES
> > > +  gEdkiiPeiMigrateTempRamPpiGuid                ## PRODUCES
> > >
> > >  [Pcd]
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> > > ## CONSUMES
> > > diff --git a/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> > > b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> > > index 1c8b0dfefc49..255e278235b1 100644
> > > --- a/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> > > +++ b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    Migrated FV information
> > >
> > > -Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> > > +Copyright (c) 2020 - 2024, Intel Corporation. All rights 
> > > +reserved.<BR>
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -50,7 +50,7 @@ typedef struct {
> > >
> > >  typedef struct {
> > >    UINT32    FvOrgBase;         // original FV address
> > > -  UINT32    FvNewBase;         // new FV address
> > > +  UINT32    FvNewBase;         // new FV address, 0 means rebased
> > data
> > > is not copied
> > >    UINT32    FvDataBase;        // original FV data, 0 means raw data is
> > not
> > > copied
> > >    UINT32    FvLength;          // Fv Length
> > >  } EDKII_MIGRATED_FV_INFO;
> > > diff --git a/MdeModulePkg/Include/Ppi/MigrateTempRam.h
> > > b/MdeModulePkg/Include/Ppi/MigrateTempRam.h
> > > new file mode 100644
> > > index 000000000000..9bbb55d5cf86
> > > --- /dev/null
> > > +++ b/MdeModulePkg/Include/Ppi/MigrateTempRam.h
> > > @@ -0,0 +1,23 @@
> > > +/** @file
> > > +  This file declares Migrate Temporary Memory PPI.
> > > +
> > > +  This PPI is published by the PEI Foundation when temporary RAM 
> > > + needs to
> > > evacuate.
> > > +  Its purpose is to be used as a signal for other PEIMs who can 
> > > + register
> > for a
> > > +  notification on its installation.
> > > +
> > > +  Copyright (c) 2024, Intel Corporation. All rights reserved.<BR>
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#ifndef PEI_MIGRATE_TEMP_RAM_PPI_H_ #define 
> > > +PEI_MIGRATE_TEMP_RAM_PPI_H_
> > > +
> > > +#define EFI_PEI_MIGRATE_TEMP_RAM_PPI_GUID \
> > > +  { \
> > > +    0xc79dc53b, 0xafcd, 0x4a6a, {0xad, 0x94, 0xa7, 0x6a, 0x3f, 
> > > +0xa9,
> > 0xe9,
> > > 0xc2 } \
> > > +  }
> > > +
> > > +extern EFI_GUID  gEdkiiPeiMigrateTempRamPpiGuid;
> > > +
> > > +#endif
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec 
> > > b/MdeModulePkg/MdeModulePkg.dec index 3a239a1687ea..43e92c68ca20
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -4,7 +4,7 @@
> > >  # and libraries instances, which are used for those modules.
> > >  #
> > >  # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights 
> > > reserved.<BR>
> > > +# Copyright (c) 2007 - 2024, Intel Corporation. All rights 
> > > +reserved.<BR>
> > >  # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>  # (C) 
> > > Copyright 2016 - 2019 Hewlett Packard Enterprise Development 
> > > LP<BR> # Copyright (c) 2017, AMD Incorporated. All rights 
> > > reserved.<BR> @@
> > > -546,6 +546,9 @@
> > >    ## Include/Ppi/MemoryAttribute.h
> > >    gEdkiiMemoryAttributePpiGuid              = { 0x1be840de, 0x2d92,
> > > 0x41ec, { 0xb6, 0xd3, 0x19, 0x64, 0x13, 0x50, 0x51, 0xfb } }
> > >
> > > +  ## Include/Ppi/MigrateTempRam.h
> > > +  gEdkiiPeiMigrateTempRamPpiGuid            = { 0xc79dc53b, 0xafcd,
> > > 0x4a6a, { 0xad, 0x94, 0xa7, 0x6a, 0x3f, 0xa9, 0xe9, 0xc2 } }
> > > +
> > >  [Protocols]
> > >    ## Load File protocol provides capability to load and unload 
> > > EFI image
> > into
> > > memory and execute it.
> > >    #  Include/Protocol/LoadPe32Image.h
> > > --
> > > 2.44.0.windows.1
> >
> >
> >
> >
> >
> > 
> >
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119606): https://edk2.groups.io/g/devel/message/119606
Mute This Topic: https://groups.io/mt/106737501/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-