Re: [Patch] SecurityPkg: Fix TPM device compatibility issue

[Leif Lindholm <leif.lindholm@linaro.org>]

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.zhang@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Yao Jiewen <jiewen.yao@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