* [Patch] SecurityPkg: Fix TPM device compatibility issue
@ 2018-11-09 6:02 Zhang, Chao B
2018-11-09 8:04 ` Laszlo Ersek
0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Chao B @ 2018-11-09 6:02 UTC (permalink / raw)
To: edk2-devel
Cc: Andrew Fish, Laszlo Ersek, Leif Lindholm, Michael D Kinney,
Yao Jiewen
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(-)
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index ad2f188b46..66aa8794ac 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -524,10 +524,17 @@ DumpPtpInfo (
Vid = 0xFFFF;
Did = 0xFFFF;
Rid = 0xFF;
PtpInterface = PcdGet8(PcdActiveTpmInterfaceType);
+ if (PtpInterface == 0xFE) {
+ //
+ // TPM interface type cache disabled. Always read Interface type from TPM
+ //
+ PtpInterface = Tpm2GetPtpInterface (Register);
+ }
+
DEBUG ((EFI_D_INFO, "PtpInterface - %x\n", PtpInterface));
switch (PtpInterface) {
case Tpm2PtpInterfaceCrb:
Vid = MmioRead16 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->Vid);
Did = MmioRead16 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->Did);
@@ -568,11 +575,18 @@ DTpm2SubmitCommand (
IN UINT8 *OutputParameterBlock
)
{
TPM2_PTP_INTERFACE_TYPE PtpInterface;
- PtpInterface = PcdGet8(PcdActiveTpmInterfaceType);
+ PtpInterface = PcdGet8(PcdActiveTpmInterfaceType);
+ if (PtpInterface == 0xFE) {
+ //
+ // Always read Interface type from TPM to get more device compatibility
+ //
+ PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+ }
+
switch (PtpInterface) {
case Tpm2PtpInterfaceCrb:
return PtpCrbTpmCommand (
(PTP_CRB_REGISTERS_PTR) (UINTN) PcdGet64 (PcdTpmBaseAddress),
InputParameterBlock,
@@ -608,10 +622,17 @@ DTpm2RequestUseTpm (
)
{
TPM2_PTP_INTERFACE_TYPE PtpInterface;
PtpInterface = PcdGet8(PcdActiveTpmInterfaceType);
+ if (PtpInterface == 0xFE) {
+ //
+ // Always read Interface type from TPM to get more device compatibility
+ //
+ PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+ }
+
switch (PtpInterface) {
case Tpm2PtpInterfaceCrb:
return PtpCrbRequestUseTpm ((PTP_CRB_REGISTERS_PTR) (UINTN) PcdGet64 (PcdTpmBaseAddress));
case Tpm2PtpInterfaceFifo:
case Tpm2PtpInterfaceTis:
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 8d64b4fefe..2aef4ba128 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -467,11 +467,12 @@
## This PCD indicates current active TPM interface type.
# Accodingt to TCG PTP spec 1.3, there are 3 types defined in TPM2_PTP_INTERFACE_TYPE.<BR>
# 0x00 - FIFO interface as defined in TIS 1.3 is active.<BR>
# 0x01 - FIFO interface as defined in PTP for TPM 2.0 is active.<BR>
# 0x02 - CRB interface is active.<BR>
- # 0xFF - Contains no current active TPM interface type.<BR>
+ # 0xFE - Disable TPM interface type cache feature.<BR>
+ # 0xFF - Enable TPM interface cache and contain no current active TPM interface type.<BR>
#
# @Prompt current active TPM interface type.
gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType|0xFF|UINT8|0x0001001E
## This PCD records IdleByass status supported by current active TPM interface.
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
index 400fe6015e..44182bb62a 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -252,11 +252,12 @@
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdActiveTpmInterfaceType_HELP #language en-US "This PCD indicates current active TPM interface type.\n"
"0x00 - FIFO interface as defined in TIS 1.3 is active.<BR>\n"
"0x01 - FIFO interface as defined in PTP for TPM 2.0 is active.<BR>\n"
"0x02 - CRB interface is active.<BR>\n"
- "0xFF - Contains no current active TPM interface type<BR>"
+ "0xFE - Disable TPM interface type cache to get more device compatibility.<BR>\n"
+ "0xFF - Enable TPM interface cache and contain no current active TPM interface type<BR>"
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdCRBIdleByPass_PROMPT #language en-US "IdleByass status supported by current active TPM interface."
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdCRBIdleByPass_HELP #language en-US "This PCD records IdleByass status supported by current active TPM interface.\n"
"Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and diretcly move to CmdReady state. <BR>"
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index 24ce3d29e1..61e2720953 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -60,10 +60,45 @@ HII_VENDOR_DEVICE_PATH mTcg2HiiVendorDevicePath = {
}
};
UINT8 mCurrentPpRequest;
+/**
+ Return PTP interface type.
+
+ @param[in] Register Pointer to PTP register.
+
+ @return PTP interface type.
+**/
+TPM2_PTP_INTERFACE_TYPE
+GetPtpInterface (
+ IN VOID *Register
+ )
+{
+ PTP_CRB_INTERFACE_IDENTIFIER InterfaceId;
+ PTP_FIFO_INTERFACE_CAPABILITY InterfaceCapability;
+
+ //
+ // Check interface id
+ //
+ InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
+ InterfaceCapability.Uint32 = MmioRead32 ((UINTN)&((PTP_FIFO_REGISTERS *)Register)->InterfaceCapability);
+
+ if ((InterfaceId.Bits.InterfaceType == PTP_INTERFACE_IDENTIFIER_INTERFACE_TYPE_CRB) &&
+ (InterfaceId.Bits.InterfaceVersion == PTP_INTERFACE_IDENTIFIER_INTERFACE_VERSION_CRB) &&
+ (InterfaceId.Bits.CapCRB != 0)) {
+ return Tpm2PtpInterfaceCrb;
+ }
+ if ((InterfaceId.Bits.InterfaceType == PTP_INTERFACE_IDENTIFIER_INTERFACE_TYPE_FIFO) &&
+ (InterfaceId.Bits.InterfaceVersion == PTP_INTERFACE_IDENTIFIER_INTERFACE_VERSION_FIFO) &&
+ (InterfaceId.Bits.CapFIFO != 0) &&
+ (InterfaceCapability.Bits.InterfaceVersion == INTERFACE_CAPABILITY_INTERFACE_VERSION_PTP)) {
+ return Tpm2PtpInterfaceFifo;
+ }
+ return Tpm2PtpInterfaceTis;
+}
+
/**
Return if PTP CRB is supported.
@param[in] Register Pointer to PTP register.
@@ -138,10 +173,17 @@ SetPtpInterface (
{
TPM2_PTP_INTERFACE_TYPE PtpInterfaceCurrent;
PTP_CRB_INTERFACE_IDENTIFIER InterfaceId;
PtpInterfaceCurrent = PcdGet8(PcdActiveTpmInterfaceType);
+ if (PtpInterfaceCurrent == 0xFE) {
+ //
+ // Always read Interface type from TPM to get more device compatibility
+ //
+ PtpInterfaceCurrent = GetPtpInterface (Register);
+ }
+
if ((PtpInterfaceCurrent != Tpm2PtpInterfaceFifo) &&
(PtpInterfaceCurrent != Tpm2PtpInterfaceCrb)) {
return EFI_UNSUPPORTED;
}
InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
@@ -897,10 +939,17 @@ InstallTcg2ConfigForm (
//
// Update TPM device interface type
//
if (PrivateData->TpmDeviceDetected == TPM_DEVICE_2_0_DTPM) {
TpmDeviceInterfaceDetected = PcdGet8(PcdActiveTpmInterfaceType);
+ if (TpmDeviceInterfaceDetected == 0xFE) {
+ //
+ // TPM interface type cache disabled. Always read Interface type from TPM
+ //
+ TpmDeviceInterfaceDetected = GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+ }
+
switch (TpmDeviceInterfaceDetected) {
case Tpm2PtpInterfaceTis:
HiiSetString (PrivateData->HiiHandle, STRING_TOKEN (STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT), L"TIS", NULL);
break;
case Tpm2PtpInterfaceFifo:
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 4a1a293bfc..cf172c3053 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -40,10 +40,45 @@ EFI_TPM2_ACPI_TABLE mTpm2AcpiTemplate = {
};
EFI_SMM_VARIABLE_PROTOCOL *mSmmVariable;
TCG_NVS *mTcgNvs;
+/**
+ Return PTP interface type.
+
+ @param[in] Register Pointer to PTP register.
+
+ @return PTP interface type.
+**/
+TPM2_PTP_INTERFACE_TYPE
+GetPtpInterface (
+ IN VOID *Register
+ )
+{
+ PTP_CRB_INTERFACE_IDENTIFIER InterfaceId;
+ PTP_FIFO_INTERFACE_CAPABILITY InterfaceCapability;
+
+ //
+ // Check interface id
+ //
+ InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
+ InterfaceCapability.Uint32 = MmioRead32 ((UINTN)&((PTP_FIFO_REGISTERS *)Register)->InterfaceCapability);
+
+ if ((InterfaceId.Bits.InterfaceType == PTP_INTERFACE_IDENTIFIER_INTERFACE_TYPE_CRB) &&
+ (InterfaceId.Bits.InterfaceVersion == PTP_INTERFACE_IDENTIFIER_INTERFACE_VERSION_CRB) &&
+ (InterfaceId.Bits.CapCRB != 0)) {
+ return Tpm2PtpInterfaceCrb;
+ }
+ if ((InterfaceId.Bits.InterfaceType == PTP_INTERFACE_IDENTIFIER_INTERFACE_TYPE_FIFO) &&
+ (InterfaceId.Bits.InterfaceVersion == PTP_INTERFACE_IDENTIFIER_INTERFACE_VERSION_FIFO) &&
+ (InterfaceId.Bits.CapFIFO != 0) &&
+ (InterfaceCapability.Bits.InterfaceVersion == INTERFACE_CAPABILITY_INTERFACE_VERSION_PTP)) {
+ return Tpm2PtpInterfaceFifo;
+ }
+ return Tpm2PtpInterfaceTis;
+}
+
/**
Software SMI callback for TPM physical presence which is called from ACPI method.
Caution: This function may receive untrusted input.
Variable and ACPINvs are external input, so this function will validate
@@ -765,10 +800,17 @@ PublishTpm2 (
&mTpm2AcpiTemplate,
sizeof(mTpm2AcpiTemplate)
);
InterfaceType = PcdGet8(PcdActiveTpmInterfaceType);
+ if (InterfaceType == 0xFE) {
+ //
+ // TPM interface type cache disabled. Always read Interface type from TPM
+ //
+ InterfaceType = GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+ }
+
switch (InterfaceType) {
case Tpm2PtpInterfaceCrb:
mTpm2AcpiTemplate.StartMethod = EFI_TPM2_ACPI_TABLE_START_METHOD_COMMAND_RESPONSE_BUFFER_INTERFACE;
mTpm2AcpiTemplate.AddressOfControlArea = PcdGet64 (PcdTpmBaseAddress) + 0x40;
ControlArea = (EFI_TPM2_ACPI_CONTROL_AREA *)(UINTN)mTpm2AcpiTemplate.AddressOfControlArea;
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 6:02 [Patch] SecurityPkg: Fix TPM device compatibility issue Zhang, Chao B
@ 2018-11-09 8:04 ` Laszlo Ersek
2018-11-09 11:13 ` Leif Lindholm
2018-11-09 14:40 ` Zhang, Chao B
0 siblings, 2 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-11-09 8:04 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel
Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Yao Jiewen
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.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 8:04 ` Laszlo Ersek
@ 2018-11-09 11:13 ` Leif Lindholm
2018-11-09 14:46 ` Zhang, Chao B
2018-11-09 14:40 ` Zhang, Chao B
1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-11-09 11:13 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Zhang, Chao B, edk2-devel, Andrew Fish, Michael D Kinney,
Yao Jiewen
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 8:04 ` Laszlo Ersek
2018-11-09 11:13 ` Leif Lindholm
@ 2018-11-09 14:40 ` Zhang, Chao B
1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Chao B @ 2018-11-09 14:40 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Cc: Andrew Fish, Leif Lindholm, Kinney, Michael D, Yao, Jiewen
Hi All:
Let me introduce more background.
We enabled Interface type Cache feature because it complies with TCG PTP 1.03 spec (also earlier PTP 00.43) and reduces traffic to communicate with TPM.
It by chance addresses defect within some TPM2.0 device that frequent touching InterfaceID register could cause permanent damage.
But in our recent test, we found other device compatibility issue. Using interface cache and skipping touching real hardware will cause NTC 1310 TPM 2.0 malfunction.
In conclusion, I think our Interface Type Cache feature is the right direction, but with the intention to keep device compatibility, we still need to expose enable/disable configuration.
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Friday, November 9, 2018 4:05 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>; edk2-devel@lists.01.org
Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
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<mailto:chao.b.zhang@intel.com>>
> Cc: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Yao Jiewen <jiewen.yao@intel.com<mailto: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.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 11:13 ` Leif Lindholm
@ 2018-11-09 14:46 ` Zhang, Chao B
2018-11-09 15:55 ` Leif Lindholm
2018-11-09 16:21 ` Laszlo Ersek
0 siblings, 2 replies; 10+ messages in thread
From: Zhang, Chao B @ 2018-11-09 14:46 UTC (permalink / raw)
To: Leif Lindholm, Laszlo Ersek
Cc: Kinney, Michael D, edk2-devel@lists.01.org, Yao, Jiewen
Hi Leif:
The NTC1310 can work well with previous EDK2 stable version (UDK2018). Interface Cache is a new feature introduced after UDK2018.
So far as we see, it causes NTC1310 fail to work.
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
Sent: Friday, November 9, 2018 7:13 PM
To: Laszlo Ersek <lersek@redhat.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@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.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> > Cc: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
> > Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Cc: Yao Jiewen <jiewen.yao@intel.com<mailto: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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 14:46 ` Zhang, Chao B
@ 2018-11-09 15:55 ` Leif Lindholm
2018-11-09 16:23 ` Laszlo Ersek
2018-11-09 16:21 ` Laszlo Ersek
1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-11-09 15:55 UTC (permalink / raw)
To: Zhang, Chao B
Cc: Laszlo Ersek, Kinney, Michael D, edk2-devel@lists.01.org,
Yao, Jiewen
On Fri, Nov 09, 2018 at 02:46:40PM +0000, Zhang, Chao B wrote:
> Hi Leif:
> The NTC1310 can work well with previous EDK2 stable version
> (UDK2018). Interface Cache is a new feature introduced after
> UDK2018.
> So far as we see, it causes NTC1310 fail to work.
OK, in that case I am willing to change my opinion from "no" to "I
would prefer not".
Again, this patch went into the tree on 25 June. It was part of
edk2-stable201808, but we only see a report during the hard freeze
period preceding edk2-stable201811.
My concern is this:
This patch _will_ affect platforms other than the ones displaying the
erroneous behaviour, and all affected platforms can manually apply
this patch until it shows up in edk2-stable201902.
Anyone not working against the stable tags can have it available in
master a week from now.
If the package maintainers do consider this a release critical bug,
it needs a bugzilla entry (referenced in the commit message). The
style issues should also be addressed.
Regards,
Leif
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
> Sent: Friday, November 9, 2018 7:13 PM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@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.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> > > Cc: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
> > > Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > > Cc: Yao Jiewen <jiewen.yao@intel.com<mailto: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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 14:46 ` Zhang, Chao B
2018-11-09 15:55 ` Leif Lindholm
@ 2018-11-09 16:21 ` Laszlo Ersek
2018-11-09 17:22 ` Kinney, Michael D
1 sibling, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2018-11-09 16:21 UTC (permalink / raw)
To: Zhang, Chao B, Leif Lindholm
Cc: Kinney, Michael D, edk2-devel@lists.01.org, Yao, Jiewen,
Andrew Fish
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-bounces@lists.01.org] On Behalf Of Leif Lindholm
> Sent: Friday, November 9, 2018 7:13 PM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@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.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
>>> Cc: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
>>> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
>>> Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>>> Cc: Yao Jiewen <jiewen.yao@intel.com<mailto: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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto: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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 15:55 ` Leif Lindholm
@ 2018-11-09 16:23 ` Laszlo Ersek
0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-11-09 16:23 UTC (permalink / raw)
To: Leif Lindholm, Zhang, Chao B
Cc: Kinney, Michael D, Yao, Jiewen, edk2-devel@lists.01.org
meta:
On 11/09/18 16:55, Leif Lindholm wrote:
> On Fri, Nov 09, 2018 at 02:46:40PM +0000, Zhang, Chao B wrote:
>> Hi Leif:
>> The NTC1310 can work well with previous EDK2 stable version
>> (UDK2018). Interface Cache is a new feature introduced after
>> UDK2018.
>> So far as we see, it causes NTC1310 fail to work.
>
> OK, in that case I am willing to change my opinion from "no" to "I
> would prefer not".
>
> Again, this patch went into the tree on 25 June. It was part of
> edk2-stable201808, but we only see a report during the hard freeze
> period preceding edk2-stable201811.
(I didn't mean to "steal" this argument -- I'm honestly seeing this
email only now, after sending my email a minute ago.)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 16:21 ` Laszlo Ersek
@ 2018-11-09 17:22 ` Kinney, Michael D
2018-11-10 3:26 ` Ni, Ruiyu
0 siblings, 1 reply; 10+ messages in thread
From: Kinney, Michael D @ 2018-11-09 17:22 UTC (permalink / raw)
To: Laszlo Ersek, Zhang, Chao B, Leif Lindholm, Kinney, Michael D
Cc: edk2-devel@lists.01.org, Yao, Jiewen
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-
> bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Friday, November 9, 2018 8:22 AM
> To: Zhang, Chao B <chao.b.zhang@intel.com>; Leif
> Lindholm <leif.lindholm@linaro.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> edk2-devel@lists.01.org; Yao, Jiewen
> <jiewen.yao@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-
> bounces@lists.01.org] On Behalf Of Leif Lindholm
> > Sent: Friday, November 9, 2018 7:13 PM
> > To: Laszlo Ersek <lersek@redhat.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> edk2-devel@lists.01.org; Yao, Jiewen
> <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@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.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> >>> Cc: Andrew Fish
> <afish@apple.com<mailto:afish@apple.com>>
> >>> Cc: Laszlo Ersek
> <lersek@redhat.com<mailto:lersek@redhat.com>>
> >>> Cc: Leif Lindholm
> <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.o
> rg>>
> >>> Cc: Michael D Kinney
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@int
> el.com>>
> >>> Cc: Yao Jiewen
> <jiewen.yao@intel.com<mailto: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
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto: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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] SecurityPkg: Fix TPM device compatibility issue
2018-11-09 17:22 ` Kinney, Michael D
@ 2018-11-10 3:26 ` Ni, Ruiyu
0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ruiyu @ 2018-11-10 3:26 UTC (permalink / raw)
To: Kinney, Michael D
Cc: Laszlo Ersek, Zhang, Chao B, Leif Lindholm,
edk2-devel@lists.01.org, Yao, Jiewen
Does it mean the current master code is good enough?
Sent from small device
> 在 2018年11月10日,上午1:23,Kinney, Michael D <michael.d.kinney@intel.com> 写道:
>
> 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-
>> bounces@lists.01.org] On Behalf Of Laszlo Ersek
>> Sent: Friday, November 9, 2018 8:22 AM
>> To: Zhang, Chao B <chao.b.zhang@intel.com>; Leif
>> Lindholm <leif.lindholm@linaro.org>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
>> edk2-devel@lists.01.org; Yao, Jiewen
>> <jiewen.yao@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-
>> bounces@lists.01.org] On Behalf Of Leif Lindholm
>>> Sent: Friday, November 9, 2018 7:13 PM
>>> To: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
>> edk2-devel@lists.01.org; Yao, Jiewen
>> <jiewen.yao@intel.com>; Zhang, Chao B
>> <chao.b.zhang@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.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
>>>>> Cc: Andrew Fish
>> <afish@apple.com<mailto:afish@apple.com>>
>>>>> Cc: Laszlo Ersek
>> <lersek@redhat.com<mailto:lersek@redhat.com>>
>>>>> Cc: Leif Lindholm
>> <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.o
>> rg>>
>>>>> Cc: Michael D Kinney
>> <michael.d.kinney@intel.com<mailto:michael.d.kinney@int
>> el.com>>
>>>>> Cc: Yao Jiewen
>> <jiewen.yao@intel.com<mailto: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
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org<mailto: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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-10 3:26 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-09 6:02 [Patch] SecurityPkg: Fix TPM device compatibility issue Zhang, Chao B
2018-11-09 8:04 ` Laszlo Ersek
2018-11-09 11:13 ` Leif Lindholm
2018-11-09 14:46 ` Zhang, Chao B
2018-11-09 15:55 ` Leif Lindholm
2018-11-09 16:23 ` Laszlo Ersek
2018-11-09 16:21 ` Laszlo Ersek
2018-11-09 17:22 ` Kinney, Michael D
2018-11-10 3:26 ` Ni, Ruiyu
2018-11-09 14:40 ` Zhang, Chao B
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox