From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web08.1558.1632501657477632499 for ; Fri, 24 Sep 2021 09:40:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=P5HhBfcL; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: jiewen.yao@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10117"; a="287781912" X-IronPort-AV: E=Sophos;i="5.85,320,1624345200"; d="scan'208";a="287781912" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2021 09:40:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,320,1624345200"; d="scan'208";a="475243180" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga007.jf.intel.com with ESMTP; 24 Sep 2021 09:40:55 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Fri, 24 Sep 2021 09:40:55 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) 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; Fri, 24 Sep 2021 09:40:54 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Fri, 24 Sep 2021 09:40:54 -0700 Received: from NAM02-BN1-obe.outbound.protection.outlook.com (104.47.51.48) 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; Fri, 24 Sep 2021 09:40:52 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=i0CVRfj+jQc++l/TJ6GNfSSEF1pXDJ1uLOioZZFEBh6MJnB4SheJaFrnnJz7LyHqoNdejqmvyJQb5Tjd1kG+qm8Z5LlRpW0qLFoSSSTtFiJ3k54mm6ujDELxKAstkCEUQXJHUYyNiZ0I5G9W/nB3qITl3ZCt/8CZFMkoxJSWS3vaqK7ivz492hMlQki6VpPguO3CcfQzEMMUxCC1574RsPBKU5qM+n1/0YeYVazLA5VvyYl8TTdz0j5jhEKEPryMFzItNwJ2fHhkBF2MDha9Hni2CV/vVCpfHJVD0XqrkQk8I+9P00XAcMufoV8y1Umq1sPSN1opUePRWqMv8CSfzQ== 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=FgzHdkE5eXU6CXHabakYSTzjvVE/Bwd/xiB4fGn/otw=; b=jj880rviqTHBTixm67+mdWVBvn7aKgM4qKozJ1F629I0YDENBJBuXjQdBtGaVSA6v8E7yGkAeeIJK9ddudHF6ffIUAIe8WqiDsnu0gslm6QkO/y0XFxtdPnlW5FdTR/RM5hp4o6hH/DwG5UjQvY8Cu4k/rrMWNgJ6KrMBvjG+WTvZIp0mD0qXNVVLr82bgV6I/hH9QKM9N3vpN/U++qb4kbP46kdbNYFoRGQREVHM81bCjLiCD65e8AdlKuVl2l2iTxR1Ll0A7HPRg5RnDk+0nIIKCyOYKunl55hydmIIxEr8M9RCbcZf96o4g42PjDBGycDKjldJgPyzGIViOhn8w== 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=FgzHdkE5eXU6CXHabakYSTzjvVE/Bwd/xiB4fGn/otw=; b=P5HhBfcLNValy8o9Vc+fNH7vivc4BIa9mcMGnzwOZ6zABwltBY8X+9eNkKxTU916j1k8W24UxUyzu92z9jzhQSOKPTJ4IVphbery8cSk044hUDyPdrLfkqEn5wT2xymc4yRmocenge7reN/6vy88/fuxcTfkoJ9r0KsT5QOk90c= Received: from PH0PR11MB4885.namprd11.prod.outlook.com (2603:10b6:510:35::14) by PH0PR11MB5062.namprd11.prod.outlook.com (2603:10b6:510:3e::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4544.13; Fri, 24 Sep 2021 16:40:46 +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.018; Fri, 24 Sep 2021 16:40:46 +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/y2AgAAYU4CAADWiAIAABQmAgAA8mYCAACMh4A== Date: Fri, 24 Sep 2021 16:40:46 +0000 Message-ID: References: <7c9aeb95-5c33-bd8d-4f0c-40133f4c7c3d@amd.com> <20210924052825.2qljhtvweonbov5q@sirius.home.kraxel.org> <20210924100726.m33otbjod4fo3vur@sirius.home.kraxel.org> <20210924140221.6b3nk32eofwbwpgb@sirius.home.kraxel.org> In-Reply-To: <20210924140221.6b3nk32eofwbwpgb@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: a80de7ba-b077-4e74-7e4f-08d97f7a0f4b x-ms-traffictypediagnostic: PH0PR11MB5062: 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:2449; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: SggKLLZZazaz/1XtRwfUgNDa5vnjMdXy2BfnUDI0QGKFl4kneG4wWDrODcijBvXfNeyfe2mIPabLTSYI0tFeu6yVWrFfRukUDMYaESPEs3Y8LYe1HOc8STB+C7uUEp4beTUS6Fvu/7LXOX0qcQlTeAj6RZdB14IJnaezgaPV56J3dG3QaJAtaLCe93PXwG5Rmp9UjOH3zeJ2JSU1mFD/7AkfKevguJRPeb3PrvsYwgt7yCNS5HoV8HZ9UZ4CesVE74SZi7NCXwpmPWjNIFdDvx8C60awA5WlOAJyB8D+Np5y8jnTzgb+JI6F9l0uUc0E1T/3EzuqHHlhRj4WISU5CwPTmgQ+QI8MSHYy67jQpFyeZIPvKQqHsVcZ17AKsPgCu+84cMZ+tTv5KJ0XOSt+oxTbZJj5KRyV2vM3dXOGebctRADFsSsIACZ1hjtZbqZb1n8CKmW5PHQtJdmmvp7Ht7KfJsOq4D31osuq8tqnVD7lk4/5epRftetQ4Htm0sLURBO1G4zhEHEmbnOtteTPdOQXXCoNYdZFMFEmfyk9YK0UbM9cZ9Xi8z2Gndd3+0GNGL+bjjOHnhpAMHOU0Zac6YpZKP2JhJDSKS2t2XdlFVzUMeKuHCfjDRj3gXAMf98hqKD53RhfcJHUmapDGMqVxY8xrKTRauWV8GhpLfAQsFe+by3iExnWAhI9V5WwhQDmpY/wHpYuMEp00H+wVIunPxTrQeeFX4IU51gRTNh/gtY4PYGVWlnCERqi2+HYMYQ2w1KwxcH2rgEtvYkDfl9o7QBrqNkc1Dbn4BXTduwOaKOWvlyjauVRpV81GMZLl/j4OCDs/62hxBuGL0NltMdbqw== 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)(33656002)(508600001)(86362001)(71200400001)(966005)(7696005)(38070700005)(4326008)(9686003)(2906002)(55016002)(6506007)(83380400001)(53546011)(76116006)(66476007)(8676002)(54906003)(186003)(26005)(66556008)(66446008)(122000001)(5660300002)(64756008)(6916009)(52536014)(8936002)(38100700002)(66946007)(316002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?2UD2S8t6ZsjfbMg3sJsPEUrGEO/ntEoz/ZWnLikq2PK81KqbJGzfmQLUeNRP?= =?us-ascii?Q?v7vjasf4b02ZecnQxufSFrCxJjU2eqrQyslJVxVCoa1OQktjwjmlL8LqjiOX?= =?us-ascii?Q?2Clk0EAToMWo8Dg9N8PgT2gKs1frHTUTwQzC+6m/CbMLgWTZUG5y68E5WYmK?= =?us-ascii?Q?GMKwJbJhILo5vAXSHacD5ezTnrfYkzSpaAHmhvhrgPWXVShFBcR72gcbXDkP?= =?us-ascii?Q?FLTGBCXG7dYcNFOHJ5rGncoshEo7dmjNUxwFZRUNMLpv1K33BcR4aAST2XDq?= =?us-ascii?Q?fF6tThKCo7kG72hW+L0YsCoce++rJB+4BiGmq6AtZhsVtisyattN7MQhX94t?= =?us-ascii?Q?thAWMGTtk3joOylKXNyO7vNqGwHtA+48kR3MWfmhaSwkXHJIn9L7/3PoQdBQ?= =?us-ascii?Q?QigevgvrpWx6hV0HLE6J1JvCkfX0l1HeToRIgJb5K+TvYGlsPiua1xQI13Jl?= =?us-ascii?Q?whepiSZoboTbK8k5v6CByS41r/nTGUcbSKjLrSX8P6s18b8pYbyUgHD4CuFd?= =?us-ascii?Q?cUKJ2+LbLKddRLlyCNkXVwjFJnYAHJk20evaibAHUYoqy0M5wLOxRIGNT/ir?= =?us-ascii?Q?RkBO6yN0cjwAkJexHAFb4lXiLrTZ5hSF0bNp6eSwZ+AUJsZa78xnwcloQS3o?= =?us-ascii?Q?rvv5cCO13PGiHhQzK3Zcy9CvOsKkIchRQaPb/8/f8Sc8pnvAE8W46Kc+AXYO?= =?us-ascii?Q?aj2cxIBNKXQUBI3rU2xGMOoDdOMXiQjvHXtaVN+edquyw8HwHx3QrVkmXlu1?= =?us-ascii?Q?5I+cqybQ7ieWEB1qOUYdLBCXDqWRoShFAAglTkp9tA+sWph9VAng63OxsKHT?= =?us-ascii?Q?1Ru9YnLigBeF7GzleSoQrqwwBzkSZ3QfWRGNueWnYe/m1MCdHukxniPPKH9C?= =?us-ascii?Q?9T5V6p1AIvKDE4+u0ISvo7Hr6t6epP7jdj4CgkgpGh7NuJlNgfHvcPA4sQr+?= =?us-ascii?Q?PLJWraTlR+xND9tbGMRbcrhrvzjIzyHMRy9mWaXY0W8/n/UWDmXUlqOX2r++?= =?us-ascii?Q?M8T0y4WvXbrYCgCNnLHeU2Mcu7f5r4sChbdBzetloy+lPJzt0tzoigdVEj0x?= =?us-ascii?Q?Nh9hQ/eYwjP6X9PDWaoaVBpslTYWnOZbFes+SP3PmgjrnIZ0kgdMK4sBzWGx?= =?us-ascii?Q?RDXXZViynm8r3zwOJbcjKg9JBlGLrYHkRsAgO4wJr0BBvlZmq4rEq8ZEOSeP?= =?us-ascii?Q?j++2M0acJKdRkwH3o1Z6hlgAe3rkYuLqCtkr062eqk8mdMDeHKcIwdio7xT1?= =?us-ascii?Q?CRpkA1wR8E1iR0YSsr34oQtZa/ILL2oK/ShDv9Gh5cSThAJeZjzPz25lXfu2?= =?us-ascii?Q?pmpJKoE42HiAgH68Yd8joqu1?= 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: a80de7ba-b077-4e74-7e4f-08d97f7a0f4b X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Sep 2021 16:40:46.8252 (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: +LZXXPEeI9f/K9qpKY0Pra7tYHWNTl9GTYLuk0KYnAVzHL1U8TxBVxV6keQ3AXH3iBahSuWVFFwt3mLe6wPZvQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5062 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: kraxel@redhat.com > Sent: Friday, September 24, 2021 10:02 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 > On Fri, Sep 24, 2021 at 10:33:35AM +0000, Yao, Jiewen wrote: > > Again. Two topics. We need discuss them separately. > > > > Topic 1: TD metadata table is an architecture way to communicate with > > VMM. We took the design from PE/COFF image section, which is flexible > > to support different binary format. > > EDKII TDVF is just one possible producer. There could be other > > producer in the future. We don't want to define something only meet > > current TDVF need. >=20 > Hmm. efi has a kind-of binary format (EFI_FIRMWARE_VOLUME_HEADER). > It's not fully self-contained though, you need to know where the > architecture places the firmware (i.e. just below 4G for x86) > because the load address isn't there. So I do see the point of > adding other headers adding that. > > 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. This is defined by PI specification, the purpose is to have a way to manage= the firmware file. It is very complicated and with overhead only needed for flash. Intel has multiple technologies requires firmware/hardware interface. None = of them uses EFI_FIRMARE_VOLUME_HEADER, because it is unnecessary to carry = the complexity. The most famous firmware/hardware interface is called Firmware Interface Ta= ble (FIT) table. - https://software.intel.com/content/dam/develop/external/= us/en/documents/firmware-interface-table-bios-specification-r1p2p1.pdf. Also using EFI_FIRMWARE_VOLUME_HEADER means you have to use PI FV layout, w= hich is another unnecessary limitation.=20 I would strongly disagree to using EFI_FIRMWARE_VOLUME_HEADER for metadata = table. > 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. >=20 > 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 of > other things and it will break backward compatibility anyway. [Jiewen] That is our previously experience, when we define a physical addre= ss, we always use 64bit to leave room for extension. Like UEFI specification defines PHYSICAL_ADDRESS to be UINT64 even in IA32 = platform. Let me tell you a story. In https://github.com/tianocore/edk2/blob/master/SecurityPkg/Include/Ppi/Fi= rmwareVolumeInfoPrehashedFV.h, we defined the physical address to be FvBase= . UINT32 FvBase; UINT32 FvLength; It is rare, because in other context, we usually define FvBase to be 64bit. Later, when we want to enable a host app based unit test for this data stru= cture, we got test crash directly, because the OS app test allocates an abo= ve 4G base address, and the test case cast it to UINT32. Do we really care to save 4 bytes in the PPI definition? I don't think so. But it just defines in this way and it brings a big burden to enable unit t= est. This code has been checked in for 2 years. Till now, I still regret that wh= y we didn't use UINT64 in the beginning. I have seen other examples to define small size, such as PI Section Size (2= 4 bit), FFS Size (24 bit), HOB size (16 bit). Then we run into problem to s= upport large structure later and we have to figure out ugly work-around to = support those cases. Defining UINT64 gives us enough flexibility for the future, including test = in above 4G environment. I am wondering that, do you really care to save 4 bytes from UINT64 to UINT= 32 ? For type, maybe 2^16 is enough. But for flags, I prefer 32bit. >=20 > I think the entry size can be cut in half with something like this: >=20 > struct { > uint32_t file_offset; > uint32_t load_address; > uint32_t section_size; > uint16_t section_type; > uint16_t section_flags; > }; >=20 > > Topic 2: In config-B we remove PEI. > > I think we should say it in different way: We add PEI back in config-A. > > In our original design we totally eliminated PEI, because it is unneces= sary. > IMHO, it is totally an overdesign in OVMF to include PEI. >=20 > Granted. PEI basically allows OEMs to plug their binary PEIMs into > early hardware initialization. For a full open source firmware there > is little reason to support that, other than maybe using PEIMs from > other edk2 Pkgs unmodified. >=20 > But, again, I don't want have two completely different initialization > code paths in OVMF. We can certainly investigate and discuss dropping > PEI, but that clearly shouldn't be a TDX-only thing. When we eliminate > 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 do= es not need PEI.=20 If you look around other edk2 platform projects, you can already find some = of them does not have PEI. And EmbeddedPkg includes some libs to support th= ose case, such as https://github.com/tianocore/edk2/tree/master/EmbeddedPkg= /Library/PrePiHobLib, https://github.com/tianocore/edk2/tree/master/Embedde= dPkg/Library/PrePiLib. But that does not mean, we need do that for every platform. Each platform owner can have their own choice. If you have intertest to remove PEI, I am happy to discuss with you on deta= il. However, I would treat "removing PEI from OVMF" and "enable TDVF config-B" = be two different tasks. >=20 > take care, > Gerd