回复: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg (Config-A)
Min: I have one minor comment for TdxLib.h. This header file doesn't need to include below header files. Other patches in MdePkg are good to me. Reviewed-by: Liming Gao #include #include #include #include Thanks Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Min Xu > 发送时间: 2022年3月10日 14:21 > 收件人: devel@edk2.groups.io; Gao, Liming > 抄送: 'Brijesh Singh' ; Dong, Eric > ; Aktas, Erdem ; Wu, Hao > A ; Wang, Jian J ; 'James > Bottomley' ; Yao, Jiewen ; > Kinney, Michael D ; Ni, Ray ; > Kumar, Rahul1 ; 'Tom Lendacky' > ; Liu, Zhiguang ; > 'Gerd Hoffmann' > 主题: Re: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg > (Config-A) > > Hi, Lingming > Besides below 2 comments in MdePkg, what's your opinion about below > patches in MdePkg? > Patch 01 includes the Intel Trust Domain Extension definitions. > Patch 07-10 is about the BaseIoLibIntrinsic > Patch 12 add macros CC_GUEST_IS_SEV / CC_GUEST_IS_TDX to check SEV / > TDX guest. > > I am looking forward your comments about Patch 07 - 10. > > 01-MdePkg-Add-Tdx.h.patch > - https://edk2.groups.io/g/devel/message/87049 > 03-MdePkg-Add-TdxLib-to-wrap-Tdx-operations.patch > - https://edk2.groups.io/g/devel/message/87051 > > 07-MdePkg-Add-helper-functions-for-Tdx-guest-in-BaseIoL.patch > - https://edk2.groups.io/g/devel/message/87055 > 08-MdePkg-Support-mmio-for-Tdx-guest-in-BaseIoLibIntrin.patch > - https://edk2.groups.io/g/devel/message/87056 > 09-MdePkg-Support-IoFifo-for-Tdx-guest-in-BaseIoLibIntr.patch > - https://edk2.groups.io/g/devel/message/87057 > 10-MdePkg-Support-IoRead-IoWrite-for-Tdx-guest-in-BaseI.patch > - https://edk2.groups.io/g/devel/message/87058 > > 12-MdePkg-Add-macro-to-check-SEV-TDX-guest.patch > - https://edk2.groups.io/g/devel/message/87060 > > Thanks much! > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of > gaoliming > > Sent: Tuesday, March 1, 2022 10:20 AM > > To: devel@edk2.groups.io; Xu, Min M > > Cc: 'Brijesh Singh' ; Dong, Eric > > ; Aktas, Erdem ; Wu, Hao > A > > ; Wang, Jian J ; 'James > > Bottomley' ; Yao, Jiewen ; > > Kinney, Michael D ; Ni, Ray > ; > > Kumar, Rahul1 ; 'Tom Lendacky' > > ; Liu, Zhiguang ; > 'Gerd > > Hoffmann' > > Subject: 回复: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg > > (Config-A) > > > > Min: > > I have two comments in MdePkg. The changes in MdeModulePkg are > good to > > me. > > 1. Seemly, new APIs (TdCall, TdVmCall, TdIsEnabled) in BaseLib are X86 > specific. > > How about define them in #if defined (MDE_CPU_IA32) || defined > > (MDE_CPU_X64) in BaseLib.h? > > 2. I don't find new resource attribute > EFI_RESOURCE_ATTRIBUTE_ENCRYPTED in > > the latest PI PI_Spec_1_7_A_final_May1.pdf. Can you let me know which > spec > > defines it? > > > > Thanks > > Liming > > > -邮件原件- > > > 发件人: devel@edk2.groups.io 代表 Min Xu > > > 发送时间: 2022年2月28日 15:21 > > > 收件人: devel@edk2.groups.io > > > 抄送: Min Xu ; Brijesh Singh > > > ; Eric Dong ; Erdem > Aktas > > > ; Hao A Wu ; Jian J > Wang > > > ; James Bottomley ; > Jiewen > > > Yao ; Liming Gao ; > > > Michael D Kinney ; Ray Ni > > > ; Rahul Kumar ; Tom > Lendacky > > > ; Zhiguang Liu ; > Gerd > > > Hoffmann > > > 主题: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg > > > (Config-A) > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249 > > > > > > Intel's Trust Domain Extensions (Intel TDX) refers to an Intel > > > technology that extends Virtual Machines Extensions (VMX) and > > > Multi-Key Total Memory Encryption (MKTME) with a new kind of virutal > > > machines guest called a Trust Domain (TD). A TD is desinged to run in > > > a CPU mode that protects the confidentiality of TD memory contents and > > > the TD's CPU state from other software, including the hosting > > > Virtual-Machine Monitor (VMM), unless explicitly shared by the TD itself. > > > > > > There are 2 configurations for TDVF to upstream. See below link for > > > the definitions of the 2 configurations. > > > https://edk2.groups.io/g/devel/message/76367 > > > > > > This patch-set is to enable Config-A in OvmfPkg. > > > - Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align > > >with existing SEV) > > > - Threat model: VMM is NOT out of TCB. (We don’t make things worse.) > > > - The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic
回复: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg (Config-A)
Min: I have two comments in MdePkg. The changes in MdeModulePkg are good to me. 1. Seemly, new APIs (TdCall, TdVmCall, TdIsEnabled) in BaseLib are X86 specific. How about define them in #if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) in BaseLib.h? 2. I don't find new resource attribute EFI_RESOURCE_ATTRIBUTE_ENCRYPTED in the latest PI PI_Spec_1_7_A_final_May1.pdf. Can you let me know which spec defines it? Thanks Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Min Xu > 发送时间: 2022年2月28日 15:21 > 收件人: devel@edk2.groups.io > 抄送: Min Xu ; Brijesh Singh > ; Eric Dong ; Erdem Aktas > ; Hao A Wu ; Jian J Wang > ; James Bottomley ; Jiewen > Yao ; Liming Gao ; > Michael D Kinney ; Ray Ni ; > Rahul Kumar ; Tom Lendacky > ; Zhiguang Liu ; Gerd > Hoffmann > 主题: [edk2-devel] [PATCH V7 00/37] Enable Intel TDX in OvmfPkg (Config-A) > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249 > > Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology > that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory > Encryption (MKTME) with a new kind of virutal machines guest called a > Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the > confidentiality of TD memory contents and the TD's CPU state from other > software, including the hosting Virtual-Machine Monitor (VMM), unless > explicitly shared by the TD itself. > > There are 2 configurations for TDVF to upstream. See below link for > the definitions of the 2 configurations. > https://edk2.groups.io/g/devel/message/76367 > > This patch-set is to enable Config-A in OvmfPkg. > - Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align >with existing SEV) > - Threat model: VMM is NOT out of TCB. (We don’t make things worse.) > - The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic boot capability. >The final binary can run on SEV/TDX/normal OVMF > - No changes to existing OvmfPkgX64 image layout. > - No need to add additional security features if they do not exist today > - No need to remove features if they exist today. > - RTMR is not supported > - PEI phase is NOT skipped in either Td or Non-Td > > Patch 01 - 23 are changes in SEC phase. Also some libraries in these > patches are workable in SEC/PEI/DXE. > > Patch 17 - 20 extract the common codes from OvmfPkg/PlatformPei to a new > PlatformInitLib. Then OvmfPkg/PlatformPei is refactored with this lib. > This is because there are 3 variants of PlatformPei in OvmfPkg and hence > many codes are duplicated. > Patch 21 then add Tdx specific codes in PlatformInitLib. > > Patch 24 - 29 are changes in PEI phase. > > Patch 30 - 34 are changes in DXE phase. > > Patch 35 - 37 are for local Apic timer DXE driver. > > [TDX]: https://software.intel.com/content/dam/develop/external/us/en/ > documents/tdx-whitepaper-final9-17.pdf > > [TDX-Module]: https://software.intel.com/content/dam/develop/external/ > us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf > > [TDVF]: https://software.intel.com/content/dam/develop/external/us/en/ > documents/tdx-virtual-firmware-design-guide-rev-1.pdf > > [GCHI]: https://software.intel.com/content/dam/develop/external/us/en/ > documents/intel-tdx-guest-hypervisor-communication-interface-1.0-344426- > 002.pdf > > Code is at https://github.com/mxu9/edk2/tree/tdvf_wave2.v7 > > v7 changes: > - Based on the comments from last review, 8 PlatformInitLib patches >are squashed into 4 patches (#17-#20). These 4 patches are not >related to Tdx guest. Tdx related codes of PlatformInitLib is >in #21. > - gUefiOvmfPkgTdxPlatformGuid is renamed as > gUefiOvmfPkgPlatformInfoGuid. >Because this GUID is used not only by Tdx guest, but also by >Legacy guest. > - PlatformInitLibNull is deleted. > - In PlatformPei Pml4Entries is cap at 512 entries when >mPhysMemAddressWidth > 48. > > v7 not-addressed comments > - Comments in MpInitLib have not been addressed yet. It will be >addressed in the following version. > - Thanks much for your understanding. > > v6 changes: > - PlatformInitLib and OvmfPkg/PlatformPei refactoring are covered in >patch from 17 - 24. These patches are not related to Tdx guest. Tdx >related codes of PlatformInitLib is in patch 25. > - In the previous patch-sets, TdHob is processed in >OvmfPkg/Sec/IntelTdx.c. Per Gerd's suggestion they are now moved >to PlatformInitLib/IntelTdx.c. So that they can be reused in Config-B. > - The default Accept page size is changed from 4K to 2M. > - The BspAcceptMemoryResourceRange is refactored according to Gerd's >comment. > - In ApRunLoop.nasm command field is set to zero as acknowledgement. >This is a fix based on the ACPI Spec v6.4,Sec titled "Multiprocessor >Wakeup Structure". > > v6 not-addressed comments > - Comments in MpInitLib have not been addressed yet. It will be >addressed in the following version. > - Thanks much for your understanding. > > v5 changes: > -
