Thanks Laszlo for copying me. From AMD, I will be soon start submitting the SNP support in the OMVF. I look forward collaborating with Yao and Min on software architecture.
On 3/9/21 6:25 PM, Yao, Jiewen wrote: > 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 <ler...@redhat.com> >> Sent: Tuesday, March 9, 2021 9:06 PM >> To: Xu, Min M <min.m...@intel.com>; devel@edk2.groups.io >> Cc: Liming Gao <gaolim...@byosoft.com.cn>; Liu, Zhiguang >> <zhiguang....@intel.com>; Justen, Jordan L <jordan.l.jus...@intel.com>; Yao, >> Jiewen <jiewen....@intel.com>; Tom Lendacky <thomas.lenda...@amd.com>; >> Brijesh Singh <brijesh.si...@amd.com>; James Bottomley >> <j...@linux.ibm.com>; Tobin Feldman-Fitzthum <to...@ibm.com>; Dov Murik >> <dov.mur...@il.ibm.com>; Dr. David Alan Gilbert <dgilb...@redhat.com> >> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3249&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cd28ff222c8714f55263008d8e35af722%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509327122407224%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lvpMxaXmLtXn8cn%2BLx2MMU9blA0kJrEyQe5IbOW4YJg%3D&reserved=0 >>>> >>>> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3249&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cd28ff222c8714f55263008d8e35af722%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509327122407224%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lvpMxaXmLtXn8cn%2BLx2MMU9blA0kJrEyQe5IbOW4YJg%3D&reserved=0 >> >> (7b) whenever you post a new version of the patch set, please add a new >> comment to >> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3249&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cd28ff222c8714f55263008d8e35af722%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509327122407224%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lvpMxaXmLtXn8cn%2BLx2MMU9blA0kJrEyQe5IbOW4YJg%3D&reserved=0>, >> 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 (#72633): https://edk2.groups.io/g/devel/message/72633 Mute This Topic: https://groups.io/mt/81195550/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
