[Patch] SecurityPkg: Fix TPM device compatibility issue

* [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