Laszlo, Given that the compatibility issue has been present for several months, I would prefer this change be deferred until after the stable tag.
We can document this as a known issue in the release page for the stable tag and point to the commit after the stable tag that resolves the compatibility issue so platforms that are impacted can choose to cherry-pick the one additional change. Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel- > boun...@lists.01.org] On Behalf Of Laszlo Ersek > Sent: Friday, November 9, 2018 8:22 AM > To: Zhang, Chao B <chao.b.zh...@intel.com>; Leif > Lindholm <leif.lindh...@linaro.org> > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; > edk2-devel@lists.01.org; Yao, Jiewen > <jiewen....@intel.com> > Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM device > compatibility issue > > On 11/09/18 15:46, Zhang, Chao B wrote: > > Hi Leif: > > The NTC1310 can work well with previous EDK2 > stable version (UDK2018). > > That's not correct. The last EDK2 stable version is not > UDK2018. The > last stable EDK2 version is "edk2-stable201808", and > the regression > (commit f15cb995bb38, according to this patch) is > already contained in > "edk2-stable201808". > > > Interface Cache is a new feature introduced after > UDK2018. > > So far as we see, it causes NTC1310 fail to work. > > That may be the case, but it's not a feature (and > resultant regression) > introduced in this development cycle. > > Personally I'm still undecided. > > - From an end-user perspective, this is certainly a > "bugfix"; given that > the NTC1310 hardware (which is the real culprit) cannot > be fixed at all. > > - In a technical edk2 sense, this is a feature-let > however (with code > duplication at that) that works around a broken device > that already > doesn't work with "edk2-stable201808". > > > I'm *slightly* leaning against this change. If the end- > user regression > had really been painful, this workaround would have > been implemented > very soon after "edk2-stable201808", not in a last > minute rush before > "edk2-stable201811". > > I'd like to hear Mike's and Andrew's opinion too. > > Thanks, > Laszlo > > > > > From: edk2-devel [mailto:edk2-devel- > boun...@lists.01.org] On Behalf Of Leif Lindholm > > Sent: Friday, November 9, 2018 7:13 PM > > To: Laszlo Ersek <ler...@redhat.com> > > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; > edk2-devel@lists.01.org; Yao, Jiewen > <jiewen....@intel.com>; Zhang, Chao B > <chao.b.zh...@intel.com> > > Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM > device compatibility issue > > > > On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo > Ersek wrote: > >> On 11/09/18 07:02, Zhang, Chao B wrote: > >>> Issue Statement: > >>> TPM InterfaceId cache feature is introduced by > f15cb995bb3880b77e15afe6facd3da05e599a17. It follows > TCG PTP spec 1.3 > >>> to improve TPM transmission performance and also > addresses defects in some TPM2.0 devices. But some > other TPM devices like > >>> NTC1310 SPI TPM is found function abnormally with > this feature, causing extra device compatibility issue. > >>> > >>> Solution: > >>> Add a policy indicator in PcdActiveTpmInterfaceType > to disable TPM interface ID cache to support those > existing TPM devices > >>> > >>> Contributed-under: TianoCore Contribution Agreement > 1.1 > >>> Signed-off-by: Zhang, Chao B > <chao.b.zh...@intel.com<mailto:chao.b.zh...@intel.com>> > >>> Cc: Andrew Fish > <af...@apple.com<mailto:af...@apple.com>> > >>> Cc: Laszlo Ersek > <ler...@redhat.com<mailto:ler...@redhat.com>> > >>> Cc: Leif Lindholm > <leif.lindh...@linaro.org<mailto:leif.lindholm@linaro.o > rg>> > >>> Cc: Michael D Kinney > <michael.d.kin...@intel.com<mailto:michael.d.kinney@int > el.com>> > >>> Cc: Yao Jiewen > <jiewen....@intel.com<mailto:jiewen....@intel.com>> > >>> --- > >>> SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | > 23 +++++++++++- > >>> SecurityPkg/SecurityPkg.dec | > 3 +- > >>> SecurityPkg/SecurityPkg.uni | > 3 +- > >>> SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | > 49 +++++++++++++++++++++++++ > >>> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | > 42 +++++++++++++++++++++ > >>> 5 files changed, 117 insertions(+), 3 deletions(-) > >> > >> I'll let others review this patch for technical > merit. > >> > >> However, I'm really undecided whether this patch > qualifies for being > >> pushed during the hard feature freeze. Comments > welcome. > > > > Unless the current behaviour causes an absolutely > horrendous security > > hole, I don't see how this qualifies for pushing > during hard freeze. > > > > According to its description, this is about > supporting (non-compliant) > > devices that have never worked with EDK2. And the > support it updates > > went in on 25 June. So there does not appear to be > any urgency. > > > > Once it does go in, I would also appreciate some > simplification via > > macros to cut down some of those very long lines, but > then I'm not the > > maintainer of this package. > > > > Regards, > > > > Leif > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org<mailto:edk2- > de...@lists.01.org> > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
