Re: [edk2-devel] [PATCH V5 00/13] Enable Tdx measurement in OvmfPkgX64

Re: [edk2-devel] [PATCH V5 00/13] Enable Tdx measurement in OvmfPkgX64

[Min Xu]

Hi, Gerd
This patch-set (v5) has re-organized the patches based on https://edk2.groups.io/g/devel/message/99209
Do you have some comments?

Thanks
Min

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Min Xu
> Sent: Saturday, January 28, 2023 9:58 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Michael Roth
> <michael.roth@amd.com>
> Subject: [edk2-devel] [PATCH V5 00/13] Enable Tdx measurement in
> OvmfPkgX64
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243
> 
> Tdx measurement (RTMR based measurement) is enabled in OvmfPkg/IntelTdx.
> This patch-set enables the feature in OvmfPkgX64 as well.
> 
> Patch #1:
>   Introduce TDX_MEASUREMETNS_DATA in SEC_TDX_WORK_AREA. That is
> because
>   the RTMR measurement of TdHob and Configuration FV (CFV) are executed
>   in very early stage of boot process. At that time the memory service is
>   not ready and the measurement values have to be stored in OvmfWorkArea.
> 
> Patch #2:
>   Introduce TdxHelperLibNull which is the NULL instance of TdxHelperLib.
> 
> Patch #3:
>   Introduce SecTdxHelperLib which is the instance of TdxHelperLib for SEC
>   Phase. This patch adds the stubs of TdxHelperLib functions. The actual
>   implementation are in the following patches.
> 
> Patch #4:
>   Re-use the data struct of PLATFORM_FIRMWARE_BLOB2_STRUCT for
>   FV_HANDOFF_TABLE_POINTERS2.
> 
> Patch #5-7:
>   These 3 patches move the functions ( which were implemented in
>   PeilessStartupLib and PlatformInitLib ) to TdxHelperLib. So that they
>   can be called in both OvmfPkgX64 and IntelTdxX64.
> 
> Patch #8/9:
>   These 2 patches are the changes for tdx measurement in IntelTdxX64.
> 
> Patch #10-13:
>   These 4 patches are the changes for OvmfPkgX64 to enable Tdx
>   measurement.
> 
> Code: https://github.com/mxu9/edk2/tree/TdxMeasurementInOvmfX64.v5
> 
> v5 changes:
>  - Re-organize the patches. Its purpose is not only to simplify review, but also
>    to simplify testing. https://edk2.groups.io/g/devel/message/99209
> 
> v4 changes:
>  - To make the code reviewable, the implementation of
>    TdxHelperBuildGuidHobForTdxMeasurement is split into 4 patches (5-8).
>  - Call Sha384HashAll instead of the 3 Sha384XXX functions so that we
>    need to allocate memory in SEC phase.
> 
> v3 changes:
>  - Use the definition of PLATFORM_FIRMWARE_BLOB2_STRUCT in
>    Library/TcgEventLogRecordLib.h.
>  - Rename TDX_ENABLE as TDX_MEASUREMENT_ENABLE because this flag is
>    introduced for Tdx-measurement.
>  - Split the patch of SecTdxHelperLib into 2 separate patches (#3/#9).
>    Patch#3 implements TdxHelperMeasureTdHob and
> TdxHelperMeasureCfvImage.
>    Patch#9 implements TdxHelperProcessTdHob. This is to make the patches
>    more reviewable. The duplicated codes of TdxHelperProcessTdHob are
>    deleted in Patch#9 as well.
>  - The implementation of TdxHelperBuildGuidHobForTdxMeasurement and
> update
>    of PeilessStartupLib are in one patch (#5). Because the implmentation
>    of TdxHelperBuildGuidHobForTdxMeasurement was once in PeilessStartupLib.
> 
> v2 changes:
>  - Split the patch of TdxHelperLib into 4 separate patches. So that it is
>    more reviewable.
>  - Add commit message in Patch#1 to emphasize that the tdx-measurement in
>    OvmfPkgX64 is supported in SEC phase.
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min M Xu (13):
>   OvmfPkg: Add Tdx measurement data structure in WorkArea
>   OvmfPkg/IntelTdx: Add TdxHelperLibNull
>   OvmfPkg/IntelTdx: Add SecTdxHelperLib
>   OvmfPkg/PeilessStartupLib: Update the define of
>     FV_HANDOFF_TABLE_POINTERS2
>   OvmfPkg: Refactor MeasureHobList
>   OvmfPkg: Refactor MeaureFvImage
>   OvmfPkg: Refactor ProcessHobList
>   OvmfPkg/IntelTdx: Measure TdHob and Configuration FV in SecMain
>   OvmfPkg/PeilessStartupLib: Delete the duplicated tdx measurement
>   OvmfPkg/IntelTdx: Add PeiTdxHelperLib
>   OvmfPkg/OvmfPkgX64: Measure TdHob and Configuration FV in SecMain
>   OvmfPkg/PlatformPei: Build GuidHob for Tdx measurement
>   OvmfPkg: Support Tdx measurement in OvmfPkgX64
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   5 +-
>  OvmfPkg/CloudHv/CloudHvX64.dsc                |   5 +-
>  OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc       |  10 +-
>  .../Include/Dsc/OvmfTpmSecurityStub.dsc.inc   |   8 +
>  OvmfPkg/Include/Library/PlatformInitLib.h     |  17 -
>  OvmfPkg/Include/Library/TdxHelperLib.h        |  70 ++
>  OvmfPkg/Include/WorkArea.h                    |  25 +-
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |   4 +-
>  OvmfPkg/IntelTdx/Sec/SecMain.c                |  17 +-
>  OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c  |  91
> +++  .../IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf |  48 ++
>  .../TdxHelperLib/SecTdxHelper.c}              | 304 +++----
>  .../IntelTdx/TdxHelperLib/SecTdxHelperLib.inf |  53 ++
>  .../TdxHelperLib/TdxHelperLibNull.inf         |  32 +
>  OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c |  79
> ++  .../IntelTdx/TdxHelperLib/TdxMeasurementHob.c | 259 ++++++
> OvmfPkg/Library/PeilessStartupLib/IntelTdx.c  | 196 -----
>  .../PeilessStartupLib/PeilessStartup.c        |  16 +-
>  .../PeilessStartupInternal.h                  |  36 -
>  .../PeilessStartupLib/PeilessStartupLib.inf   |   6 -
>  OvmfPkg/Library/PlatformInitLib/IntelTdx.c    | 768 ------------------
>  .../Library/PlatformInitLib/IntelTdxNull.c    |  20 -
>  .../PlatformInitLib/PlatformInitLib.inf       |   1 -
>  OvmfPkg/Microvm/MicrovmX64.dsc                |   5 +-
>  OvmfPkg/OvmfPkg.dec                           |   4 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  20 +-
>  OvmfPkg/OvmfPkgX64.fdf                        |   7 +
>  OvmfPkg/PlatformPei/IntelTdx.c                |   3 +
>  OvmfPkg/Sec/SecMain.c                         |  17 +-
>  29 files changed, 915 insertions(+), 1211 deletions(-)  create mode 100644
> OvmfPkg/Include/Library/TdxHelperLib.h
>  create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c
>  create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf
>  copy OvmfPkg/{Library/PlatformInitLib/IntelTdx.c =>
> IntelTdx/TdxHelperLib/SecTdxHelper.c} (80%)  create mode 100644
> OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
>  create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
>  create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c
>  create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
>  delete mode 100644 OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
> 
> --
> 2.29.2.windows.2
> 
> 
> 
> 
>