Very good suggestion. Thanks Laszlo. For 3), Min Xu and I will be the reviewer for Intel TDX change for OVMF.
For 6), agree. Although there is some architecture difference, e.g, AMD using PSP - a co-processor while Intel using TDX module - a new CPU execution mode, we should align as much as possible between Intel TDX and AMD SEV, especially for pure software architecture. I will be the Intel reviewer for confidential computing topic. Welcome AMD/IBM/... having a representative too. Min and I will sync and submit the patch for maintainer.txt Thank you Yao Jiewen > -----Original Message----- > From: Laszlo Ersek <[email protected]> > Sent: Tuesday, March 9, 2021 9:06 PM > To: Xu, Min M <[email protected]>; [email protected] > Cc: Liming Gao <[email protected]>; Liu, Zhiguang > <[email protected]>; Justen, Jordan L <[email protected]>; Yao, > Jiewen <[email protected]>; Tom Lendacky <[email protected]>; > Brijesh Singh <[email protected]>; James Bottomley > <[email protected]>; Tobin Feldman-Fitzthum <[email protected]>; Dov Murik > <[email protected]>; Dr. David Alan Gilbert <[email protected]> > Subject: Re: [PATCH V3 0/3] Add TdxLib support for Intel TDX > > On 03/09/21 13:57, Laszlo Ersek wrote: > > On 03/09/21 07:12, Min Xu wrote: > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249 > >> > >> The patch series provides lib support for Intel Trust Domain Extensions > >> (Intel TDX). > >> > >> 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. > >> > >> The Intel TDX module uses the instruction-set architecture for Intel TDX > >> and the MKTME engine in the SOC to help serve as an intermediary between > >> the host VMM and the guest TD. TDCALL is the instruction which allows TD > >> guest privileged software to make a call for service into an underlying > >> TDX-module. > >> > >> TdxLib is created with functions to perform the related Tdx operation. > >> This includes functions for: > >> - TdCall : to cause a VM exit to the Intel TDX module > >> - TdVmCall : it is a leaf function 0 for TDCALL > >> - TdVmCallCpuid : enable the TD guest to request VMM to emulate CPUID > >> - TdReport : to retrieve TDREPORT_STRUCT > >> - TdAcceptPages : to accept pending private pages > >> - TdExtendRtmr : to extend one of the RTMR registers > >> > >> The base function in MdePkg will not do anything and will return an error > >> if a return value is required. It is expected that other packages > >> (like OvmfPkg) will create a version of the library to fully support a TD > >> guest. > >> > >> We create an OVMF version of this library to begin the process of providing > >> full support of TDX in OVMF. > >> > >> To support the emulation and test purpose, 2 PCDs are added in OvmfPkg.dec > >> - PcdUseTdxAcceptPage > >> Indicate whether TdCall(AcceptPage) is used. > >> - PcdUseTdxEmulation > >> Indicate whether TdxEmulation is used. > > > > (1) per Jiewen's feedback, please drop these PCDs -- importantly, please > > drop DB-encoded instructions in assembly source code > > > > (2) It's not really helpful to post three versions of a patch set over > > the course of a few hours. I don't suggest posting more frequently than > > once per day, unless agreed otherwise. > > > > (3) Please add a new section to Maintainers.txt for TDX content in > > OvmfPkg. At least two Intel developers should be listed there as > > Reviewers. I'd like to permanently delegate TDX reviews to Intel > > contributors. > > > > See also the "OvmfPkg: SEV-related modules" section in "Maintainers.txt". > > > > (4) The patches contain numerous style issues: > > > > - overlong lines, > > > > - incomplete "@retval" comments, > > > > - Library #include directives mixed with non-library #include directives, > > > > - variables that should be STATIC but are not declared like that, > > > > - whitespace errors: missing space character between function designator > > (or macro name) and opening paren > > > > - more whitespace errors: missing space characters around "if" and > > "else" keywords > > > > (5) Some of the source files have outdated license blocks (e.g., > > open-coding the 2-clause BSDL and stating a copyright year of 2020, > > rather than stating 2021 and using "SPDX-License-Identifier: > > BSD-2-Clause-Patent") > > > > Please go over the patches with a fine-toothed comb and refresh them. > > > > (6) It would be nice if SEV-related patch sets and TDX-related patch > > sets were cross-CC'd between AMD and Intel contributors. (With the > > intent being code reuse, and perhaps "design reuse".) > > > > Maybe we should have an additional "confidential computing" reviewers > > section in "Maintainers.txt", covering both SEV and TDX modules. This > > would allow for a wider set of CC's, without obscuring who should review > > TDX vs. who should review SEV. I think this unified section should list > > a number of IBM developers too. > > (7) Some more admin stuff: > > (7a) every patch in this series should carry the following line in the > commit message: > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3249 > > (7b) whenever you post a new version of the patch set, please add a new > comment to <https://bugzilla.tianocore.org/show_bug.cgi?id=3249>, > linking the just-posted version (the cover letter email) from the > mailing list archive. > > This is important in case we want to review the evolution of the patch > series later. It's more difficult to find relevant email threads later > than to link each posting immediately in the bugzilla ticket. > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72598): https://edk2.groups.io/g/devel/message/72598 Mute This Topic: https://groups.io/mt/81195550/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
