From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 03 Jul 2019 13:37:57 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 37F9B30860A3; Wed, 3 Jul 2019 20:37:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-89.ams2.redhat.com [10.36.116.89]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4FF76413B; Wed, 3 Jul 2019 20:37:46 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe To: imran.desai@intel.com, Jian J Wang References: <20190528204049.86463-1-imran.desai@intel.com> <20190528204049.86463-6-imran.desai@intel.com> From: "Laszlo Ersek" Cc: devel@edk2.groups.io, Jordan Justen , Ard Biesheuvel , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Stefan Berger , Stephano Cetola , Michael Kinney , Andrew Fish , Leif Lindholm Message-ID: <9c92afaf-b368-628e-f686-64257831343b@redhat.com> Date: Wed, 3 Jul 2019 22:37:40 +0200 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: <20190528204049.86463-6-imran.desai@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Wed, 03 Jul 2019 20:37:54 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 05/28/19 22:40, Imran Desai wrote: >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1781 >=20 > EDK2 Support for SM3 digest algorithm is needed to enable TPM with SM3 = PCR > banks. This digest algorithm is part of the China Crypto algorithm suit= e. > This integration has dependency on the openssl_1_1_1b integration into > edk2. > This patch links SM3 support into Tcg2Pei and Tcg2Dxe. >=20 >=20 > Signed-off-by: Imran Desai > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Marc-Andr=C3=A9 Lureau > Cc: Stefan Berger > --- > OvmfPkg/OvmfPkgIa32.dsc | 2 ++ > OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++ > OvmfPkg/OvmfPkgX64.dsc | 2 ++ > 3 files changed, 6 insertions(+) Wow, what just happened here? I'm noticing now that this patch has been pushed to the master branch as commit a7c7d21ffa9a. However, *NONE* of the OvmfPkg co-maintainers or reviewers have reviewed this patch! The commit message includes "Cc:" lines, but that's a lie. Probably not an intentional lie, but a lie nonetheless. These patches have *never* been delivered to my inbox, and if I look at the address list on the message instance that was reflected by the mailing list, that address list confirms the same. I'm pretty sure Imran's git configuration has a bug related to CC's. (I've extended the address list now, manually.) Jian: please revert this patch immediately, stating, as reason, that the patch review process was not honored. I'm sorry but I cannot let this slide -- if you look at commit a7c7d21ffa9a now, it suggests that the OvmfPkg maintainers / reviewers were CC'd (they weren't), but they ignored the patch (they didn't -- they couldn't see it), and that another maintainer pushed the patch just the same (which is factual, but *wrong*). After the revert, Imran can resubmit the patch, with *functional* CC's, and then we can discuss it. In general, it is fine for one maintainer to push a series that touches multiple top-level packages. However, that maintainer *MUST* make sure that each patch has sufficient "M" reviews, and he/she is responsible for picking up the feedback tags for *all* patches from the list. Come on now, guys -- have you really known me to be a person that *silently ignores* an OvmfPkg patch for more than a month? No automated out-of-office reply, no "please give me some time to review" reply, just silence? And even if I missed a patch like that, don't you think a maintainer deserves a ping first? ... Why are we, as a community, *still* failing at this process? Laszlo > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 578fc6c98ec8..fb5944aa6945 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -628,6 +628,7 @@ [Components] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !if $(TPM2_CONFIG_ENABLE) =3D=3D TRUE > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf > @@ -914,5 +915,6 @@ [Components] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !endif > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index eade8f62d3de..64c231f735c2 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -636,6 +636,7 @@ [Components.IA32] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !if $(TPM2_CONFIG_ENABLE) =3D=3D TRUE > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf > @@ -924,5 +925,6 @@ [Components.X64] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !endif > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 733a4c9d8a43..7e46d401a36f 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -635,6 +635,7 @@ [Components] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !if $(TPM2_CONFIG_ENABLE) =3D=3D TRUE > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf > @@ -922,5 +923,6 @@ [Components] > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSh= a256.inf > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSh= a384.inf > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSh= a512.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.i= nf > } > !endif >=20