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

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

Comment below:

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, September 24, 2021 12:54 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io;
> 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
> 
> On Thu, Sep 23, 2021 at 01:38:52PM +0000, Yao, Jiewen wrote:
> > Good point, Min.
> >
> > If https://github.com/AMDESE/ovmf/blob/snp-
> v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm is the proposal, then I have
> more comment:
> >
> > Type: OVMF_SECTION_TYPE_CODE, OVMF_SECTION_TYPE_VARS are NOT
> used for SEV. I am not sure why they are there.
> 
> tdx needs them (for measurement).  It's not a tdx-specific concept,
> possibly sev-snp wants use that too in the future.
That means this is only for TDX. SEV does not need this type. Then this is TDX specific.

> 
> > Type: OVMF_SECTION_TYPE_CPUID should be SEV specific. TDX does not need
> CPUID page.
> 
> A cpuid page can be used without sev too.
I don't think TDX need this field. This is SEV specific.


> 
> > 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.


> 
> > Type: OVMF_SECTION_TYPE_SNP_SECRETS /
> OVMF_SECTION_TYPE_SNP_SEC_MEM is SEV specific.
> 
> Yes.
> 
> > The SEV table is totally different with TDX metadata table.
> 
> I can't see a fundamental difference.  In both cases the VMM needs
> to know the firmware memory layout for (a) attestation, and (b)
> pre-validating/pre-acceptance of memory, and (c) some
> hardware-specific ranges such as snp secrets page.
> 
> > 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. Or Define something purely for SEV, but not useful for TDX.
I don't treat it as benefit. Instead I think it is big burden.


> 
> take care,
>   Gerd