Re: [PATCH V4 06/12] OvmfPkg/PeilessStartupLib: Build GuidHob for Tdx measurement

["Gerd Hoffmann" <kraxel@redhat.com>]

On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243
> 
> 2 new functions are added in PeilessStartupLib/IntelTdx.c.
>  - BuildTdxMeasurementGuidHob
>  - InternalBuildGuidHobForTdxMeasurement
> 
> These 2 functions build GuidHob for Tdx measurement.

But you don't use them anywhere?  The point of splitting the patches is
not only to simplify review, but also to simplify testing (and in case a
bug shows up later finding it with bisecting).

So, current state of the code is:

There are MeasureHobList() + MeasureFvImage(), doing measurement and
logging in one go, using TpmMeasureAndLogData().  Problem is this
doesn't work in SEC, so you want split.

So, you add TdxHelperMeasureTdHob (doing the measurement part of
MeasureHobList) and TdxHelperMeasureCfvImage (likewise doing the
measurement part of MeasureFvImage) and logging both is handled by
TdxHelperBuildGuidHobForTdxMeasurement().

So I think the series should have:

  (1) One or more patches doing cleanups (like reusing the struct).
  (2) A patch removing MeasureHobList and adding TdxHelperMeasureTdHob
      with the first half of TdxHelperBuildGuidHobForTdxMeasurement
  (3) A patch removing MeasureFvImage and adding TdxHelperMeasureCfvImage
      with the second half of TdxHelperBuildGuidHobForTdxMeasurement
  (4) A patch moving the code from PlatformInitLib to TdxHelperLib.
  (5) A patch moving the calls to TdxHelperMeasureTdHob and
      TdxHelperMeasureCfvImage to SEC.
  (6) A patch adding the Tdxhelper* calls to OvmfPkgX64.

What is the reason to create a new TdxHelperLib btw.?
Are there any problems with the code being in PlatformInitLib?

take care,
  Gerd