From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web08.1377.1622762398716325481 for ; Thu, 03 Jun 2021 16:19:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=itxAwvCY; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: jiewen.yao@intel.com) IronPort-SDR: AK5G0zryhU0HaSaZ/Pro1XOxCUoP4Huk0d4DaYtZvoeyq9ORpubqzvKrwNgQd5MEw/ZJth0dna Edx7XmSVN+BA== X-IronPort-AV: E=McAfee;i="6200,9189,10004"; a="225486678" X-IronPort-AV: E=Sophos;i="5.83,246,1616482800"; d="scan'208";a="225486678" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jun 2021 16:19:54 -0700 IronPort-SDR: 4/thKiM77TCpvfQ8B3j7b/uAY8JpqWF2W8fcIpdlo5jcruaeqoH5nzuCfiTi+pTP4FwNBVwTFU WnQIVnC1ASvA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.83,246,1616482800"; d="scan'208";a="483668120" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by fmsmga002.fm.intel.com with ESMTP; 03 Jun 2021 16:19:54 -0700 Received: from orsmsx605.amr.corp.intel.com (10.22.229.18) by ORSMSX606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4; Thu, 3 Jun 2021 16:19:53 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx605.amr.corp.intel.com (10.22.229.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4 via Frontend Transport; Thu, 3 Jun 2021 16:19:53 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.106) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.4; Thu, 3 Jun 2021 16:19:53 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TVe/uHM4twm7pn5ghSOm629b7whg+JFvJzzLkJ3F2YRpLdS/KK+GaIkrod3BgSvd4ET7G4tnuMJeNOtC5G7zPhp4dE0D5p6AP3hhD4JAwm2//cpnwnI/fTbDzBQVc3o/XoM0J8SJuQ45ErEj4ME+lpZX4r13mKYz8smRf0Tl8fTHV17gxd0i30okXGKJ9P4YvtOOPusNf6rpH0cQ/mNgHjl2spGvv8Hs83vpl9eQ2Kl2H8oaxJCpJWC/Cmaew00tiQ/C5eqc7zoPU3Dj59Yv8hRmj0EYNDP2O5X6/DnUm3Qzl4fiC+YnDYbzQR5FFxNuVqzdvI2uQ/swx79eONzOlw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ETgH+kttTqiOv5FSGGJPSf1VVomI5QkSEdXW4Qgqp6o=; b=VpggRPMOCfcDEP133DXrclLsy0ujNcyDFe1tbNrGQDw4JwZXLOBBgsaiWSMNx8KLVEAzU5yceEDsjPy6r7176NmvPt3r8+nI2CEJkBDiE0FzNxK+t26BsHeD5bRYGtd0cy1M5Mg5XxBJ5vktkQNeC1TVj0ljYOXRdm8TztJmnttFO14ERvoy35UvrEKVSHurjbgKO2AHwTohIq+jit7P8fjuopRegHskXmN+SwcYcqLFjQRRE7d8xh5JHpLu2r2XxvBfALKJAICs7lB4dA77lN6hDHdxlHbkPLsBJADIxxAROQkUpugFNAzUdHuzl4CWy1c30Ocoju0C0ozXs9IC5Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ETgH+kttTqiOv5FSGGJPSf1VVomI5QkSEdXW4Qgqp6o=; b=itxAwvCYoDEu940htevlYMY25rHLvvg3D+VwRPR5MG/l25sk6waznttXsvisCzwxCKwYcxlqxljB7jCVaFcUkq1+sUz+u+5VA2WkHvUz3T6KORyM5saNkJN9l0pHRxu4DofupuwlTCxJAtxsruaOnvJTuRMy/P3h99bxXQlXEa4= Received: from PH0PR11MB4885.namprd11.prod.outlook.com (2603:10b6:510:35::14) by PH0PR11MB4997.namprd11.prod.outlook.com (2603:10b6:510:31::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4195.20; Thu, 3 Jun 2021 23:19:51 +0000 Received: from PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::547d:4eb3:f37e:dac4]) by PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::547d:4eb3:f37e:dac4%9]) with mapi id 15.20.4195.024; Thu, 3 Jun 2021 23:19:51 +0000 From: "Yao, Jiewen" To: "rfc@edk2.groups.io" , "lersek@redhat.com" , "devel@edk2.groups.io" CC: "jejb@linux.ibm.com" , Brijesh Singh , Tom Lendacky , "erdemaktas@google.com" , "cho@microsoft.com" , "bret.barkelew@microsoft.com" , Jon Lange , Karen Noel , Paolo Bonzini , Nathaniel McCallum , "Dr. David Alan Gilbert" , Ademar de Souza Reis Jr. , "Yao, Jiewen" Subject: Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF Thread-Topic: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF Thread-Index: AddYf4DUPECuZ9ubQaOwhq0M0PgYbAAE6sKAAA50MlA= Date: Thu, 3 Jun 2021 23:19:51 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.5.1.3 dlp-product: dlpe-windows dlp-reaction: no-action authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=intel.com; x-originating-ip: [101.87.139.49] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 42785293-2459-4de7-3841-08d926e6168e x-ms-traffictypediagnostic: PH0PR11MB4997: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:612; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 6WRl4uSROEn3Dgshw4dc6LVnIZMLUKRKUvoD7pmaq9aCBV1nA6yE8lDod0IaJMm9+1kB7PyXn7MEF1/za85B3vPbyidPvgNFqzjz6R0qsYgdV71Zt5OSFjJtOvwAhHy9OrCJt0jMKfpwgm+lt9cB7pd7PHnktM93pP6jWppqp+AUor18KG3dFyilIjC8GDqKRwz3nvlPXHgSeylwFZQft7utpR2GZ07CMarP+fJnt+7yjOOCDTRmaOr5hZjduCGRbjlEU3NR1IJU0oBAol0gbks6WimCMF4RmYcwjmOV6uSW1ubzmvkowDJbKYbEMwqnWg5FlWOptSr5IXDexJ7u1iNNIr8xwpQzpN0vg5OzWjju1msi0TdjXm1qVIu/VWtT+JezxJgJzPPLSJxR5V3EEdVqt6CfSH1mYS+eYx+c0OgJG3LEciQtX0NK6opolgBxmd92c8AYhnIjJqu8fLhtwA05w3zJzA+h4LYXrCKxPbqjIfdA7ZST4eP29IxFdjM2oy/77cPxh1kY1nFVeVKukYB0HtYcJ9Q4nubQmFZ0pVUfA/1uIZOWuPqsJnbz/LOO8Xu4y+TLeag0Hg8vZLQysHuN9XBZ2YCAF1eGhWu6xN+MXHhbYcFvEK6tUKoHT6dAghbFWqcLcgpw5nzSXKbuaPi/6HYZQjjPMuB8VWapWZDzleZlw5mKK5XZTKfdKaeEXR8K1PQSNgS597uUlxQTCex81Hdus46Gu7ITJthMNc8m9r8ZvAqjsReF7OZInjSJUVac5xWHI6YzTPVRjNynJw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH0PR11MB4885.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(346002)(396003)(366004)(39860400002)(136003)(376002)(55016002)(83380400001)(30864003)(8676002)(186003)(478600001)(26005)(9686003)(107886003)(71200400001)(45080400002)(53546011)(6506007)(2906002)(7696005)(122000001)(76116006)(5660300002)(7416002)(4326008)(52536014)(86362001)(19627235002)(54906003)(66446008)(110136005)(66946007)(64756008)(66556008)(66476007)(316002)(8936002)(33656002)(38100700002)(966005)(559001)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?qZ2BsJjJM1gFUHZO62xrwaAqmfBDEIKFme9T1nd9wMV/R7JWR3Dc93ffYJnH?= =?us-ascii?Q?4sRz35uNMsI5TzSrn+mb1Rzr3KXsHAJGMX2GZWRvoGhkiAFeVSOwnH8RYz2F?= =?us-ascii?Q?QsCCUYqkZkOu+Kh/0i7MZ4VZHUHSGct11mG4ezWXXJQ7s8xIeXjtJmiZHryg?= =?us-ascii?Q?L0dB3JWiGlep1d9JnGVeI1nruFX45R8UXxnZaCqgrdfbO3soUXgOi7VEdgtv?= =?us-ascii?Q?DjWTT0qfNE9yq4/TK6+T62ir6x99otPwA0gJYEethyo1EqtMLbAmltuGTb8G?= =?us-ascii?Q?4blmGAqe9rUc5axhQ0Ipl6EvVvHRpUn+kLYVOoZUJVYU+ifn/C4OZfHc5U5H?= =?us-ascii?Q?zGMCErv8uy8av5o6iQ4oXuD9tLIX9A57sWuMwXvZ59QKBX8Y6CIyYlWpI4XC?= =?us-ascii?Q?y9dRWEAKr76Sqe24ruMqq57gGEtxeXBRs7v8eJJJCP5HYgvtvTaG1S4ertJF?= =?us-ascii?Q?ypaTIM7nb9DEV9jyIdV/0tdT6eTI7D3B1cpNKaZuT72hjZX9NRRjD3oPOBg9?= =?us-ascii?Q?CA5IR4B9VJxDf2jhVcmAbELARs4zaa+S2ktHqeHtTzUymQkPclx/MMfU1NZE?= =?us-ascii?Q?MD9SZfgGp+O2QHCg12szWR5eBEapiT8/afU62ohTslVmp6FR5h2omcoqFXiP?= =?us-ascii?Q?2WVXWsvfBawyvGzqQ/RPfGEa8k2qVcytWZvmiSjSpetyKxXuGi2Woj4/1QZV?= =?us-ascii?Q?SDJtGzJmG/MK9WUbmRAxKt1cUL7nIV9OWHUQVW+C8++s4VsMi2BaRKeLz/dm?= =?us-ascii?Q?OphLT6vEyzVE95EgptyJR4XX7TkpQQmwXdL4LnF3alDxEeLStgNOzWzHJaGc?= =?us-ascii?Q?n5kwlY6WwRX8iP3/7nxOVTwPR96ofDiYXJmwBKhKFZkHOnERF6IGJoj3atF1?= =?us-ascii?Q?8G1DKc1Y/jvKeDIFw3BvZXJc9aVJhT0CgRMBMC0UoKgK8ob5hDf1nSU2414m?= =?us-ascii?Q?Hbly9JlgRda28/opcg5OktGpG4/hJsDVxhSrWmQapj35NfR1ekDE8cld5QKd?= =?us-ascii?Q?QPfVEkxvphTh7eLvEUuAz3kUcfYHmazPnJESRG0Dnn37n4MuBGUsqhVnoUn9?= =?us-ascii?Q?auA8Gwq2Q/NjYx8XMYD4I52EOl7iBIh3X9r48t8cy2sFH+4oX0gt1G3GdoTW?= =?us-ascii?Q?2HfF+CsI8LdQFYBY1NBGmvGY3R+3R2CUixj7kTj5QdDjr5Z/WtYKpJDseYyO?= =?us-ascii?Q?T5wFPIPDHsGXWXE3O32MaoALiSFb14chk/R/O7ampOyjIpXkA+90+vQuuKX4?= =?us-ascii?Q?iCn7wE9G5fRYenq0TDCtIPnvxXFtjHkcdYQJM3L9cEwCFfPzw6PlDZT6cTqZ?= =?us-ascii?Q?0EBTcytz3oAiA+5mS88kLw5o?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4885.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 42785293-2459-4de7-3841-08d926e6168e X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2021 23:19:51.1243 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: fsHKzzN7Rvq3o6ALO5QtM52CNifHZGbpX2nVDNC51rfkhiPf3oD0ke8gNcKlgyV2AfEGfu355Slm9xEG7xuv6A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4997 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo. To clarify your "one binary" feedback below, do you mean you suggest A) cr= eate a separate DSC (for example OvmfPkg/ConfidentialComputing.dsc) for a f= ull solution including AMD SEC + Intel TDX + NonConfidentialComputing? Or B) to create a standalone DSC for Intel TDX (for example, create a Ovmf= Pkg/IntelTdx/IntelTdxXS64.dsc) ? To me, A) does not change too much, we just create another DSC file - that= is it. Then the original OvmfPkgX64.dsc will only be used for POC/Testing purpose= . It does not provide any security guarantee. (The threat model is: we don't trust VMM. Without attestation, you don't k= now if VMM modified the OVMF.) I don't know how "simply" it means. To enable TDX to make it work is not a= simple work. Some architecture changes are mandatory, such as reset vector, IO/MMIO acc= ess, #VE handler, IOMMU based shared memory access, etc. I think AMD SEV al= ready did those. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D My point is that the "one binary" that the slide deck refers to (i.e., OvmfPkgX64.dsc) might prove OK for utilizing the Intel TDX *technology* in itself. Simply enabling OVMF + a guest OS to boot in a TDX domain. But "OvmfPkgX64.dsc" is *not* the "one binary" that is suitable for remote attestation, which has a much broader scope, involving multiple computers, networking, deployment, guest-owner/host-owner separation, whatever. For the latter, we needed a separate platform anyway, even with only SEV in mind; that's why "OvmfPkg/AmdSev/AmdSevX64.dsc" exists. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Thank you Yao Jiewen > -----Original Message----- > From: rfc@edk2.groups.io On Behalf Of Laszlo Ersek > Sent: Friday, June 4, 2021 12:12 AM > To: Yao, Jiewen ; rfc@edk2.groups.io; > devel@edk2.groups.io > Cc: jejb@linux.ibm.com; Brijesh Singh ; Tom > Lendacky ; erdemaktas@google.com; > cho@microsoft.com; bret.barkelew@microsoft.com; Jon Lange > ; Karen Noel ; Paolo Bonzini > ; Nathaniel McCallum ; > Dr. David Alan Gilbert ; Ademar de Souza Reis Jr. > > Subject: Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF >=20 > On 06/03/21 15:51, Yao, Jiewen wrote: > > Hi, All > > We plan to do a design review for TDVF in OVMF package. > > > > > > The TDVF Design slides for TinaoCore Design Review Meeting (Jun 11) is > > now available in blow link: > > https://edk2.groups.io/g/devel/files/Designs/2021/0611. > > > > The Bugzilla is https://bugzilla.tianocore.org/show_bug.cgi?id=3D3429 > > > > > > > > You can have an offline review first. You comments will be warmly > > welcomed and we will continuously update the slides based on the > > feedbacks. >=20 > Resending my earlier comments in this mailing list thread, with the > feedback inserted at the proper spots that has been given in the > off-list thread since then: >=20 >=20 > *** Slides 4, 6, 7: the "one binary requirement". >=20 > (1) The presentation refers to "OvmfPkgX64.dsc" as "the one" on slide#4, > but then the explanation for the requirement, given on slide 7, speaks > about "common attestation interface". >=20 > I think we have a misunderstanding here. The "OvmfPkgX64.dsc" platform > indeed contains SEV, SEV-ES, and (in the future, will contain) SEV-SNP > support. In that sense, adding TDX support to the same platform should > be (hopefully) possible, at the cost of ugly gymnastics in the reset > vector. >=20 > But "OvmfPkgX64.dsc" is *already* different from the remotely attested > OVMF platform, namely "OvmfPkg/AmdSev/AmdSevX64.dsc". >=20 > The latter has some additional modules (secret PEIM and DXE driver), it > has the Grub binary built in, and -- probably most importantly -- it > trusts host-originated information less than "OvmfPkgX64.dsc". >=20 > For example, "OvmfPkg/AmdSev/AmdSevX64.dsc" has a dedicated > PlatformBootManagerLib instance, one that ignores the non-volatile UEFI > variables Boot#### and BootOrder, and ignores (thus far) the fw_cfg > originated kernel/initrd/cmdline as well. >=20 > It remains an "area of research" to see what else should be removed from > the traditional host-guest integration (which integration is usually > desirable for management and convenience), in the remotely-attested boot > scenario. See e.g. > . >=20 > My point is that the "one binary" that the slide deck refers to (i.e., > OvmfPkgX64.dsc) might prove OK for utilizing the Intel TDX *technology* > in itself. Simply enabling OVMF + a guest OS to boot in a TDX domain. >=20 > But "OvmfPkgX64.dsc" is *not* the "one binary" that is suitable for > remote attestation, which has a much broader scope, involving multiple > computers, networking, deployment, guest-owner/host-owner separation, > whatever. For the latter, we needed a separate platform anyway, even > with only SEV in mind; that's why "OvmfPkg/AmdSev/AmdSevX64.dsc" > exists. >=20 >=20 > *** Slides 8-9: general boot flow -- TDVF; TDVF Flow >=20 > I'm likely missing a big bunch of background here, so just a few > questions: >=20 > (2) Not sure what RTMR is, but it's associated with "Enable TrustedBoot" > -- so is a virtual TPM a hard requirement? >=20 > ... Back from slide 10: "TCG measurement and event log framework w/o > TPM" -- that's curious. >=20 > [response from Dave Gilbert:] > > My reading of this is that the RTMR (and another set of similar > > registers) are a TDX thing that is like the PCRs from a TPM but > > without the rest of the TPM features; so you can do the one-way > > measurement into the RTMRs just like you do into a TPM PCR, and the > > measurements pop out somewhere in the TDX quote. Just like a TPM you > > need the event log to make any sense of how the final hashed value > > supposedly got to where it did. >=20 > [response from Erdem Aktas:] > > +1 to David on this. TDX provides 2 kinds of measurement registers: > > MRTDs and RTMRs > > > (https://software.intel.com/content/dam/develop/external/us/en/docume > nts/tdx-module-1eas-v0.85.039.pdf > > section 10.1.2) . MRTDs are build-time measurement registers which are > > updated when TDX is being created. Once TDX is finalized (before the > > first run), the MRTDs are finalized and cannot be updated anymore. On > > the other hand, while the TD is running, TD can extend RTMRs through > > TDCALLs which will provide TPM PCR kind of capabilities. >=20 > ... Back from slide 43: feel free to skip this now; I will comment in > more detail below. >=20 > (3) Prepare AcpiTable -- OVMF fetches ACPI tables from QEMU; is this a > new (firmware originated) table that is installed in addition, or does > it replace QEMU's tables? >=20 > ... Ignore for now, will comment on the MADT stuff later. >=20 > (4) Does DMA management mean a different IOMMU protocol? That is > going > to conflict with the SEV IOMMU protocol. Depexes in OVMF expect one or > zero IOMMU protocols to be present. >=20 > ... Back from slide 40: feel free to skip this now; I'll comment on this > separately, below. >=20 > (5) Enumerate VirtIO -- virtio enumeration is PCI based on x86. But I > see no mention of PCI. If you mean VirtioMmioDeviceLib, that's no good, > because it does not support virtio-1.0, only virtio-0.9.5. >=20 > ... Back from slide 42: I got my answer to this on slide 42, so don't > worry about this point. >=20 > (6) The PEI phase is skipped as a whole. I don't see how that can be > reasonably brought together with either "OvmfPkgX64.dsc" or > "OvmfPkg/AmdSev/AmdSevX64.dsc". I guess you can always modify SEC to > jump into DXE directly, but then why keep the PEI core and a bunch of > PEIMs in the firmware binary? >=20 > Also touched on by slide 9, TDVF Flow -- PEI is eliminated but SEC > becomes more heavy-weight. >=20 > Wouldn't this deserve a dedicated, separate platform DSC? The > 8-bit/32-bit branching at the front of the reset vector is a smaller > complication in comparison. >=20 > Slide 6 references the mailing list archive: >=20 > https://edk2.groups.io/g/devel/topic/81969494#74319 >=20 > and in that message, I wrote: >=20 > I'm doubtful that this is a unique problem ("just fix the reset > vector") the likes of which will supposedly never return during the > integration of SEV and TDX >=20 > See also: >=20 > https://listman.redhat.com/archives/edk2-devel-archive/2021- > April/msg00784.html >=20 > where I said: >=20 > It's not lost on me that we're talking about ~3 instructions. >=20 > Let's keep a close eye on further "polymorphisms" that would require > hacks. >=20 > The first 9 slides in the presentation introduce much-much more > intrusive problems than the "polymorphism" of the reset vector. Would I > be correct to say that my concern in the above messages was right? I > think I was only given a fraction of the necessary information when I > was asked "do you agree 'one binary' is viable". >=20 > [response from Erdem Aktas:] > > Let's not worry about this for now. We want the one binary solution > > for practical reasons and also for simplicity. In the end, we want to > > do what is right and good for everyone. > > Those are legit concerns and I think what Intel is trying to do (sorry > > for mind reading) is to discuss all those concerns and questions to > > make the right decision. I really appreciate their effort on preparing > > those slides and bringing it to the community to review. > > > > I will also read your comments more carefully and provide my thoughts > > on them. > > Sorry for being a little slow on this. >=20 >=20 > *** Slide 10 -- Key impact to firmware >=20 > (7) "CPUs are left running in long mode after exiting firmware" -- what > kind of "pen" are we talking about? Does a HLT loop still work? >=20 > (8) "I/O, MMIO, MSR accesses are different" -- those are implemented by > low-level libraries in edk2; how can they be configured dynamically? >=20 > ... Back from slide 53: I'll comment on slide 53 separately; ignore > this. >=20 >=20 > *** Slide 11 -- TDVF Image (1) >=20 > (9) CFV -- Configuration Firmware Volume (VarStore.fdf.inc), > containing SB keys -- how is this firmware volume populated (at build > time)? Is this a hexdump? >=20 > ... Back from slide 16: it seems like CFV is a raw hexdump indeed; how > is that managed when keys change (at build time)? >=20 > (10) This slide (slide 11) basically describes an intrusive > reorganization of "OvmfPkgX64.fdf". I don't think I can agree to that. > While confidential computing is important, it is not relevant for many > users. Even if we don't cause outright regressions for existent setups, > the maintenance cost of the traditional OVMF platform will skyrocket. >=20 > The big bunch of areas that SEV-ES introduced to MEMFD is already a big > complication. I'd feel much better if we could isolate all that to a > dedicated "remote attested boot" firmware platform, and not risk the > functionality and maintenance of the traditional platform. I think this > ties in with my comment (1). >=20 > For example, seeing a configuration firmware volume (CFV) with secure > boot keys embedded, in the "usual" FDF, will confuse lots of people, me > included. In the traditional OVMF use case, we use a different method: > namely OvmfPkg/EnrollDefaultKeys, for "priming" a variable store > template, in the build environment. >=20 > Edk2 (and PI and UEFI) are definitely flexible enough for accommodating > TDX, but the existent, traditional OVMF platforms are a bad fit. In my > opinion. >=20 >=20 > *** Slide 12: TDVF Image (2) >=20 > (11) "Page table should support both 4-level and 5-level page table" >=20 > As a general development strategy, I would suggest building TDX support > in small, well-isolated layers. 5-level paging is not enabled (has never > been tested, to my knowledge) with OVMF on QEMU/KVM, regardless of > confidential computing, for starters. If 5-level paging is a strict > requirement for TDX, then it arguably needs to be implemented > independently of TDX, at first. So that the common edk2 architecture be > at least testable on QEMU/KVM with 5-level paging enabled. >=20 >=20 > *** Slide 13: >=20 > (12) My comment is that the GUID-ed structure chain already starts at a > fixed GPA (the "AMD SEV option"). Ordering between GUID-ed structures is > irrelevant, so any TDX-specific structures should be eligible for > appending to, prepending to, or intermixing with, other (possibly > SEV-specific) structures. There need not be a separate entry point, just > different GUIDS. >=20 > (13) Regarding "4G-0x20[0x10] is OVMF AP reset vector (used in OVMF > implementation)" -- I think this is a typo: this "AP reset vector" is > *not* used in OVMF. To my knowledge, it is a vestige from the UefiCpuPkg > reset vector. In OVMF, APs are booted via MpInitLib (in multiple > firmware phases), using INIT-SIPI-SIPI, and the reset vector for the > APs, posited through those IPIs, is prepared in low RAM. >=20 >=20 > *** Slides 14 through 16: >=20 > I consider these TDVF firmware image internals, implementation details > -- do whatever you need to do, just don't interfere with existing > platforms / use cases. See my comment (10) above. >=20 >=20 > *** Slides 17-21: >=20 > (14) Again, a very big difference from traditional OVMF: APs never enter > SEC in traditional OVMF. I assume this new functionality is part of > TdxStartupLib (from slide 18) -- will there be a Null instance of that? >=20 > Last week I posted a 43-part patch series to edk2-devel, for separating > out the dynamic Xen enlightenments from the IA32, IA32X64, X64 > platforms, in favor of the dedicated OvmfXen platform. TDX seems to > bring in incomparably more complications than Xen, and the OvmfPkg > maintainers have found even the Xen complications troublesome in the > long term. >=20 > If I had had access to all this information when we first discussed "one > binary" on the mailing list, I'd have never agreed to "one binary". I'm > OK with attempting one firmware binary for "confidential computing", but > that "one platform" cannot be "OvmfPkgX64.dsc". >=20 > Even if I make a comparison with just the "technology" (not the > remotely-attested deployment) of SEV and SEV-ES, as it is included in > "OvmfPkgX64.dsc", TDX is hugely more complicated and intrusive than > that. SEV proved possible to integrate into existing modules, into the > existing boot flow, maybe through the addition of some new drivers (such > as a new IOMMU protocol implementation, and some "clever" depexes). > But > we never had to restructure the FD layout, eliminate whole firmware > phases, or think about multiprocessing in the reset vector or the SEC > phase. >=20 > In order to bring an example from the ARM world, please note that > platforms that use a PEI phase, and platforms that don't, are distinct > platforms. In ArmVirtPkg, two examples are ArmVirtQemu and > ArmVirtQemuKernel. The latter does not include the PEI Core. >=20 >=20 > *** Slides 22 through 34: >=20 > (15) All these extra tasks and complications are perfectly fine, as long > as they exist peacefully *separately* from the traditional ("legacy") > OVMF platforms. >=20 > Honestly, in the virtual world, picking your firmware binary is easy. > The approach here reminds me of a physical firmware binary that includes > everything possible from "edk2-platforms/Features/Intel", just so it can > be deployed to any physical board imaginable. That's not how Intel > builds physical firmware, right? We have "edk2-platforms/Platform/Intel" > and "edk2-platforms/Silicon/Intel" with many-many separate DSC files. >=20 >=20 > *** Slide 35-36: DXE phase >=20 > (16) "Some DXE Drivers not allowed to load/start in Td guest -- Network > stack, RNG, ..." >=20 > Same comment as (several times) above. The Linuxboot project is a good > example for eliminating cruft from DXEFV (in fact, for eliminating most > of the DXE phase). In a TDX environment, why include drivers in the > firmware binary that are never used? Meanwhile, DXEFV in OVMF grows by > a > MB every 1.5 years or so. Again, remove these drivers from the DSC/FDF > then, and it needs to be a separate platform. >=20 > (17) "Other DXE Phase drivers -- [...] AcpiPlatformDxe" >=20 > I'm not sure what this section is supposed to mean. Other DXE phase > drivers included, or excluded? Without AcpiPlatformDxe, the guest OS > will not see QEMU's ACPI content, and will almost certainly malfunction. >=20 > ... Back from slide 48: ignore this for now, I'll comment in more detail > later. >=20 >=20 > *** Slide 37: DXE Core >=20 > (18) says "SMM is not supported in Td guest" -- how is the variable > store protected from direct hardware (pflash) access from the guest OS? > Without SMM, the guest OS need not go through gRT->SetVariable() to > update authenticated non-volatile UEFI variables, and that undermines > Secure Boot. >=20 > Note that, while SEV-ES has the same limitation wrt. SMM, the > "OvmfPkg/AmdSev/AmdSevX64.dsc" platform doesn't even include the > Secure > Boot firmware feature. For another example, the OVMF firmware binary in > RHEL that's going to support SEV-ES is such a build of "OvmfPkgX64.dsc" > that does not include the Secure Boot feature either. >=20 > But in this TDX slide deck, Secure Boot certificates are embedded in the > CFV (configuration firmware volume) -- see slide 11 and slide 16 --, > which suggests that this platform does want secure boot. >=20 > ... Back from slide 48: I'm going to make *additional* comments on this, > when I'm at slide 48, too. >=20 > The rest of this slide (slide 37) looks reasonable (generic DXE Core > changes -- possibly PI spec changes too). >=20 >=20 > *** Slides 38 through 39: >=20 > These seem reasonable (TdxDxe assumes some responsibilities of > OvmfPkg/PlatformPei) >=20 >=20 > *** Slides 40 through 42: >=20 > *If* you really can implement TDX support for the IOMMU driver *this* > surgically, then I'm OK with it. The general logic in the IOMMU driver > was truly difficult to write, and I'd be seriously concerned if those > parts would have to be modified. Customizing just the page > encryption/decryption primitives for TDX vs. SEV is OK. >=20 >=20 > *** Slides 43 through 47: >=20 > (19) Slide 46 and slide 47 are almost identical. Please consolidate them > into a single slide. >=20 > (20) the TPM2 infrastructure in edk2 is baroque (over-engineered), in my > opinion. It has so many layers that I can never keep them in mind. When > we added TPM support to OVMF, I required commit messages that would > help > us recall the layering. In particular, please refer to commit > 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module", 2018-03-09). Here's an > excerpt: >=20 > TPM 2 consumer driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance > | > v > TCG2 protocol interface > | > v > TCG2 protocol provider: Tcg2Dxe.inf driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibRouter instance > | > v > NULL class: Tpm2InstanceLibDTpm instance > (via earlier registration) > | > v > TPM2 chip (actual hardware) >=20 > The slide deck says that EFI_TD_PROTOCOL is supposed to reuse the > EFI_TCG2_PROTOCOL definition. If that's the case, then why don't we push > the TDX specifics (more or less: the replacement of PCRs with RTMR) down > to the lowest possible level? >=20 > Can we have "Tpm2InstanceLibTdxRtmr", plugged into the same Tcg2Dxe.inf > driver? >=20 > If not, can we have a new TdTcg2Dxe.inf driver, but make it so that it > install the same protocol as before (EFI_TCG2_PROTOCOL -- same old > protocol GUID)? Then DxeTpmMeasurementLib doesn't have to change. >=20 > As long as there is *at most* one EFI_TCG2_PROTOCOL instance published > in the protocol database, DxeTpmMeasurementLib should be fine. In SEV* > guests, the standard Tcg2Dxe driver provides that protocol. In TDX > guests, TdTcg2Dxe.inf should provide the protocol. Arbitration between > the two can be implemented with the pattern seen in the following > commits: >=20 > 1 05db0948cc60 EmbeddedPkg: introduce EDKII Platform Has ACPI GUID > 2 786f476323a6 EmbeddedPkg: introduce PlatformHasAcpiLib > 3 65a69b214840 EmbeddedPkg: introduce EDKII Platform Has Device Tree > GUID > 4 2558bfe3e907 ArmVirtPkg: add PlatformHasAcpiDtDxe >=20 > The basic idea is that Tcg2Dxe can have a depex on a new "running in > SEV" protocol GUID, and TdTcg2Dxe can have a depex on a new "running in > TDX" protocol GUID. A separate platform driver can install the proper > GUID -- possibly *neither* of those GUIDs. >=20 > And, we don't have to change the depex section of > "SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf" for this; we can implement a > library instance with an empty constructor, but a non-empty depex, and > then hook this lib instance into Tcg2Dxe.inf via module scope NULL lib > class override in the DSC file. Basically we could forcibly restrict > Tcg2Dxe's DEPEX by making it inherit the new DEPEX from the library. >=20 >=20 > *** Slide 48: DXE Phase -- Other Modules >=20 > Regarding IncompatiblePciDeviceSupportDxe: the proposed update sounds > plausible and simple enough. >=20 > (21) "AcpiPlatformDxe: Support for MADT/ACPI addition to report Td > Mailbox entry" >=20 > Firmware-owned tables must not be installed from this driver. >=20 > Please refer to my "Xen removal" patch set again, for > , which I mention > above in point (14). As part of the Xen removal, the AcpiPlatformDxe > driver in OvmfPkg is significantly trimmed: all unused (dead) cruft is > removed, including any ACPI table templates that are built into the > firmware. >=20 > OvmfPkg/AcpiPlatformDxe is responsible *solely* for implementing the > client side of the QEMU ACPI linker/loader. >=20 > If you need to prepare & install different ACPI tables, please do it > elsewhere, in another DXE driver. A bunch of other firmware modules do > that (NFIT, IBFT, BGRT, ...). >=20 > For example, the OvmfPkg/TdxDxe DXE_DRIVER is supposed to be launched > early in the DXE phase, via APRIORI section -- please consider > registering a protocol notify in that driver, for > EFI_ACPI_TABLE_PROTOCOL, and when it becomes available, install > whatever > *extra* tables you need. >=20 > Note that if you need to *customize* an ACPI table that QEMU already > provides, then you will have to modify the ACPI generator on the QEMU > side. It is a design tenet between QEMU and OVMF that OVMF include no > business logic for either parsing or fixing up ACPI tables that QEMU > provides. AcpiPlatformDxe contains the minimum (which is already a whole > lot, unfortunately) that's necessary for implementing the QEMU ACPI > linker/loader client in the UEFI environment. >=20 > The slide deck mentions MADT, which is also known as the "APIC" table -- > and indeed, QEMU does provide that. (See acpi_build_madt() > [hw/i386/acpi-common.c].) So if TDX needs MADT customizations, that > should go into QEMU. >=20 > (22) EmuVariableFvbRuntimeDxe >=20 > Ouch, this is an unpleasant surprise. >=20 > First, if you know for a fact that pflash is not part of the *board* in > any TDX setup, then pulling >=20 > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf >=20 > into the firmware platform is useless, as it is mutually exclusive with >=20 > OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf >=20 > (via dynamic means -- a dynamic PCD). >=20 > Note that the FDF file places QemuFlashFvbServicesRuntimeDxe in APRIORI > DXE when SMM_REQUIRE is FALSE. This driver checks for pflash presence, > and lets EmuVariableFvbRuntimeDxe perform its in-RAM flash emulation > only in case pflash is not found. >=20 > So this is again in favor of a separate platform -- if we know pflash is > never part of the board, then QemuFlashFvbServicesRuntimeDxe is never > needed, but you cannot remove it from the traditional DSC/FDF files. >=20 > Second, EmuVariableFvbRuntimeDxe consumes the PlatformFvbLib class, > for > the PlatformFvbDataWritten() API (among other things). This lib class is > implemented by two instances in OvmfPkg, PlatformFvbLibNull and > EmuVariableFvbLib. The latter instance allows Platform BDS to hook an > event (for signaling) via "PcdEmuVariableEvent" into the > EmuVariableFvbRuntimeDxe driver. >=20 > In old (very old) QEMU board configurations, namely those without > pflash, this (mis)feature is used by OVMF's PlatformBootManagerLib to > write out all variables to the EFI system partition in a regular file > called \NvVars, with the help of NvVarsFileLib, whenever > EmuVariableFvbRuntimeDxe writes out an emulated "flash" block. For this > purpose, the traditional OVMF DSC files link EmuVariableFvbLib into > EmuVariableFvbRuntimeDxe. >=20 > But it counts as an absolute disaster nowadays, and should not be > revived in any platform. If you don't have pflash in TDX guests, just > accept that you won't have non-volatile variables. And link > PlatformFvbLibNull into EmuVariableFvbRuntimeDxe. You're going to need > a > separate PlatformBootManagerLib instance anyway. >=20 > (We should have removed EmuVariableFvbRuntimeDxe a long time ago > from > the traditional OVMF platforms, i.e. made pflash a hard requirement, > even when SMM is not built into the platform -- but whenever I tried > that, Jordan always shot me down.) >=20 > My point is: using EmuVariableFvbRuntimeDxe in the TDX platform may be > defensible per se, but we must be very clear that it will never provide > a standards-conformant service for non-volatile UEFI variables, and we > must keep as much of the \NvVars mess out of EmuVariableFvbRuntimeDxe > as > possible. This will require a separate PlatformBootManagerLib instance > for TDX anyway (or maybe share PlatformBootManagerLibGrub with > "OvmfPkg/AmdSev/AmdSevX64.dsc"). >=20 > Apart from the volatility aspect, let's assume we have this in-RAM > emulated "flash device", storing authenticated UEFI variables for Secure > Boot purposes. And we don't have SMM. >=20 > What protects this in-RAM variable store from tampering by the guest OS? > It's all guest RAM only, after all. What provides the privilege barrier > between the guest firmware and the guest OS? >=20 >=20 > *** Slide 50: Library >=20 > (23) Should we introduce Null instances for all (or most) new lib > classes here? Code size is a concern (static linking). If we extend a > common OvmfPkg module with new hook points, it's one thing to return > from that hook point early *dynamically*, but it's even better (given > separate platforms) to allow the traditional platform firmware to use a > Null lib instance, to cut out all the dead code statically. >=20 >=20 > *** Slides 51 through 52 >=20 > Seems OK. >=20 >=20 > *** Slide 53: >=20 > (24) It might be worth noting that BaseIoLibIntrinsic already has some > SEV enlightenment, as the FIFO IO port operations (which normally use > the REP prefix) are not handled on SEV. I don't have an immediate idea > why this might matter, we should just minimize code duplication if > possible. >=20 >=20 > *** Slides 54-56: >=20 > No comments, this stuff seems reasonable. >=20 >=20 > *** Slide 57: MpInitLib >=20 > I don't know enough to give a summary judgement. >=20 >=20 > All in all, I see the controversial / messy parts in the platform > bringup, and how all that differs from the traditional ("legacy") OVMF > platforms. I admit I *may* be biased in favor of SEV, possibly because > SEV landed first -- if you find signs of such a bias in my comments, > please don't hesitate to debunk those points. Yet my general impression > is that the early bringup stuff is significantly different from > everything before, and because of this, a separate platform is > justified. >=20 > Definitely separate from the traditional OVMF IA32, IA32X64, and X64 > platforms, and *possibly* separate from the "remote attestation" > AmdSevX64.dsc platform. I would approach the TDX feature-set in complete > isolation (exactly how Intel commenced the work, if I understand > correctly), modulo obviously shareable / reusable parts, and then slowly > & gradually work on extracting / refactoring commonalities. >=20 > (But, given my stance on Xen for example, I could disagree even with the > latter, retroactive kind of unification -- it all boils down to shared > developer and user base. Component sharing should reflect the community > structure, otherwise maintenance will be a nightmare.) >=20 > Thanks > Laszlo >=20 >=20 >=20 >=20 >=20