Re: [PATCH v2 0/9] Need add a FSP binary measurement

["Wang, Jian J" <jian.j.wang@intel.com>]

Hi Qi,

Two common comments here. More specific comments will be given separately
in each patch email later.

c1. SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c and 
      SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c are almost
      the same. Consider consolidating the code in some way, like a shared lib
     or shared folder.
c2. TpmMeasurementGetFvName() or similar is duplicated in at least four places:
      DxeTpmMeasurementLib, PeiTpmMeasurementLib, BaseFspMeasurementLib
      and Tcg2Pei. Consider consolidate the code.


Regards,
Jian

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH v2 0/9] Need add a FSP binary measurement
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> The EDKII BIOS calls FSP API in FSP Wrapper Pkg.
> This FSP code need to be measured into TPM.
> 
> We need add a generic module in FSP Wrapper Pkg code to measure:
> 1) FSP-T, FSP-M, FSP-S in API mode.
> 2) FSP-T in Dispatch-mode. The FSP-M and FSP-S will be reported
>    as standard FV and they will be measured by TCG-PEI.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> 
> Jiewen Yao (8):
>   MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
>   MdeModulePkg/NullTpmMeasurementLib: Add new API.
>   SecurityPkg/DxeTpmMeasurementLib: Add new API.
>   SecurityPkg/PeiTpmMeasurementLib: Add new API.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
>   IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
>   IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and
>     PcdFspMeasurementConfig.
> 
> Qi Zhang (1):
>   SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
> 
>  .../FspmWrapperPeim/FspmWrapperPeim.c         |  90 ++++-
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  20 +-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         |  85 ++++-
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  27 +-
>  .../Include/Library/FspMeasurementLib.h       |  39 ++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  17 +
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc   |   5 +-
>  .../BaseFspMeasurementLib.inf                 |  54 +++
>  .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
>  .../Include/Library/TpmMeasurementLib.h       |  48 ++-
>  .../TpmMeasurementLibNull.c                   |  61 ++-
>  .../TpmMeasurementLibNull.inf                 |   6 +-
>  SecurityPkg/Include/Ppi/Tcg.h                 |   5 +
>  .../DxeTpmMeasurementLib.inf                  |   6 +-
>  .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib.inf                  |   4 +
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c             |  12 +-
>  18 files changed, 1233 insertions(+), 31 deletions(-)
>  create mode 100644
> IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLi
> b.inf
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
>  create mode 100644
> SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
>  create mode 100644
> SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> 
> --
> 2.26.2.windows.1