Re: [PATCH V2 04/10] OvmfPkg/IntelTdx: Implement other helper functions in SecTdxHelperLib

["Min Xu" <min.m.xu@intel.com>]

On January 20, 2023 6:18 PM, Gerd Hoffmann wrote:
> On Fri, Jan 20, 2023 at 08:10:45AM +0000, Yao, Jiewen wrote:
> > > Can we define FV_HANDOFF_TABLE_POINTERS2 and
> FV_HANDOFF_TABLE_POINTERS2 in
> MdePkg/Include/IndustryStandard/UefiTcgPlatform.h?
> >
> > [Jiewen] No. We cannot move to MdePkg.
> > TCG defines the field to be variable length. Something like below:
> >
> > typedef struct {
> >   UINT8                      TableDescriptionSize;
> >   UINT8                      TableDescription[TableDescriptionSize];
> >   UINT64                     NumberOfTables;
> >   EFI_CONFIGURATION_TABLE    TableEntry[NumberOfTables];
> > } HANDOFF_TABLE_POINTERS2;
> >
> > typedef struct {
> >   UINT8                   BlobDescriptionSize;
> >   UINT8                   BlobDescription[BlobDescriptionSize];
> >   EFI_PHYSICAL_ADDRESS    BlobBase;
> >   UINT64                  BlobLength;
> > } HANDOFF_TABLE_POINTERS2;
> >
> > The implementation can choose its own length as they wish.
> 
> Why doesn't follow TDX standard TCG practices here?
> 
As Jiewen mentioned TCG defines the field to be variable length. The implementation can choose its own length. Below are some examples.
Tcg2Pei defines its FV_HANDOFF_TABLE_POINTERS2. (https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c#L126-L136)
SmbiosMeasurementDxe defines its SMBIOS_HANDOFF_TABLE_POINTERS2 (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c#L113-L123)
TcgEventLogRecordLib defines the PLATFORM_FIRMWARE_BLOB2_STRUCT and HANDOFF_TABLE_POINTERS2_STRUCT. https://github.com/tianocore/edk2/blob/master/SecurityPkg/Include/Library/TcgEventLogRecordLib.h#L14-L32

I think TDX follow the same practice above to define its own TDX_HANDOFF_TABLE_POINTERS2 and FV_HANDOFF_TABLE_POINTERS2. (FV_HANDOFF_TABLE_POINTERS2 happens to be same as the one in Tcg2Pei.) To make the definition more clear, TDX can define the name as CFV_HANDOFF_TABLE_POINTERS2.

@Gerd, Hoffmann what's your thought?

Thanks
Min