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

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

On January 20, 2023 9:51 PM, Gerd Hoffmann wrote:
> 
> > > > [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/Libr
> > ary/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.)
> 
> Ok, that makes sense.  The TdHob is tdx-specific, measuring a firmware
> volume is not.  I'm still wondering why the structs for standard events (like
> the firmware volume) are not in some shared header file ...
> 
Hi, Gerd
Actually I tried to find some common header file to define the events (the firmware volume). But it seems there is no such header file. Let me check the code again and see if there is such header file.

Thanks
Min