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

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

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 (#119546): https://edk2.groups.io/g/devel/message/119546
Mute This Topic: https://groups.io/mt/106383928/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-