Re: [PATCH 06/10] OvmfPkg: Add TdxStartupLib

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

On December 15, 2021 6:09 PM, Gerd Hoffmann wrote:
> > +  PageTables = 0;
> > +  if (FeaturePcdGet (PcdDxeIplBuildPageTables)) {
> 
> Can this ever be false in a x64 build?
You're right. This cannot be false in X64 build. I will refactor  it in the next version.
> 
> > +EFI_STATUS
> > +FindDxeCore (
> > +  IN INTN                         FvInstance,
> > +  IN OUT     EFI_PEI_FILE_HANDLE  *FileHandle
> > +  )
> > +{
> 
> > +  if (FvInstance != -1) {
> > +    //
> > +    // Caller passed in a specific FV to try, so only try that one
> > +    //
> [ ... ]
> > +  } else {
> > +    // Assume the FV that contains the SEC (our code) also contains a
> compressed FV.
> 
> This branch never runs, FvInstance is explicity set to 1 by the caller.
Thanks for reminder. It will be fixed in the next version.
> 
> > +++ b/OvmfPkg/IntelTdx/TdxStartupLib/X64/PageTables.h
> 
> There is OvmfPkg/Include/IndustryStandard/PageTable.h
> 
> > +VOID
> > +Split2MPageTo4K (
> 
> Yet another copy of this.  Please don't.
I will figure out a Page walker lib so that these duplicated code can be avoided. Thanks for reminder.

Thanks
Min