Re: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector

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

[-- Attachment #1: Type: text/plain, Size: 6813 bytes --]

I propose to submit independent patch with independent metadata (2 tables) - don't complicate thing.

We can revisit to see if there is need to merge to 1 table or how to merge, as a separate/standalone patch.

Thank you
Yao Jiewen


From: Singh, Brijesh <brijesh.singh@amd.com>
Sent: Friday, October 1, 2021 1:40 AM
To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io; kraxel@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>
Cc: 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>; Lendacky, Thomas <Thomas.Lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector


[AMD Official Use Only]

Yes, I will try to make it work for the unified Metadata. Let's do it indepent of SNP and TDX series. You can pick the generic patch from my series and add the additional fields we need for the TDX and submit it.

Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>
Sent: Thursday, September 30, 2021 12:31:56 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; kraxel@redhat.com<mailto:kraxel@redhat.com> <kraxel@redhat.com<mailto:kraxel@redhat.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Erdem Aktas <erdemaktas@google.com<mailto:erdemaktas@google.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Lendacky, Thomas <Thomas.Lendacky@amd.com<mailto:Thomas.Lendacky@amd.com>>
Subject: RE: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector


[AMD Official Use Only]


Hi, Brijesh

In the current discussion there are 2 options for the metadata, a unified Metadata and 2 separate Metadata (SEV and TDX metadata).

My understanding to your last mail is that you're going to use the unified Metadata option, right?



As to the offset of metadata, absolute offset is a good idea. I will update it in my next version.



Thanks!

Min

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Brijesh Singh via groups.io
Sent: Tuesday, September 28, 2021 11:24 PM
To: Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; kraxel@redhat.com<mailto:kraxel@redhat.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Erdem Aktas <erdemaktas@google.com<mailto:erdemaktas@google.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Lendacky, Thomas <Thomas.Lendacky@amd.com<mailto:Thomas.Lendacky@amd.com>>
Subject: Re: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector



[AMD Official Use Only]



May I ask to use the OvmfMetadata instead of the of TdxMetadata for the Guided structure name label (same as what I did in SNP series patch #4). If you can send the metadata introduction as a patch separately then add the TDX descriptor in TDX series. I can try to make it work for the SNP series and add SNP specific descriptors. Additionally, I think you want to provide an absolute offset for the start of the metadata instead relative value so that VMM can very easily reach to the start of metadata.

e.g



+OvmfMetadataOffsetStart:
+  DD      (fourGigabytes - OvmfMetadataGuid - 16)
+  DW      OvmfMetadataOffsetEnd - OvmfMetadataOffsetStart
+  DB      0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+  DB      0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+OvmfMetadataOffsetEnd:



For SNP series, I will 3 section types #1 CPUID, # Secrets, and #3 SEC_MEM and will probably add a total of 3 more descriptors.



________________________________

From: Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>
Sent: Tuesday, September 28, 2021 2:35 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; kraxel@redhat.com<mailto:kraxel@redhat.com> <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Erdem Aktas <erdemaktas@google.com<mailto:erdemaktas@google.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Lendacky, Thomas <Thomas.Lendacky@amd.com<mailto:Thomas.Lendacky@amd.com>>
Subject: RE: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector



On September 28, 2021 12:43 PM, Gerd Hoffmann wrote:
>   Hi,
>
> > > Can you move the metadata changes to a separate patch please?
> > Yes, the metadata changes will be in a separate patch in the next version.
>
> Can you also add a comment block documenting the format?  Not only those
> parts which are used for TDVF, but everything?  The description in tdx-virtual-
> firmware-design-guide-rev-1.pdf seems to be incomplete, specifically the
> option to use the table for TD memory allocation (as mentioned by Jiewen) is
> not covered.  And possibly there is more which is missing ...
Sure. I will add the comment in IntelTdxMetadata.asm to describe the format of Tdx Metadata.
Here is the PR I would send as the next version. https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F2018&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cf49ea5bc7d79474e572108d982529cbd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637684113590273535%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bGOxYMIKtHYKhcfk0Wt4qoIgiz3b9DM%2FAD%2Fui3ByVrU%3D&amp;reserved=0<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F2018&data=04%7C01%7Cbrijesh.singh%40amd.com%7Ccd13cf923fe248bdb7f408d983e464f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637685839234430342%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TmaDGDDGeGXGlDwOnlEUeho8v4N0FC6yC%2F4UP%2BH1PJQ%3D&reserved=0>
You can have a preliminary review if you want.
>
> thanks,
>   Gerd
>
>
>
>
>



[-- Attachment #2: Type: text/html, Size: 13834 bytes --]