From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web10.6420.1582713985189275569 for ; Wed, 26 Feb 2020 02:46:25 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=OF3vxLnP; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582713984; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=I/vNZDQp/6XTtxGUi/rQTBTUAotqTqbfkABAHSdg3WA=; b=OF3vxLnP9YqfM1Pp8XroACTihliKSqgU2sNOnVfCqvFOy4MWMA0KpF6UixiAGURf1zAO0w iLfvVuzcET60MJuU/HY6yB+vr9ARsakvEPhPVVN1rQfoy19QhqEBxAyshajCSGYLveGrAP bLXuIYPot19NA2jsZ0/Awktwhq2qnFw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-411-DRMX6F_ENDWzc2APVGr8OQ-1; Wed, 26 Feb 2020 05:46:20 -0500 X-MC-Unique: DRMX6F_ENDWzc2APVGr8OQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 60D726B7F0; Wed, 26 Feb 2020 10:46:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-185.ams2.redhat.com [10.36.116.185]) by smtp.corp.redhat.com (Postfix) with ESMTP id ACDBC5DA60; Wed, 26 Feb 2020 10:46:15 +0000 (UTC) Subject: Re: [PATCH v3 6/6] OvmfPkg: add TCG Configuration menu to the Device Manager menu To: marcandre.lureau@redhat.com, devel@edk2.groups.io Cc: simon.hardy@itdev.co.uk, stefanb@linux.ibm.com References: <20200226093459.1131530-1-marcandre.lureau@redhat.com> <20200226093459.1131530-7-marcandre.lureau@redhat.com> From: "Laszlo Ersek" Message-ID: <71012a37-5c55-1977-8191-66f39888be17@redhat.com> Date: Wed, 26 Feb 2020 11:46:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200226093459.1131530-7-marcandre.lureau@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi, On 02/26/20 10:34, marcandre.lureau@redhat.com wrote: > From: Marc-Andr=C3=A9 Lureau >=20 > Matches TPM 2.0 commit 3103389043bd ("OvmfPkg: Add TCG2 Configuration > menu to the Device Manager menu", 2019-02-11). >=20 > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++ > OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++ > OvmfPkg/OvmfPkgX64.dsc | 6 ++++++ > 3 files changed, 18 insertions(+) >=20 > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 2fc10d2393e3..02300886563e 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -936,4 +936,10 @@ > > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceL= ibDTpm.inf > } > +!if $(TPM_CONFIG_ENABLE) =3D=3D TRUE > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf { > + > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > + } > +!endif > !endif > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index cd9d2ac724ca..3adc75223d05 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -950,4 +950,10 @@ > > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceL= ibDTpm.inf > } > +!if $(TPM_CONFIG_ENABLE) =3D=3D TRUE > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf { > + > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > + } > +!endif > !endif > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 317a23b994b8..5f3740ae890a 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -948,4 +948,10 @@ > > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceL= ibDTpm.inf > } > +!if $(TPM_CONFIG_ENABLE) =3D=3D TRUE > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf { > + > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > + } > +!endif > !endif >=20 I've got two comments on this: (1) I'm unsure why we need to explicitly specify the PcdLib class resolution here. The module in question is of type DXE_DRIVER, and we already have the following in the DSC files: [LibraryClasses.common.DXE_DRIVER] PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf So I'd think the bracketed { ... } parts should be possible to omit. (2) The driver is not being added to the FDF files, so they will be complied, but not included in the firmware executable. TBH I'd suggest simply dropping this patch. If Stefan insists on including this patch in the series, then please fix (1) and (2) above. Like that: Reviewed-by: Laszlo Ersek Thanks! Laszlo