From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web11.29018.1632737131312673536 for ; Mon, 27 Sep 2021 03:05:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=y/brKp73; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: jiewen.yao@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10119"; a="211694711" X-IronPort-AV: E=Sophos;i="5.85,326,1624345200"; d="scan'208";a="211694711" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2021 03:05:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,326,1624345200"; d="scan'208";a="518506879" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga008.fm.intel.com with ESMTP; 27 Sep 2021 03:05:22 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Mon, 27 Sep 2021 03:05:21 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Mon, 27 Sep 2021 03:05:21 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Mon, 27 Sep 2021 03:05:21 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.172) 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.12; Mon, 27 Sep 2021 03:05:21 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WfRdz5CZhopOV0BHiKpa3Jr2ObdlfAh/QpWyP8nQ+J/yw6xuadAqzgpYz8VV2dUTJ/7CZ/0h9hyiyukkgYLSqVPO0s+1BWWuM+OCM1IViA7gboPQLyd0xawhFhF7H0zIzbWmqMLV0aQQnKu4a+/bOZC9wp3cJSXQwTuLIjWLJ61T9K4c7f8Tvu8ipRAzqfagrXntPwuIhgT5Jo4CvdU+A/J059wNUEBWmIGynNUy0ciKPMlbfuQzD7HnppJk4HQvG2C1fg3ieRMKz7HrXkR6kfdT/fhsesa8uH2WIUTpwg+sftEA/GSpeylRoGIGAxZt2NdKNiMUPyEO7WmGhmV2Rw== 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; bh=Bu75W3JmdNkAiw5TMW1RVFL0iBymFGd6uRqJ+teYU3w=; b=aIBrmPwG/EIgEIqTTfDnLDgFZkOJ78TBjfYA1X7QVnNUJ9F0M1vWs3AqOxTRYIYzrfvXeZ1sc2g9EXE/nDjCH4lepjpYLHcmhH6Qlyeoo7fnu5bNQpgc3qhBDEzm3TgY5LSLKZRyFHs4ukVUMwu+TwN3olZC+7KSPuOidWQ/7oQB+7zr5I3CV4nSsYm7jctXD49Fpu5oDLtnolMuUFewT01GRV3HBQublZ078XHywgIP/Wb6eF+IaczH0yPT21N/zkfQ7rJCibHPZlaFejTxKyshfyam6fseGlKaQfQQecYAhvQi2aYKdZyooLVSi18E/VBgDDnOzZMLYa0SoyPhQw== 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=Bu75W3JmdNkAiw5TMW1RVFL0iBymFGd6uRqJ+teYU3w=; b=y/brKp73xK01qrWPv4pwjxsuaJ0tHW5ZxZS5A5KPZuRFOUg0J636u5543qzeitkr8qtftrI+obhx8nLNcBCalrq+/if0WwOqWl4wXLBs76cEl2OZ9YiUTtzPogtDUbyBpy/7YO5G9LaxZ4AQiVVBBhiPLXkvRNVNNaI92mn+e44= Received: from PH0PR11MB4885.namprd11.prod.outlook.com (2603:10b6:510:35::14) by PH0PR11MB4934.namprd11.prod.outlook.com (2603:10b6:510:30::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4544.13; Mon, 27 Sep 2021 10:05:18 +0000 Received: from PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::754e:42e9:16cd:1306]) by PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::754e:42e9:16cd:1306%6]) with mapi id 15.20.4544.021; Mon, 27 Sep 2021 10:05:18 +0000 From: "Yao, Jiewen" To: "devel@edk2.groups.io" , "kraxel@redhat.com" CC: "Xu, Min M" , Brijesh Singh , Ard Biesheuvel , "Justen, Jordan L" , Erdem Aktas , "James Bottomley" , Tom Lendacky Subject: Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector Thread-Topic: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector Thread-Index: AQHXrsfv6qwUC8Li5kiwlPdQeu+jg6uvry6AgAEZ5ACAAIjkgIAALkqAgAAWigCAAAcDAIAAA09wgAAI9ICAAAMxAIAA/y2AgAAYU4CAADWiAIAABQmAgAA8mYCAACMh4IAEMBkAgAAYt5A= Date: Mon, 27 Sep 2021 10:05:18 +0000 Message-ID: References: <20210924052825.2qljhtvweonbov5q@sirius.home.kraxel.org> <20210924100726.m33otbjod4fo3vur@sirius.home.kraxel.org> <20210924140221.6b3nk32eofwbwpgb@sirius.home.kraxel.org> <20210927080516.a64levdyyg6b4hne@sirius.home.kraxel.org> In-Reply-To: <20210927080516.a64levdyyg6b4hne@sirius.home.kraxel.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.6.200.16 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-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: cc92bdc4-df56-4f90-d11f-08d9819e4f8f x-ms-traffictypediagnostic: PH0PR11MB4934: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: GZFMKiLRWQgQTw/W6pYlynugepkWqApZ7TjEPu42COO4b/33wtgugaPlVuSYJ8xE2TD82iPQIp6n9I3nAmMrCXD6UD0zpU0bfHvskGJAaZJMmZ1bhGZ9nBMQIMyA0zW2FS2ryg1Nlcky3Ll7LtFyGjxksJXdZ89amAy3JWtb23488SYAUvWzd6/FtXCfDbbyNKvZgzq3dk25W4PTLEkdPZMKdL0d0+PDL2QxuVNxPmNWuFZdQwVWJsy31TqIXrXPtzf4wET599wh4Nfk4NJQGmmqESfKx/NfI4Vgzo6VH4bm2PyhsSHutkLpIsLAR9fu4EuXSoau7Ypg1UOlIzNKZQ+iNBuygFWZdv8zmKdHklQNmruw5IiHvZMqtU+S8ReyCyw8X4qKhM7v6Ha1KwjaQ6htLmT8hMEMIhTq8nCNkRl88M+erZ+SgUpUDswx6tVSs7ewQr6jGBaGoCXECw+wcmvEmCR7mu4QHCGQhZNzPIfCeaE3tXb4WAaCGZxi2vuJkLkKwUaJ2MtOVyKCMvEgj1rxB70erIwArwsx/npU0I9d5qb4XfRbZd49pO5TkAZGRNtDjBu+cJ8C1E1Tv1q2ifSebXr/1v3jiDt8Hzai7dvdmj03bfpldbiETMusP6ZCCCGxIUDFVnD2JZTXh5gZnRt+KN8HP9OwveK3Rq4bIrOF7eeT4AtNGSz8tQrECK/XqwGoPkkEMwPhhAq3Em84g5oP2clYc4p4RHJEdAW39fdAE4SHwwk5wmu+Zbn5lM0C3pRtV+LZDZ88unD2YQhVR9X4AdwRTK1N4ivTaA0V8p6sm1qRhXEWrRC1IXIOUOkqrM/3sUzWnliec9aVJT3x9Q== 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:(4636009)(366004)(38100700002)(966005)(66556008)(2906002)(9686003)(66446008)(55016002)(6506007)(86362001)(8936002)(8676002)(64756008)(5660300002)(33656002)(7696005)(4326008)(52536014)(316002)(53546011)(186003)(122000001)(26005)(71200400001)(508600001)(66946007)(38070700005)(76116006)(54906003)(66476007)(110136005)(83380400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?KwQbPpjOHIgfulO2fMWi49ttw950yZa8SZ1SyW2ruD9u9g6KdNWuV6wBQDHx?= =?us-ascii?Q?yPFwnvGh8tThTObnRz3n82mFRtrLcyu6RzxNZQHMKJMPdNH2gSugMwWB7lM8?= =?us-ascii?Q?o6VMcH9HfNsqaqDnTn+Fe7+6qSDNiNA65df4/sgiH0+kWgOSpupYTzpb21yk?= =?us-ascii?Q?+4EraKwt+dEfwzTds+jyTmWmL5vMY+7teN1egY0CI9OSnQ3qSbx51IsMkQDy?= =?us-ascii?Q?tKl11zVKEYSM0U0qzl3JCyvTutGUpiaGaP2wEa2yBQwSn0/9V77FhuE7c3tL?= =?us-ascii?Q?5s4fDPy0YAxwotX+Ls0wRuOtcZLuVOhUBia+e7PJ5BMTprrOS00MCQm9muoM?= =?us-ascii?Q?L86Jo3gyxNTaocVyspmuc8SwbcRqktJm9uEisotSb3yM+/My2M1Oe6Qt483U?= =?us-ascii?Q?mIJagufCtc48/IfUbRpcUYa+fzVpNzH7SNGCtyGTSkjGUR72hlPO00rphBmq?= =?us-ascii?Q?H+JRYJT4ZtHIXdD2XQQzhywnpH3/OCKwP4ENUh0hY53ziT9ClQTmIU7LbLO/?= =?us-ascii?Q?Nvx+LZ6VQE7DVWWUjgN/DgXWrHQBoHEWlX4BcphcGJV3voFwrx3YByPNmmFn?= =?us-ascii?Q?2yvpP0gPk8CoSm14IEsOyDCU+rky8W6vgpboOPQslTrjdojrFwm7sk04jSRK?= =?us-ascii?Q?ITj+ebWk42XOOCheGF9ChTXapoNqDR2aNBsp0d1KUNOh0YNIgXSK30i5bU2R?= =?us-ascii?Q?kkMzqVvH5n4bWLIZCQJx4le9+K+U8HpZudLqPGZ0VQ9jF8XIm3oPPqbqTNKI?= =?us-ascii?Q?E4rr/LH1epJOh4fm1RjHCSZ/S1U3i38S6tu0LUgpqil5OxWuTnd0zrOv2KCg?= =?us-ascii?Q?1/eMGZJEL1sCSv/805jz3qAMhUAr2yDiqkt1/gQyMXzRlef/Vt9kjnLjWLDx?= =?us-ascii?Q?lYf9nMFmD8Xsyn87u0cShKW9QXbj2Sb+qAxYvOKn9iPtAQSQrgLVn60WqcT4?= =?us-ascii?Q?lXhY1y3Yb/+W050GTBGYYKoyr+vM9u/u2bv6q+EvKhgrlcYA2TcajJQtTNLI?= =?us-ascii?Q?UXFaUSh7cRZkcwQC+Gem4vv7mQkl4VQ29kwFLDSGCabSOkUEYfaVWGMWyzOi?= =?us-ascii?Q?H2izfRIRPnbyNoQsqPFxfOustgxHVRD/GZLDS8Yk4/3nQX+vcHd9k9tsKMAf?= =?us-ascii?Q?tE9J+2uMew4OFhbTa4/2inCpLvPVcg9ePiZ/wBa9o9YBWsw4xiK5oFW0SZWj?= =?us-ascii?Q?08Eh9h8pv6ulQbTbGbRz62MXeheDrcuAF1H1oYo76B1hAMTw1XuMrjCofG4e?= =?us-ascii?Q?GAZHD3rrqWxQqrORVWhBJZvGVFuTdGN5O+LPNfGFbpcT99TyXa3IgUju3ogw?= =?us-ascii?Q?CVtvaRv4BN3giw64q3bgTs7F?= 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: cc92bdc4-df56-4f90-d11f-08d9819e4f8f X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Sep 2021 10:05:18.8271 (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: reNnT3GzjbvQ0OvXse9CvWaZlOMcdUoAH9DTzXKGMDQkICNT8hDazcobhCQ7yyD0H8tDoBvVnVX1dPmuzHKjcQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4934 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 Comment below: > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Gerd > Hoffmann > Sent: Monday, September 27, 2021 4:05 PM > To: Yao, Jiewen > Cc: devel@edk2.groups.io; Xu, Min M ; Brijesh Singh > ; Ard Biesheuvel ; > Justen, Jordan L ; Erdem Aktas > ; James Bottomley ; Tom > Lendacky > Subject: Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVect= or >=20 > Hi, >=20 > > > Possible alternative approach: Define an extension > > > (EFI_FIRMWARE_VOLUME_EXT_HEADER) for the load address and use that > > > instead of defining something new. > > > > [Jiewen] I would say it is terrible idea to use > EFI_FIRMWARE_VOLUME_HEADER. > > > > [ ... details snipped ... ] >=20 > Ok, lets scratch the idea then. >=20 > > > Still not clear why the size is in there twice. I think you could > > > instead use a flag telling whenever a section must be loaded from > > > the image or not. This is what COFF and ELF are doing too. > > > > > > Also not clear why you want stick to 64bit base address. Loading the > > > firmware above 4G isn't going to work without also changing a bunch o= f > > > other things and it will break backward compatibility anyway. > > > > [Jiewen] That is our previously experience, when we define a physical a= ddress, > we always use 64bit to leave room for extension. > > Like UEFI specification defines PHYSICAL_ADDRESS to be UINT64 even in I= A32 > platform. >=20 > Sure, physical address space on IA32 is 64G which simply doesn't fit into > UINT32 ... >=20 > > Defining UINT64 gives us enough flexibility for the future, including t= est in > above 4G environment. > > I am wondering that, do you really care to save 4 bytes from UINT64 to > UINT32 ? >=20 > Well, MEMFD isn't that big, so we have to care about the size there ... >=20 > > For type, maybe 2^16 is enough. But for flags, I prefer 32bit. >=20 > Making one 16bit and the other 32bit doesn't make much sense due > to struct padding and alignment. So with 64-bit load address and > fields reordered so we don't have any padding the struct could look > like this: >=20 > struct { > uint64_t load_address; > uint32_t file_offset; > uint32_t section_size; > uint32_t section_type; > uint32_t section_flags; > }; [Jiewen] This data structure does not work in a special use case - A TD may= want to have a fixed memory size. It is TD that tells the VMM how many DRAM should be allocated by using meta= data table. Not the case that a VMM tells the TD how many DRAM is allocated= by using HOB. In that special case, the TD_HOB is NOT required. The VMM parses the metada= ta to allocate the DRAM (AUG page). The runtime section size must be UINT64, otherwise we cannot support > 4G m= emory section. The build time section size can be UINT32. We don't expect to create a >4G = binary in near future. And we need both. I can understand why you think there is no needed fields, based upon what y= ou see in EDKII/TDVF project. However, the usage in current TDVF is just a subset, but not all usages. The TDX metadata structure is carefully designed to support those variants.= Also, leaving some room for future is a common practice. Besides EDKII/TDVF, we are doing other TDX related projects reusing the sam= e metadata structure. (But sorry, I cannot tell more at this time.) The benefit is that the KVM or cloud hypervisor can have a common logic to = handle "TDX boot", instead of using different table in different use cases. Defining something only feasible in EDKII/OVMF will bring a burden. That is= NOT a way we want to go. Let me say in this way: I think it is OK, if SEV wants to reuse the existing TDX metadata table. (W= e need SEV people agree.) Then we can have one metadata table.=20 But we cannot reuse the SEV defined metadata table in https://github.com/AM= DESE/ovmf/blob/snp-v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm, because it = missed some fields we need. If that is the case, then we have to use 2 meta= data tables. >=20 > > > But, again, I don't want have two completely different initialization > > > code paths in OVMF. We can certainly investigate and discuss droppin= g > > > PEI, but that clearly shouldn't be a TDX-only thing. When we elimina= te > > > PEI, we should do it for all OVMF builds. > > > > [Jiewen] I think this is out of scope of TDVF config-B patch series. > > I don't think it is fair to enable OVMF to remove PEI, just because > > TDVF does not need PEI. >=20 > Hmm? TDVF is based on OVMF. My expectation would be that TDVF is > basically OVMF with TDX support added and most code being identical. > So with TDVF being able to boot without PEI most of the work should > already be done ... >=20 > > Each platform owner can have their own choice. >=20 > Why do you consider TDVF a separate platform? I see it as OVMF variant. >=20 > Or did you just fork OVMF for config-b? Doing so is certainly easier > for initial development and prototyping, but it's bad in the long run. > Having similar code twice in the code base means more long-term > maintenance work, which isn't fair either. >=20 > > If you have intertest to remove PEI, I am happy to discuss with you on > > detail. However, I would treat "removing PEI from OVMF" and "enable > > TDVF config-B" be two different tasks. >=20 > Given TDVF wants boot without PEI the later depends on the former > though. Or TDVF config-B continues to use the PEI-based boot workflow > for now like config-A does, and we look into removing PEI from OVMF > later. [Jiewen] We don't fork OVMF in config-B. Instead of we will create new Tdvf/Tdvf.dsc and Tdvf/Tdvf.fdf in config-B, = similar to https://github.com/tianocore/edk2/tree/master/OvmfPkg/AmdSev or = https://github.com/tianocore/edk2/tree/master/OvmfPkg/Bhyve That is why I treat it as different platform. Non-PEI is not a new concept. It is quite mature known configuration. I reluctant to merge it back to Ovmf.dsc/fdf. The reason is the main Ovmf s= upports some features (such as S3, TPM) which may depend on PEI modules, bu= t it is NOT needed in TDVF. We need re-evaluate the effort to enable those features in non-PEI configur= ation in OVMF. - That is totally unnecessary in TDVF enabling task. That is why I treat them as separate task. And something the OVMF community= may look into in the future. >=20 > take care, > Gerd >=20 >=20 >=20 >=20 >=20