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.web10.7572.1632788489927006635 for ; Mon, 27 Sep 2021 17:21:30 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=FpEDQXIu; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: jiewen.yao@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10120"; a="211840537" X-IronPort-AV: E=Sophos;i="5.85,327,1624345200"; d="scan'208";a="211840537" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2021 17:21:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,328,1624345200"; d="scan'208";a="486332517" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by orsmga008.jf.intel.com with ESMTP; 27 Sep 2021 17:21:29 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx604.amr.corp.intel.com (10.18.126.84) 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 17:21:28 -0700 Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by fmsmsx611.amr.corp.intel.com (10.18.126.91) 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 17:21:28 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx602.amr.corp.intel.com (10.18.126.82) 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 17:21:28 -0700 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (104.47.57.176) by edgegateway.intel.com (192.55.55.70) 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 17:21:27 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KmGcCMv+gWFSTY9tw8aahGFXYma7RXat/K2KyPzvbH9WEybudJ24qh7zUv1B6Wsk91VTNDTpoqa9puDerQPMLtlu9tnGqItvPKukeJ3Ci9S5PyWKKbcImrrC7ReSdY4pKcOjzUFhZhUFUIUS3NkxsKirL3XlzsUui+lLFUCcN8tzp4HTSwHHm+5ZerXZQU1LKeg3ZLJw0ZXqmWiD3sHIOn4JYr/ROR0k+QThSdbZBtfpYLT6o6lN/saHWcwlr0Panep6S8zflzOrhgb3k1ELAZNXZjf8WCFfWhSK5tKVSaEzxakPFvKn4P6Nnn8DPVHD5OiMbnbMy9tefHgwBGaUfw== 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=EMLus+UICrUfqiffH4xx7YEyUix705IzBZYh0rxJST4=; b=DGBc85vcEhDdlWKOJMEkQldxvcpIy+dJOM70nWMjyaWW1yhzrD0tQRAEEAdTpHTITLFKznlSPV1sbdmVjqf5Mz+TRc79RtWq3roF+79XpmnTJL8jFjyuuI8EGOJHAtAc9iG4Q789rRz+nS3hFZpq5RfGCZg9RsBKcy6kjY3eBMnWXMWYVHk/78LZG3vFz5EuJTES/SQnM3fsZCfVRdRD0EGQWOZnzjPs5GGAT7ekzPL2I0up5i+WmWceMzIga8arWlzGyOZ8TbWDO22Q+8ApI8Y72Y/RD14lLpct8SUKaZ9mZeTqsoToYvqoPg16w+ncwBonzbh+xapFYhymDH2+8w== 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=EMLus+UICrUfqiffH4xx7YEyUix705IzBZYh0rxJST4=; b=FpEDQXIu1mOSBAbRF9kiuBbdUClmZ7LVTvHEQVy239nPHocXtwEmo8nQgsVCyUecSWhGpYbS2C7NZIJ65hvkjk1udpXep2TJan8cFr1XhtTPdwkOFNplb8BKHdiLeBNABdTNCd+8y4rnNlmRt3JTQ9nTapqbRJtcfaUjlvRdmi0= Received: from PH0PR11MB4885.namprd11.prod.outlook.com (2603:10b6:510:35::14) by PH0PR11MB4933.namprd11.prod.outlook.com (2603:10b6:510:33::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4544.15; Tue, 28 Sep 2021 00:21:25 +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; Tue, 28 Sep 2021 00:21:25 +0000 From: "Yao, Jiewen" To: "kraxel@redhat.com" 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 ResetVector Thread-Topic: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector Thread-Index: AQHXrsfv6qwUC8Li5kiwlPdQeu+jg6uvry6AgAEZ5ACAAIjkgIAALkqAgAAWigCAAAcDAIAAA09wgAAI9ICAAAMxAIAA/y2AgAAYU4CAADWiAIAABQmAgAA8mYCAACMh4IAEMBkAgAAYt5CAAFryAIAAmLsw Date: Tue, 28 Sep 2021 00:21:25 +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> <20210927145914.7r6ezdzno3zt2flq@sirius.home.kraxel.org> In-Reply-To: <20210927145914.7r6ezdzno3zt2flq@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: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 50ae23e5-723c-4459-83ae-08d98215e885 x-ms-traffictypediagnostic: PH0PR11MB4933: 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: wSf3Hj7aIv0D7GK0XwwpM4e6Ac7GQSV9rmNxSU9B2nOiWTKziUz2STSUM+/jBeTihVfnqTZphOckKhLagDtFD4oVjCkNJ12R1sVi17NaXH8iXutVpadxREMaZHT9nrKl89TAwr8eZavAH16XXNv/QfJ0ulSfUmlxkZpVFDZv6fZ5ctbf34duv1OaLPzJqImNqDD7R2kN83Xi3IlcGoGJfBJoWvjUi1SMt7xS2qoKZgYaRu+IDRSHtfuakzEWr+W4GbAfGgF+2g429J6veRgOaT6adEtz53W3XqyD1wlO8F4dOqbGfjTL3cG1lxnFJHLIB2dnHOoTsZu0sMJ8u+4sUJ1vnvie64EbIkgmuewpn/DME6h8qU48Fp/Mf7Yme9K/iaV2rOR3RKVz4cFkbcsMEYvk0B127mB5Z9GCg9CkDgufgFuwc7f1WYftr4LiW/w+OEFRiFVhBgk9TvjewYkw6sPt41PfMdnJIKnPOz7ERsZ0eF42eSHzlGfizVY1ePlVgoWoxWXcMMo40+TlKtiEJVqL+6MZEkaTAaeCUElV8YwejoGGsAslL3nv91uBbB5aqxt9DRO3PWjA/qSDCJp70GDAXTJ6XJdXsU0xmyr5iikiMHHfFb6PYyWzH1O0LyY3ZdfWCfENmpOsJ6bBdp28yLG2XqqM49i8W6FOmU0uUylWezmhXQsVKluLVsx5a9nmU317QIRgQGmADsFeeT4XijdFvkoH9uMcPlsVI6JY4MxAkJafWnri2VSKjgx+UYnCdGnlf4kSgb4wYIRBbMrJWtb1nog0MIYnvONMAU+VqUlvLYOtIIylnYF87yrfRNiHAB2y8mjdJHc2M0fMusAPLw== 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)(66446008)(966005)(66476007)(66556008)(64756008)(38100700002)(9686003)(2906002)(5660300002)(7696005)(6916009)(38070700005)(122000001)(52536014)(508600001)(33656002)(26005)(66946007)(55016002)(4326008)(86362001)(6506007)(8676002)(8936002)(19627235002)(71200400001)(83380400001)(186003)(76116006)(53546011)(54906003)(316002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?4xwoJ1e51Pp5TFgkm0SvBsrIgYtwjmQvaFi83/G1aEEhskQg4TIUOOEFL6/s?= =?us-ascii?Q?OkbmmcV4yvj8FvB5TCDrACkGL10pBecXu+sLskhR1/vGpklVTXZwVa5SUzoI?= =?us-ascii?Q?g33A+molKjOEgSgOIO1Oiw5lFLX1Y9pfoSImeHsM6T/kElJCwy9OMvPeSAYP?= =?us-ascii?Q?A5YEtCIOcbxlurgpR3dvPe1Mqz52HxEpdjKG7hbLEx98cUbXNNFuVwxYHd8J?= =?us-ascii?Q?qzQfFwXKrDDC8X5V8cJ2oML8DJMSpqTZAfNYPji6WQp/QDXdixMUJriSyqI+?= =?us-ascii?Q?IGZLNj7vX2gG7OhhALhRLxTQqpLtyQ/ZaxcvnOd6EEpWIPVArZ+4QhWYrXz1?= =?us-ascii?Q?XsfmsFIOuH27AzbEVVc4h+2LxpU7AcMtu5WzO4jX6lAgofBYDV+830A1fYMK?= =?us-ascii?Q?bdaGUWS5TDj+HYbQEIKZGjO4q0h9/jOFnGymyRgmZax670rPWmnTu0b1rLrr?= =?us-ascii?Q?RmvqRSCWCQWLmC8cRENhw5oV1eUKf4JbrkdKP6Gt9wsND9OABEMDBBvsg7Ca?= =?us-ascii?Q?fdE9cL5UDiTCFW6frChP7c4T0IVm3Mcmicv4hknptMp7SRa5xZWWXZ1J7j64?= =?us-ascii?Q?Hpx8eljqm+WxUu2uDDTxN0jEG8N7Coe0RESt4XFY6z+tx3DS2NmcE5kenxh7?= =?us-ascii?Q?uiAZVgkYBU+aNTVF9uJiowt2aYl+uavaatqIrzM5nwGM0TONNIWCM/74N6Ek?= =?us-ascii?Q?mp8MsHVzIPrkotb1uXFR+lipYN/GOQoX7QXBxvxVUQ18pxPf2lBqMw+3L7Jo?= =?us-ascii?Q?UnIitkttTOYOd/PfbdWq+47JNE3v8yklZlJuom0pW3AASndTmrJUZPHK+FPk?= =?us-ascii?Q?zZ6Sg6frU1K2HtWnzVmaIPJiB5ws4OKcc5Vcwj8+Btroz6s+0Pq0Rmek+pLg?= =?us-ascii?Q?Z9xggEMS+qLZ1UIN/Peug0W06mRWp3sMgPyiDdyBm0kKb9imaty0QWR7FqlS?= =?us-ascii?Q?SBtx9NygtpyulYEsM4SeKOgrQ65QKStdfajHL792CG00MlBsRXtLX/d+IVho?= =?us-ascii?Q?MUVTgtV9xioEpk8f6NhIR47TIRkzQga5I7GfoRmRDEb3qgaGzuWAwK/HhaaF?= =?us-ascii?Q?I2pUNfE2o6ojTxUcydBRTGUJLg30I57KOLq4Gstb/n7J0+z/Q0OzCAl3An3M?= =?us-ascii?Q?NSGmc/jhr+ErxnSfW7yjV8mL11cC3zF8mVqaTUe2oJrYJEa+gkY3Dbrc5Sex?= =?us-ascii?Q?zfZMy7lvIeJi2FAWsaAxTRCsHBzvfSbnPYeYZiWXyAxPqyL3pbSqh25kuKjU?= =?us-ascii?Q?0f8EVQUMIOWmUmx0SGUEG6gz0tF5IRNw5gwV3EggWjCEQdzBzGXyHUe5RU/X?= =?us-ascii?Q?xfhnMs7yZnIQVna3uaCs9bQB?= 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: 50ae23e5-723c-4459-83ae-08d98215e885 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Sep 2021 00:21:25.5180 (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: dfgH5lrauK/ewx3BydhI/1w+exDGreicHUNYATh80LJYfzXzF4Fmlcnpr7hc40msH4S7a3jCdWdT1WpVL6AuMw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4933 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 For size field, please refer to PE/COFF specification https://docs.microsof= t.com/en-us/windows/win32/debug/pe-format The "Section Table (Section Headers)" defines two fields: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D VirtualSize - The total size of the section when loaded into memory. If thi= s value is greater than SizeOfRawData, the section is zero-padded. This fie= ld is valid only for executable images and should be set to zero for object= files. SizeOfRawData - The size of the section (for object files) or the size of t= he initialized data on disk (for image files). For executable images, this = must be a multiple of FileAlignment from the optional header. If this is le= ss than VirtualSize, the remainder of the section is zero-filled. Because t= he SizeOfRawData field is rounded but the VirtualSize field is not, it is p= ossible for SizeOfRawData to be greater than VirtualSize as well. When a se= ction contains only uninitialized data, this field should be zero. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D We took similar concept here. RawDataSize =3D=3D size in the file. MemoryDataSize =3D=3D size in the memory. They are totally different concep= t. For example, you can have 0xC81 RawDataSize, but the MemoryDataSize is 0x10= 00. If one project enforces RawDataSize =3D=3D MemoryDataSize, then only one si= ze is needed. But if one project wants to RawDataSize <=3D MemoryDataSize, then we need t= wo size fields. Thank you Yao Jiewen > -----Original Message----- > From: kraxel@redhat.com > Sent: Monday, September 27, 2021 10:59 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 > > > 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 metadata 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 > > metadata to allocate the DRAM (AUG page). >=20 > Hmm. Not covered in tdx-virtual-firmware-design-guide-rev-1.pdf >=20 > > The runtime section size must be UINT64, otherwise we cannot support > = 4G > memory 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. >=20 > That still doesn't explain why you need two sizes. Instead of depending > on zero-fill in case MemoryDataSize > RawDataSize you can just use two > entries, simliar to ELF binaries which have separate '.data' and '.bss' > sections too. >=20 > > I can understand why you think there is no needed fields, based upon > > what you see in EDKII/TDVF project. However, the usage in current > > TDVF is just a subset, but not all usages. >=20 > So you are doing stuff behind closed doors ... > > 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 same metadata structure. (But sorry, I cannot tell more at this > > time.) >=20 > ... and don't want tell details. Even the fact that you are doing that > is disclosed only after poking for a while because the patches submitted > leave a bunch of questions open. >=20 > This is NOT how Open Source Development works. >=20 > If the patches can't speak for themself in cases like this the very > minimum requirement is proper documentation. It is not acceptable > that I have to ask five times to figure that the format is supposed > to cover use cases beyond TDVF. >=20 > > 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. >=20 > The benefit of a unified table for tdx and sev is that the VMM can > have common logic to find page ranges which need special > initialization. >=20 > But I suspect at that point we are going to trade code sharing at one > place for code sharing at another place. >=20 > > I think it is OK, if SEV wants to reuse the existing TDX metadata > > table. (We need SEV people agree.) Then we can have one metadata > > table. >=20 > So, when submitting the next revision of this series, please ... >=20 > (1) Move the tdx metadata changes to a separate patch. > (2) Add *complete* documentation for the TDX metadata format >=20 > ... so the SEV people can make up their mind whenever they want use > that or not. >=20 > Please do also clarify what the process to allocate section type numbers > (or reserve a number range) for SEV would be. >=20 > > [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. >=20 > The differences between Ovmf and AmdSev are very small. >=20 > Bhyve has more differences, but it's a different hypervisor > so it isn't surprising it has its own PlatformPei. >=20 > How does Tdvf handle the platform setup? It must be done in SEC > somehow, so I suspect you have a (possibly stripped-down) version > of the PlatformPei adapted for SEC? That is exactly the kind of > code duplication I want avoid. >=20 > > I reluctant to merge it back to Ovmf.dsc/fdf. >=20 > I don't worry that much about Ovmf.dsc/fdf files. Whenever we add a > compile-time option (-D ENABLE_TDX) to Ovmf.dsc/fdf or whenever we add a > separate Tdvf.dsc/fdf doesn't make that much of a difference. >=20 > I'm more worried about the code duplication and the completely different > (PEI-less) initialization workflow. When touching the platform setup > code both cases (with/without PEI) must be considered, which increases > development and testing and maintenance effort long-term. >=20 > I want less variants, not more. Ideally I'd like to also get rid of the > OvmfPkgIa32X64.dsc/fdf for example. It seems some features have a > dependency on PEI running in 32-bit mode though. >=20 > > The reason is the main Ovmf supports some features (such as S3, TPM) > > which may depend on PEI modules, but it is NOT needed in TDVF. >=20 > So using PEI in OVMF isn't that over-engineered, isn't it? >=20 > And I suspect SMM support can be added to the list of features which > depend on the PEI phase (at least when we want reuse the existing common > code in UefiCpuPkg, MdePkg and elsewhere). >=20 > > We need re-evaluate the effort to enable those features in non-PEI > > configuration in OVMF. - That is totally unnecessary in TDVF enabling > > task. >=20 > Well, it's surely additional upfront work. But I expect it will pay off > long-term. Less maintenance work, less testing work, lower risk of > adding regressions due to SEC and PEI init code paths variants running > out of sync. So "totally unnecessary" only when you ignore the work > needed after the initial merge. >=20 > take care, > Gerd