Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector

["Yao, Jiewen" <jiewen.yao@intel.com>]

Again. Two topics. We need discuss them separately.

Topic 1: TD metadata table is an architecture way to communicate with VMM.
We took the design from PE/COFF image section, which is flexible to support different binary format.
EDKII TDVF is just one possible producer. There could be other producer in the future. We don't want to define something only meet current TDVF need.
The ATTRIBUTE field are separated from TYPE field, because they are different thing. Currently, you may use TYPE to predict what ATTRIBUTE will be. But that is NOT always correct. We are adding more ATTRIBUTE for the future.

Topic 2: In config-B we remove PEI.
I think we should say it in different way: We add PEI back in config-A.
In our original design we totally eliminated PEI, because it is unnecessary. IMHO, it is totally an overdesign in OVMF to include PEI.
However, in order to be compatible with current OVMF and make "minimal" change for config-A, we add PEI back.

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Friday, September 24, 2021 6:07 PM
> To: Xu, Min M <min.m.xu@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; devel@edk2.groups.io; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Erdem Aktas <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector
> 
>   Hi,
> 
> > > The question why TDX_BFV_RAW_DATA_OFFSET and
> > > TDX_BFV_RAW_DATA_SIZE are needed and why TDX_BFV_MEMORY_BASE +
> > > TDX_BFV_MEMORY_SIZE can't be used is still open.
> > Here is a BFV metadata.
> >     37                                                           <1> _Bfv:
> >     38 00000140 00400800                     <1>   DD TDX_BFV_RAW_DATA_OFFSET
> >     39 00000144 00C03700                     <1>   DD TDX_BFV_RAW_DATA_SIZE
> >     40 00000148 0040C8FF00000000    <1>   DQ TDX_BFV_MEMORY_BASE
> >     41 00000150 00C0370000000000    <1>   DQ TDX_BFV_MEMORY_SIZE
> >     42 00000158 00000000                      <1>   DD
> TDX_METADATA_SECTION_TYPE_BFV
> >     43 0000015C 01000000                      <1>   DD
> TDX_METADATA_ATTRIBUTES_EXTENDMR
> >
> > TDX_BFV_RAW_DATA_OFFSET/TDX_BFV_RAW_DATA_SIZE indicates the
> offset/size of BFV in Ovmf.fd.
> > TDX_BFV_MEMORY_BASE/ TDX_BFV_MEMORY_SIZE is the physical memory
> address which is to be mapped.
> > TDX-QEMU use these information to 1) do the measurement of the BFV 2) map
> the BFV to the physical memory
> 
> TDX_BFV_RAW_DATA_SIZE + TDX_BFV_MEMORY_SIZE are identical.  Why do
> you
> need both?  Yes, I know, some entries have RAW_DATA_SIZE=0 because
> nothing is loaded for them.  You can also figure that by looking at the
> section type.
> 
> TDX_BFV_RAW_DATA_OFFSET can be calculated from
> TDX_BFV_MEMORY_BASE, it's
> a fixed offset (depending on firmware size).  At least as long as the
> firmware loading uses to the usual x86 workflow (place right below 4G).
> 
> Also: When placing the firmware below 4G MEMORY_BASE + MEMORY_SIZE can
> be DD.
> 
> The attribute field can be added to the ovmf meta data, or we make that
> part of the section type.
> 
> > *Config-A* enables a minimum functionality in OvmfPkgX64.dsc without
> > breaking existing functionality.
> 
> > *Config-B* is a separate platform  configuration file can be used to
> > provide all the security guarantees that TDX provides.
> 
> Why does config-b need a completely different initialization code path
> (i.e. skip PEI, see slide 11 of the tdvf design review)?
> 
> take care,
>   Gerd
> 
> 
> 
> 
>