Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe

["Wang, Jian J" <jian.j.wang@intel.com>]

Hi Laszlo,

Very sorry for the mistake. I'm totally responsible for it.

Regards,
Jian


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, July 04, 2019 4:38 AM
> To: Desai, Imran <imran.desai@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> Cc: devel@edk2.groups.io; Justen, Jordan L <jordan.l.justen@intel.com>;
> Ard Biesheuvel <ard.biesheuvel@linaro.org>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; Stefan Berger <stefanb@linux.ibm.com>;
> Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Andrew Fish <afish@apple.com>; Leif
> Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into
> Tcg2Pei and Tcg2Dxe
> 
> On 05/28/19 22:40, Imran Desai wrote:
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1781
> >
> > 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 suite.
> > This integration has dependency on the openssl_1_1_1b integration into
> > edk2.
> > This patch links SM3 support into Tcg2Pei and Tcg2Dxe.
> >
> >
> > Signed-off-by: Imran Desai <imran.desai@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  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/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -914,5 +915,6 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !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/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -924,5 +925,6 @@ [Components.X64]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !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/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -922,5 +923,6 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !endif
> >