Re: [PATCH V3 5/9] OvmfPkg/TdxHelperLib: Implement TdxHelperBuildGuidHobForTdxMeasurement

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

On Thu, Jan 26, 2023 at 07:09:55AM +0000, Xu, Min M wrote:
> On January 25, 2023 8:22 PM, Gerd Hoffmann wrote:
> > > +#define FV_HANDOFF_TABLE_DESC  "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> > XXXXXXXXXXXX)"
> > > +typedef PLATFORM_FIRMWARE_BLOB2_STRUCT
> > CFV_HANDOFF_TABLE_POINTERS2;
> > 
> > > -#define FV_HANDOFF_TABLE_DESC  "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> > XXXXXXXXXXXX)"
> > > -typedef struct {
> > > -  UINT8                   BlobDescriptionSize;
> > > -  UINT8                   BlobDescription[sizeof (FV_HANDOFF_TABLE_DESC)];
> > > -  EFI_PHYSICAL_ADDRESS    BlobBase;
> > > -  UINT64                  BlobLength;
> > > -} FV_HANDOFF_TABLE_POINTERS2;
> > 
> > That update makes sense, but can you please split this (and possibly
> > other) code changes to a separate patch so this patch does nothing but
> > moving code and the absolute necessary changes needed to make it work
> > (update library references, function renames).
> > 
> Some of the data struct and functions in TdxMeasurementHob.c are moved from PeilessStartupLib/IntelTdx.c
>  - TDX_HANDOFF_TABLE_POINTERS2
>  - CFV_HANDOFF_TABLE_POINTERS2
>  - GetFvName
> 
> BuildTdxMeasurementGuidHob is partially copied from TpmMeasureAndLogData@SecTpmMeasurementLibTdx.c.
> InternalBuildGuidHobForTdxMeasurement is newly added.
> 
> That is because in the previous PeilessStartupLib/IntelTdx.c the
> measurement uses the TpmMeasureAndLogData which is exported by
> SecTpmMeasurementLibTdx.c. It does not only measurement/extending, but
> also builds GuidHob of the measurement.
> 
> Now in this patch-set, the measurement/extending and the building of
> GuidHob are split.  Firstly HobList and CFV image are measured and
> extended (at the same time the measurement values are stored in
> WorkArea because at that time the Hob service is not ready).  Then
> after hob service is ready, GuidHob of the measurements are built.
> 
> So it's hard to separate the patch into the one that does nothing but
> moving code.

I think you can first do the code changes (split the functions so
measurement and GuidHob building can be done separately, but keep things
in IntelTdx.c), then move the code without functional changes to the new
places.

take care,
  Gerd