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

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

You are right. My statement for page table is wrong. Both TDX and SEV need them.

That is NOT our original design. But I can understand why it is changed today.

I compare https://github.com/tianocore/edk2-staging/blob/TDVF/OvmfPkg/ResetVector/X64/TdxMetadata.asm and https://github.com/AMDESE/ovmf/blob/snp-v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm.

There are 8 entries in TDX, and 10 entries in SEV.
2 of them are same, page table and TEMP RAM.
6 entries are TDX unique. 8 entries are SEV unique.

I still feel it is burden to merge them, because some attributes field is NOT required for SEV but needed for TDX.
And TDX parsing tool need rule out SEV entries, and SEV parsing tool need rule out TDX entries.



Thank you
Yao Jiewen

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, September 24, 2021 5:34 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; brijesh.singh@amd.com; 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,
> 
> > > > Type: OVMF_SECTION_TYPE_SEC_MEM also seems for SEV. TDX does not
> > > need this special memory, such as Page table. It is already covered by code.
> > >
> > > These are "needs pre-validation / pre-acceptance" regions.
> > > TDX surely needs that too.
> > I don't think TDX need this. The page table should be covered by CODE already.
> 
> I think you are wrong here, the patch has this ...
> 
> +_OvmfPageTable:
> +  DD 0
> +  DD 0
> +  DQ OVMF_PAGE_TABLE_BASE
> +  DQ OVMF_PAGE_TABLE_SIZE
> +  DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
> +  DD 0
> 
> ... and a few simliar entries.
> 
> > > > I really cannot see the benefit to merge into one table.
> > >
> > > Keep reset vector small?
> > > Have common parser structs and code?
> >
> > I think it is opposite. This proposal makes reset vector larger, if we
> > need define more structure to satisfy TDX, but it is not needed by
> > SEV.
> 
> The sev and tdx specific entries will be there anyway, no matter
> whenever we place them into one or two separate tables.
> 
> Shared items like the page table memory will be there only once
> when we use a unified table, but twice with two separate tables.
> 
> take care,
>   Gerd