From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: suse.com, ip: 15.124.2.86, mailfrom: glin@suse.com) Received: from m4a0040g.houston.softwaregrp.com (m4a0040g.houston.softwaregrp.com [15.124.2.86]) by groups.io with SMTP; Wed, 03 Jul 2019 20:59:36 -0700 Received: FROM m4a0040g.houston.softwaregrp.com (15.120.17.147) BY m4a0040g.houston.softwaregrp.com WITH ESMTP; Thu, 4 Jul 2019 03:58:51 +0000 Received: from M4W0334.microfocus.com (2002:f78:1192::f78:1192) by M4W0335.microfocus.com (2002:f78:1193::f78:1193) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1591.10; Thu, 4 Jul 2019 03:58:28 +0000 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (15.124.8.10) by M4W0334.microfocus.com (15.120.17.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1591.10 via Frontend Transport; Thu, 4 Jul 2019 03:58:28 +0000 Received: from DM6PR18MB2489.namprd18.prod.outlook.com (20.179.105.16) by DM6PR18MB2460.namprd18.prod.outlook.com (20.179.104.155) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2052.15; Thu, 4 Jul 2019 03:58:26 +0000 Received: from DM6PR18MB2489.namprd18.prod.outlook.com ([fe80::c953:1927:cc0a:dcae]) by DM6PR18MB2489.namprd18.prod.outlook.com ([fe80::c953:1927:cc0a:dcae%7]) with mapi id 15.20.2032.019; Thu, 4 Jul 2019 03:58:26 +0000 From: "Gary Lin" To: "devel@edk2.groups.io" , "lersek@redhat.com" CC: Jordan Justen , Stefan Berger , =?iso-8859-1?Q?Marc-Andr=E9_Lureau?= Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg: Only import DxeTpmMeasurementLib when TPM is enabled Thread-Topic: [edk2-devel] [PATCH 1/1] OvmfPkg: Only import DxeTpmMeasurementLib when TPM is enabled Thread-Index: AQHVMYlHJPM5Pv2uK02R7xr3titCA6a5Te/SgACIYIA= Date: Thu, 4 Jul 2019 03:58:26 +0000 Message-ID: <20190704035808.GC32340@GaryWorkstation> References: <20190703102228.25441-1-glin@suse.com> <5f74464e-1a3a-455a-ba46-0f00e20f4ce7@redhat.com> In-Reply-To: <5f74464e-1a3a-455a-ba46-0f00e20f4ce7@redhat.com> Accept-Language: zh-TW, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: DB7PR03CA0103.eurprd03.prod.outlook.com (2603:10a6:10:72::44) To DM6PR18MB2489.namprd18.prod.outlook.com (2603:10b6:5:184::16) authentication-results: spf=none (sender IP is ) smtp.mailfrom=GLin@suse.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [202.47.205.198] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b730d14b-c197-4a83-efc1-08d70033de07 x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020);SRVR:DM6PR18MB2460; x-ms-traffictypediagnostic: DM6PR18MB2460: x-ms-exchange-purlcount: 4 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0088C92887 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(4636009)(7916004)(376002)(396003)(39860400002)(346002)(136003)(366004)(199004)(189003)(6306002)(8936002)(99286004)(6486002)(9686003)(6512007)(86362001)(25786009)(52116002)(68736007)(71200400001)(4326008)(14454004)(229853002)(71190400001)(80792005)(5660300002)(8676002)(81156014)(81166006)(53936002)(6436002)(966005)(256004)(486006)(76176011)(14444005)(6246003)(386003)(316002)(54906003)(2906002)(53546011)(6506007)(66066001)(102836004)(478600001)(110136005)(1076003)(73956011)(72206003)(476003)(11346002)(33656002)(66946007)(66446008)(64756008)(66556008)(2501003)(66476007)(33716001)(305945005)(186003)(19627235002)(446003)(7736002)(3846002)(6116002)(26005);DIR:OUT;SFP:1102;SCL:1;SRVR:DM6PR18MB2460;H:DM6PR18MB2489.namprd18.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: suse.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: ftJ89dJmi319DgApsxXYEglONs9KDzRyfilm7/njur+YVSnhlSkK2XOZTO3tcjRSQZSjyD4B/eFy5w0u3LOGKDaHC9RUftd7laEX8HlxLZps/zMaJZdNLqArsF8ioOi3GfCP2CXx1cA6vHpVCdWatky4kF+8vS0MLRoI5Tbp4HCTl3FDMtlt1NdduUdXcOqDWSo8dobUjBDUSScYJH8gYZDpii8t58QTf7QWnyDZbymdMl1oh07sNZ0DEUXz4IUD+SxljHlAWKpoPNuf15mo9QqAXXAL3tv31EpTD+FYrRVKryYkE/VL8J/46lPCqMYJKu2ZSKRmf9SAasYhkp1YWac0zye64ylNIv1B1BN23RjhMfD6OnriFCXHF/0IvhMuNuaMZO8zXKidpk5g7VDVTwIAZSXDDoDwODO2WNCpxTs= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b730d14b-c197-4a83-efc1-08d70033de07 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jul 2019 03:58:26.6777 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 856b813c-16e5-49a5-85ec-6f081e13b527 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: GLin@suse.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR18MB2460 Return-Path: GLin@suse.com X-OriginatorOrg: suse.com Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-ID: <6E7D2D9489CD00429BCC61D16A5C1CF3@namprd18.prod.outlook.com> Content-Transfer-Encoding: quoted-printable On Wed, Jul 03, 2019 at 09:49:26PM +0200, Laszlo Ersek wrote: > Hi Gary, >=20 > On 07/03/19 12:22, Gary Lin wrote: > > DxeTpmMeasurementLib is only useful when TPM is enabled. > > > > Cc: Jordan Justen > > Cc: Laszlo Ersek > > Cc: Marc-Andr=E9 Lureau > > Cc: Stefan Berger > > Signed-off-by: Gary Lin > > --- > > OvmfPkg/OvmfPkgIa32.dsc | 10 +++++++--- > > OvmfPkg/OvmfPkgIa32X64.dsc | 10 +++++++--- > > OvmfPkg/OvmfPkgX64.dsc | 10 +++++++--- > > 3 files changed, 21 insertions(+), 9 deletions(-) >=20 > This is a good patch, thank you for it. I see two opportunities for > improvement. >=20 > (1) There's something weird going on with your newline characters. The > view I get (in both my INBOX and in my list folder) is identical to > mail-archive.com's view: >=20 > http://mid.mail-archive.com/20190703102228.25441-1-glin@suse.com >=20 > Can you double check your settings, please? >=20 I didn't change my git settings except the mail server due to our recent server migration. Not sure if it's caused by the new mail server or not... >=20 > (2) The commit message should be more convincing. How about this: >=20 Will follow your suggestion to update the patch. BTW, just found that there is a TPM2_ENABLE block below the SECURE_BOOT_ENABLE block. I'll move TpmMeasurementLib there to reduce the lines of change. Thanks, Gary Lin > ----------- > (a) OvmfPkg first had to resolve the TpmMeasurementLib class -- for > SECURE_BOOT_ENABLE only -- when the DxeImageVerificationLib instance > became dependent on TpmMeasurementLib. For details, refer to commit > 0d28d286bf4d ("OvmfPkg: resolve TpmMeasurementLib dependency > introduced in r14687", 2013-09-21). >=20 > (b) At the time, only one instance of TpmMeasurementLib existed, namely > DxeTpmMeasurementLib. This lib instance didn't do anything -- like i= t > was desirable for OVMF --, because OVMF didn't include any Tcg / TrE= E > protocol implementations. >=20 > (c) In commit 308521b13354 ("MdeModulePkg: Move TpmMeasurementLib > LibraryClass from SecurityPkg", 2015-07-01), TpmMeasurementLibNull w= as > introduced. >=20 > (d) In commit 285542ebbb03 ("OvmfPkg: Link AuthVariableLib for following > merged variable driver deploy", 2015-07-01), a TpmMeasurementLib > resolution became necessary regardless of SECURE_BOOT_ENABLE. And so > TpmMeasurementLib was resolved to TpmMeasurementLibNull in OVMF, but > only in the non-SECURE_BOOT_ENABLE case. This step -- possibly, the > larger series containing commit 285542ebbb03 -- missed an opportunit= y > for simplification: given (b), the DxeTpmMeasurementLib instance > should have been simply replaced with the TpmMeasurementLibNull > instance, regardless of SECURE_BOOT_ENABLE. >=20 > (e) In commit 1abfa4ce4835 ("Add TPM2 support defined in trusted computi= ng > group.", 2015-08-13), the TrEE dependency was replaced with a Tcg2 > dependency in DxeTpmMeasurementLib. >=20 > (f) Starting with commit 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module"= , > 2018-03-09), OVMF would include a Tcg2 protocol implementation, > thereby satisfying DxeTpmMeasurementLib's dependency. With > TPM2_ENABLE, it would actually make sense to consume > DxeTpmMeasurementLib -- however, DxeTpmMeasurementLib would never be > used without SECURE_BOOT_ENABLE. >=20 > Therefore, we have the following four scenarios: >=20 > - TPM2_ENABLE + SECURE_BOOT_ENABLE: works as expected. >=20 > - Neither enabled: works as expected. >=20 > - Only TPM2_ENABLE: this build is currently incorrect, because > Variable/RuntimeDxe consumes TpmMeasurementLib directly, but > TpmMeasureAndLogData() will never reach the TPM because we link > TpmMeasurementLibNull into the variable driver. This is a problem from > the larger series containing (f). >=20 > - Only SECURE_BOOT_ENABLE: this build works as expected, but it is > wasteful -- given that the protocol database will never contain Tcg2 > without TPM2_ENABLE, we should simply use TpmMeasurementLibNull. This = is > a problem from (d). >=20 > Resolving TpmMeasurementLib to DxeTpmMeasurementLib as a function of > *only* TPM2_ENABLE, we can fix / optimize the last two cases. > ----------- >=20 > To reflect the reasoning in the subject line, I suggest: >=20 > OvmfPkg: use DxeTpmMeasurementLib if and only if TPM2_ENABLE >=20 > If you agree, please submit a v2 like this. >=20 > Thanks! > Laszlo >=20 >=20 >=20 >=20