public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/2] Add CRB IdleByPass Support
@ 2018-06-25  4:44 Zhang, Chao B
  2018-06-25  4:44 ` [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register Zhang, Chao B
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Zhang, Chao B @ 2018-06-25  4:44 UTC (permalink / raw)
  To: edk2-devel

Add CRB IdleByPass Support

Zhang, Chao B (2):
  Add CapCRBIdleBypass definition to interface ID register. It complies
    with existing register
  SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support

 MdePkg/Include/IndustryStandard/TpmPtp.h           |  5 +-
 .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  | 19 +++++
 .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf        |  1 +
 .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c        | 19 +++++
 .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf      |  3 +-
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c    | 98 +++++++++++++++++++---
 SecurityPkg/SecurityPkg.dec                        | 10 +++
 SecurityPkg/SecurityPkg.uni                        | 10 ++-
 8 files changed, 149 insertions(+), 16 deletions(-)

-- 
2.16.2.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register
  2018-06-25  4:44 [Patch 0/2] Add CRB IdleByPass Support Zhang, Chao B
@ 2018-06-25  4:44 ` Zhang, Chao B
  2018-06-25  4:44 ` [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support Zhang, Chao B
  2018-06-27  0:55 ` [Patch 0/2] Add CRB IdleByPass Support Long, Qin
  2 siblings, 0 replies; 6+ messages in thread
From: Zhang, Chao B @ 2018-06-25  4:44 UTC (permalink / raw)
  To: edk2-devel

Signed-off-by: Zhang, Chao B <chao.b.zhang@intel.com>
---
 MdePkg/Include/IndustryStandard/TpmPtp.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/TpmPtp.h b/MdePkg/Include/IndustryStandard/TpmPtp.h
index 0796512688..c7ff8fdc58 100644
--- a/MdePkg/Include/IndustryStandard/TpmPtp.h
+++ b/MdePkg/Include/IndustryStandard/TpmPtp.h
@@ -1,10 +1,10 @@
 /** @file
   Platform TPM Profile Specification definition for TPM2.0.
   It covers both FIFO and CRB interface.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -336,11 +336,12 @@ typedef PTP_CRB_REGISTERS  *PTP_CRB_REGISTERS_PTR;
 typedef union {
   struct {
     UINT32   InterfaceType:4;
     UINT32   InterfaceVersion:4;
     UINT32   CapLocality:1;
-    UINT32   Reserved1:2;
+    UINT32   CapCRBIdleBypass:1;
+    UINT32   Reserved1:1;
     UINT32   CapDataXferSizeSupport:2;
     UINT32   CapFIFO:1;
     UINT32   CapCRB:1;
     UINT32   CapIFRes:2;
     UINT32   InterfaceSelector:2;
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
  2018-06-25  4:44 [Patch 0/2] Add CRB IdleByPass Support Zhang, Chao B
  2018-06-25  4:44 ` [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register Zhang, Chao B
@ 2018-06-25  4:44 ` Zhang, Chao B
  2018-06-27  8:18   ` Gary Lin
  2018-06-27  0:55 ` [Patch 0/2] Add CRB IdleByPass Support Long, Qin
  2 siblings, 1 reply; 6+ messages in thread
From: Zhang, Chao B @ 2018-06-25  4:44 UTC (permalink / raw)
  To: edk2-devel; +Cc: Long Qin, Yao Jiewen, Chao Zhang

Directly transition from CMD completion to CMD Ready state if device
supports IdleByPass

Cc: Long Qin <qin.long@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Zhang, Chao B <chao.b.zhang@intel.com>
---
 .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  | 19 +++++
 .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf        |  1 +
 .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c        | 19 +++++
 .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf      |  3 +-
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c    | 98 +++++++++++++++++++---
 SecurityPkg/SecurityPkg.dec                        | 10 +++
 SecurityPkg/SecurityPkg.uni                        | 10 ++-
 7 files changed, 146 insertions(+), 14 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
index 3feb64df7e..e6fe563b40 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
@@ -29,10 +29,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 TPM2_PTP_INTERFACE_TYPE
 Tpm2GetPtpInterface (
   IN VOID *Register
   );
 
+/**
+  Return PTP CRB interface IdleByPass state.
+
+  @param[in] Register                Pointer to PTP register.
+
+  @return PTP CRB interface IdleByPass state.
+**/
+UINT8
+Tpm2GetIdleByPass (
+  IN VOID *Register
+  );
+
 /**
   This service enables the sending of commands to the TPM2.
 
   @param[in]      InputParameterBlockSize  Size of the TPM2 input parameter block.
   @param[in]      InputParameterBlock      Pointer to the TPM2 input parameter block.
@@ -138,15 +150,22 @@ EFIAPI
 Tpm2DeviceLibConstructor (
   VOID
   )
 {
   TPM2_PTP_INTERFACE_TYPE  PtpInterface;
+  UINT8                    IdleByPass;
 
   //
   // Cache current active TpmInterfaceType only when needed
   //
   if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
     PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
     PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
   }
+
+  if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
+    IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+    PcdSet8S(PcdCRBIdleByPass, IdleByPass);
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
index 634bbae847..2e54a78cc0 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
@@ -53,5 +53,6 @@
   PcdLib
 
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress            ## CONSUMES
   gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType    ## PRODUCES
+  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
\ No newline at end of file
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
index 01f78bf0be..edcdb72a79 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
@@ -32,10 +32,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 TPM2_PTP_INTERFACE_TYPE
 Tpm2GetPtpInterface (
   IN VOID *Register
   );
 
+/**
+  Return PTP CRB interface IdleByPass state.
+
+  @param[in] Register                Pointer to PTP register.
+
+  @return PTP CRB interface IdleByPass state.
+**/
+UINT8
+Tpm2GetIdleByPass (
+  IN VOID *Register
+  );
+
 /**
   Dump PTP register information.
 
   @param[in] Register                Pointer to PTP register.
 **/
@@ -95,10 +107,11 @@ Tpm2InstanceLibDTpmConstructor (
   VOID
   )
 {
   EFI_STATUS               Status;
   TPM2_PTP_INTERFACE_TYPE  PtpInterface;
+  UINT8                    IdleByPass;
 
   Status = Tpm2RegisterTpm2DeviceLib (&mDTpm2InternalTpm2Device);
   if ((Status == EFI_SUCCESS) || (Status == EFI_UNSUPPORTED)) {
     //
     // Unsupported means platform policy does not need this instance enabled.
@@ -109,10 +122,16 @@ Tpm2InstanceLibDTpmConstructor (
       //
       if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
         PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
         PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
       }
+
+      if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
+        IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
+        PcdSet8S(PcdCRBIdleByPass, IdleByPass);
+      }
+
       DumpPtpInfo ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
     }
     return EFI_SUCCESS;
   }
   return Status;
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
index 876a5a63c4..24e4c35d55 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
@@ -48,6 +48,7 @@
   DebugLib
   PcdLib
 
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress          ## CONSUMES
-  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
\ No newline at end of file
+  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
+  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
\ No newline at end of file
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 1bc153a2c0..5bce9f8e02 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -172,14 +172,34 @@ PtpCrbTpmCommand (
         DEBUG ((EFI_D_VERBOSE, "%02x ", BufferIn[Index]));
       }
     }
     DEBUG ((EFI_D_VERBOSE, "\n"));
   );
-  TpmOutSize = 0;
+  TpmOutSize         = 0;
 
   //
   // STEP 0:
+  // if CapCRbIdelByPass == 0, enforce Idle state before sending command
+  //
+  if (PcdGet8(PcdCRBIdleByPass) == 0 && (MmioRead32((UINTN)&CrbReg->CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0){ 
+    Status = PtpCrbWaitRegisterBits (
+              &CrbReg->CrbControlStatus,
+              PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+              0,
+              PTP_TIMEOUT_C
+              );
+    if (EFI_ERROR (Status)) {
+      //
+      // Try to goIdle to recover TPM
+      //
+      Status = EFI_DEVICE_ERROR;
+      goto GoIdle_Exit;
+    }
+  }
+
+  //
+  // STEP 1:
   // Ready is any time the TPM is ready to receive a command, following a write
   // of 1 by software to Request.cmdReady, as indicated by the Status field
   // being cleared to 0.
   //
   MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
@@ -189,25 +209,25 @@ PtpCrbTpmCommand (
              PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
              PTP_TIMEOUT_C
              );
   if (EFI_ERROR (Status)) {
     Status = EFI_DEVICE_ERROR;
-    goto Exit;
+    goto GoIdle_Exit;
   }
   Status = PtpCrbWaitRegisterBits (
              &CrbReg->CrbControlStatus,
              0,
              PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
              PTP_TIMEOUT_C
              );
   if (EFI_ERROR (Status)) {
     Status = EFI_DEVICE_ERROR;
-    goto Exit;
+    goto GoIdle_Exit;
   }
 
   //
-  // STEP 1:
+  // STEP 2:
   // Command Reception occurs following a Ready state between the write of the
   // first byte of a command to the Command Buffer and the receipt of a write
   // of 1 to Start.
   //
   for (Index = 0; Index < SizeIn; Index++) {
@@ -219,11 +239,11 @@ PtpCrbTpmCommand (
 
   MmioWrite64 ((UINTN)&CrbReg->CrbControlResponseAddrss, (UINT32)(UINTN)CrbReg->CrbDataBuffer);
   MmioWrite32 ((UINTN)&CrbReg->CrbControlResponseSize, sizeof(CrbReg->CrbDataBuffer));
 
   //
-  // STEP 2:
+  // STEP 3:
   // Command Execution occurs after receipt of a 1 to Start and the TPM
   // clearing Start to 0.
   //
   MmioWrite32((UINTN)&CrbReg->CrbControlStart, PTP_CRB_CONTROL_START);
   Status = PtpCrbWaitRegisterBits (
@@ -249,16 +269,16 @@ PtpCrbTpmCommand (
     if (EFI_ERROR(Status)) {
       //
       // Still in Command Execution state. Try to goIdle, the behavior is agnostic.
       //
       Status = EFI_DEVICE_ERROR;
-      goto Exit;
+      goto GoIdle_Exit;
     }
   }
 
   //
-  // STEP 3:
+  // STEP 4:
   // Command Completion occurs after completion of a command (indicated by the
   // TPM clearing TPM_CRB_CTRL_Start_x to 0) and before a write of a 1 by the
   // software to Request.goIdle.
   //
 
@@ -281,40 +301,72 @@ PtpCrbTpmCommand (
   CopyMem (&Data16, BufferOut, sizeof (UINT16));
   // TPM2 should not use this RSP_COMMAND
   if (SwapBytes16 (Data16) == TPM_ST_RSP_COMMAND) {
     DEBUG ((EFI_D_ERROR, "TPM2: TPM_ST_RSP error - %x\n", TPM_ST_RSP_COMMAND));
     Status = EFI_UNSUPPORTED;
-    goto Exit;
+    goto GoIdle_Exit;
   }
 
   CopyMem (&Data32, (BufferOut + 2), sizeof (UINT32));
   TpmOutSize  = SwapBytes32 (Data32);
   if (*SizeOut < TpmOutSize) {
+    //
+    // Command completed, but buffer is not enough
+    //
     Status = EFI_BUFFER_TOO_SMALL;
-    goto Exit;
+    goto GoReady_Exit;
   }
   *SizeOut = TpmOutSize;
   //
   // Continue reading the remaining data
   //
   for (Index = sizeof (TPM2_RESPONSE_HEADER); Index < TpmOutSize; Index++) {
     BufferOut[Index] = MmioRead8 ((UINTN)&CrbReg->CrbDataBuffer[Index]);
   }
-Exit:
+
   DEBUG_CODE (
     DEBUG ((EFI_D_VERBOSE, "PtpCrbTpmCommand Receive - "));
     for (Index = 0; Index < TpmOutSize; Index++) {
       DEBUG ((EFI_D_VERBOSE, "%02x ", BufferOut[Index]));
     }
     DEBUG ((EFI_D_VERBOSE, "\n"));
   );
 
+GoReady_Exit:
   //
-  // STEP 4:
-  // Idle is any time TPM_CRB_CTRL_STS_x.Status.goIdle is 1.
+  // Goto Ready State if command is completed succesfully and TPM support IdleBypass
+  // If not supported. flow down to GoIdle
+  //
+  if (PcdGet8(PcdCRBIdleByPass) == 1) {
+    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
+    return Status;
+  }
+
+  //
+  // Do not wait for state transition for TIMEOUT_C
+  // This function will try to wait 2 TIMEOUT_C at the beginning in next call. 
+  //
+GoIdle_Exit:
+
+  //
+  //  Return to Idle state by setting TPM_CRB_CTRL_STS_x.Status.goIdle to 1.
   //
   MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+
+  //
+  // Only enforce Idle state transition if execution fails when CRBIndleBypass==1 
+  // Leave regular Idle delay at the beginning of next command execution
+  //
+  if (PcdGet8(PcdCRBIdleByPass) == 1){
+    Status = PtpCrbWaitRegisterBits (
+               &CrbReg->CrbControlStatus,
+               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+               0,
+               PTP_TIMEOUT_C
+               );
+  }
+
   return Status;
 }
 
 /**
   Send a command to TPM for execution and return response data.
@@ -392,10 +444,32 @@ Tpm2GetPtpInterface (
     return Tpm2PtpInterfaceFifo;
   }
   return Tpm2PtpInterfaceTis;
 }
 
+/**
+  Return PTP CRB interface IdleByPass state.
+
+  @param[in] Register                Pointer to PTP register.
+
+  @return PTP CRB interface IdleByPass state.
+**/
+UINT8
+Tpm2GetIdleByPass (
+  IN VOID *Register
+  )
+{
+  PTP_CRB_INTERFACE_IDENTIFIER  InterfaceId;
+
+  //
+  // Check interface id
+  //
+  InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
+
+  return (UINT8)(InterfaceId.Bits.CapCRBIdleBypass);
+}
+
 /**
   Dump PTP register information.
 
   @param[in] Register                Pointer to PTP register.
 **/
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 60f1c0a0e3..e24b563bdb 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -472,7 +472,17 @@
   #  0xFF - Contains 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.
+  #  Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and 
+  #  diretcly move to CmdReady state. <BR>
+  #  0x00 - Do not support IdleByPass.<BR>
+  #  0x01 - Support IdleByPass.<BR>
+  #  0xFF - IdleByPass State is not synced with TPM hardware.<BR>
+  #
+  # @Prompt IdleByass status supported by current active TPM interface.
+  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass|0xFF|UINT8|0x0001001F
+
 [UserExtensions.TianoCore."ExtraFiles"]
   SecurityPkgExtra.uni
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
index c34250e423..000bc83d80 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -252,6 +252,14 @@
 
 #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>"
\ No newline at end of file
+                                                                                          "0xFF - Contains 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>"
+                                                                                          "0x01 - Do not support IdleByPass.<BR>\n"
+                                                                                          "0x02 - Support IdleByPass.<BR>\n"
+                                                                                          "0xFF - IdleByPass State is not synced with TPM hardware.<BR>"
\ No newline at end of file
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch 0/2] Add CRB IdleByPass Support
  2018-06-25  4:44 [Patch 0/2] Add CRB IdleByPass Support Zhang, Chao B
  2018-06-25  4:44 ` [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register Zhang, Chao B
  2018-06-25  4:44 ` [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support Zhang, Chao B
@ 2018-06-27  0:55 ` Long, Qin
  2 siblings, 0 replies; 6+ messages in thread
From: Long, Qin @ 2018-06-27  0:55 UTC (permalink / raw)
  To: Zhang, Chao B, edk2-devel@lists.01.org

Series Reviewed-by: Long Qin <qin.long@intel.com>


Best Regards & Thanks,
LONG, Qin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Zhang, Chao B
> Sent: Monday, June 25, 2018 12:44 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] Add CRB IdleByPass Support
> 
> Add CRB IdleByPass Support
> 
> Zhang, Chao B (2):
>   Add CapCRBIdleBypass definition to interface ID register. It complies
>     with existing register
>   SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
> 
>  MdePkg/Include/IndustryStandard/TpmPtp.h           |  5 +-
>  .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf        |  1 +
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c        | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf      |  3 +-
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c    | 98
> +++++++++++++++++++---
>  SecurityPkg/SecurityPkg.dec                        | 10 +++
>  SecurityPkg/SecurityPkg.uni                        | 10 ++-
>  8 files changed, 149 insertions(+), 16 deletions(-)
> 
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
  2018-06-25  4:44 ` [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support Zhang, Chao B
@ 2018-06-27  8:18   ` Gary Lin
  2018-07-03 23:25     ` Zhang, Chao B
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Lin @ 2018-06-27  8:18 UTC (permalink / raw)
  To: Zhang, Chao B; +Cc: edk2-devel, Yao Jiewen, Long Qin

On Mon, Jun 25, 2018 at 12:44:21PM +0800, Zhang, Chao B wrote:
> Directly transition from CMD completion to CMD Ready state if device
> supports IdleByPass
> 
> Cc: Long Qin <qin.long@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Zhang, Chao B <chao.b.zhang@intel.com>
> ---
>  .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf        |  1 +
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c        | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf      |  3 +-
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c    | 98 +++++++++++++++++++---
>  SecurityPkg/SecurityPkg.dec                        | 10 +++
>  SecurityPkg/SecurityPkg.uni                        | 10 ++-
>  7 files changed, 146 insertions(+), 14 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> index 3feb64df7e..e6fe563b40 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> @@ -29,10 +29,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  TPM2_PTP_INTERFACE_TYPE
>  Tpm2GetPtpInterface (
>    IN VOID *Register
>    );
>  
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  );
> +
>  /**
>    This service enables the sending of commands to the TPM2.
>  
>    @param[in]      InputParameterBlockSize  Size of the TPM2 input parameter block.
>    @param[in]      InputParameterBlock      Pointer to the TPM2 input parameter block.
> @@ -138,15 +150,22 @@ EFIAPI
>  Tpm2DeviceLibConstructor (
>    VOID
>    )
>  {
>    TPM2_PTP_INTERFACE_TYPE  PtpInterface;
> +  UINT8                    IdleByPass;
>  
>    //
>    // Cache current active TpmInterfaceType only when needed
>    //
>    if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
>      PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>      PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
>    }
> +
> +  if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
I got a build error with PtpInterfaceCrb:

SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c: In function ‘Tpm2DeviceLibConstructor’:
SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c:165:45: error: ‘PtpInterfaceCrb’ undeclared (first use in this function); did you mean ‘PtpInterface’?
   if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
                                             ^~~~~~~~~~~~~~~
                                             PtpInterface

I assume you mean Tpm2PtpInterfaceCrb?

Cheers,

Gary Lin

> +    IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> +    PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> +  }
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> index 634bbae847..2e54a78cc0 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> @@ -53,5 +53,6 @@
>    PcdLib
>  
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress            ## CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType    ## PRODUCES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> index 01f78bf0be..edcdb72a79 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> @@ -32,10 +32,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  TPM2_PTP_INTERFACE_TYPE
>  Tpm2GetPtpInterface (
>    IN VOID *Register
>    );
>  
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  );
> +
>  /**
>    Dump PTP register information.
>  
>    @param[in] Register                Pointer to PTP register.
>  **/
> @@ -95,10 +107,11 @@ Tpm2InstanceLibDTpmConstructor (
>    VOID
>    )
>  {
>    EFI_STATUS               Status;
>    TPM2_PTP_INTERFACE_TYPE  PtpInterface;
> +  UINT8                    IdleByPass;
>  
>    Status = Tpm2RegisterTpm2DeviceLib (&mDTpm2InternalTpm2Device);
>    if ((Status == EFI_SUCCESS) || (Status == EFI_UNSUPPORTED)) {
>      //
>      // Unsupported means platform policy does not need this instance enabled.
> @@ -109,10 +122,16 @@ Tpm2InstanceLibDTpmConstructor (
>        //
>        if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
>          PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>          PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
>        }
> +
> +      if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
> +        IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> +        PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> +      }
> +
>        DumpPtpInfo ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>      }
>      return EFI_SUCCESS;
>    }
>    return Status;
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> index 876a5a63c4..24e4c35d55 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> @@ -48,6 +48,7 @@
>    DebugLib
>    PcdLib
>  
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress          ## CONSUMES
> -  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
> \ No newline at end of file
> +  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1bc153a2c0..5bce9f8e02 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -172,14 +172,34 @@ PtpCrbTpmCommand (
>          DEBUG ((EFI_D_VERBOSE, "%02x ", BufferIn[Index]));
>        }
>      }
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
> -  TpmOutSize = 0;
> +  TpmOutSize         = 0;
>  
>    //
>    // STEP 0:
> +  // if CapCRbIdelByPass == 0, enforce Idle state before sending command
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 0 && (MmioRead32((UINTN)&CrbReg->CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0){ 
> +    Status = PtpCrbWaitRegisterBits (
> +              &CrbReg->CrbControlStatus,
> +              PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> +              0,
> +              PTP_TIMEOUT_C
> +              );
> +    if (EFI_ERROR (Status)) {
> +      //
> +      // Try to goIdle to recover TPM
> +      //
> +      Status = EFI_DEVICE_ERROR;
> +      goto GoIdle_Exit;
> +    }
> +  }
> +
> +  //
> +  // STEP 1:
>    // Ready is any time the TPM is ready to receive a command, following a write
>    // of 1 by software to Request.cmdReady, as indicated by the Status field
>    // being cleared to 0.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> @@ -189,25 +209,25 @@ PtpCrbTpmCommand (
>               PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
>               PTP_TIMEOUT_C
>               );
>    if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>    Status = PtpCrbWaitRegisterBits (
>               &CrbReg->CrbControlStatus,
>               0,
>               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>               PTP_TIMEOUT_C
>               );
>    if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>  
>    //
> -  // STEP 1:
> +  // STEP 2:
>    // Command Reception occurs following a Ready state between the write of the
>    // first byte of a command to the Command Buffer and the receipt of a write
>    // of 1 to Start.
>    //
>    for (Index = 0; Index < SizeIn; Index++) {
> @@ -219,11 +239,11 @@ PtpCrbTpmCommand (
>  
>    MmioWrite64 ((UINTN)&CrbReg->CrbControlResponseAddrss, (UINT32)(UINTN)CrbReg->CrbDataBuffer);
>    MmioWrite32 ((UINTN)&CrbReg->CrbControlResponseSize, sizeof(CrbReg->CrbDataBuffer));
>  
>    //
> -  // STEP 2:
> +  // STEP 3:
>    // Command Execution occurs after receipt of a 1 to Start and the TPM
>    // clearing Start to 0.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlStart, PTP_CRB_CONTROL_START);
>    Status = PtpCrbWaitRegisterBits (
> @@ -249,16 +269,16 @@ PtpCrbTpmCommand (
>      if (EFI_ERROR(Status)) {
>        //
>        // Still in Command Execution state. Try to goIdle, the behavior is agnostic.
>        //
>        Status = EFI_DEVICE_ERROR;
> -      goto Exit;
> +      goto GoIdle_Exit;
>      }
>    }
>  
>    //
> -  // STEP 3:
> +  // STEP 4:
>    // Command Completion occurs after completion of a command (indicated by the
>    // TPM clearing TPM_CRB_CTRL_Start_x to 0) and before a write of a 1 by the
>    // software to Request.goIdle.
>    //
>  
> @@ -281,40 +301,72 @@ PtpCrbTpmCommand (
>    CopyMem (&Data16, BufferOut, sizeof (UINT16));
>    // TPM2 should not use this RSP_COMMAND
>    if (SwapBytes16 (Data16) == TPM_ST_RSP_COMMAND) {
>      DEBUG ((EFI_D_ERROR, "TPM2: TPM_ST_RSP error - %x\n", TPM_ST_RSP_COMMAND));
>      Status = EFI_UNSUPPORTED;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>  
>    CopyMem (&Data32, (BufferOut + 2), sizeof (UINT32));
>    TpmOutSize  = SwapBytes32 (Data32);
>    if (*SizeOut < TpmOutSize) {
> +    //
> +    // Command completed, but buffer is not enough
> +    //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto Exit;
> +    goto GoReady_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
>    // Continue reading the remaining data
>    //
>    for (Index = sizeof (TPM2_RESPONSE_HEADER); Index < TpmOutSize; Index++) {
>      BufferOut[Index] = MmioRead8 ((UINTN)&CrbReg->CrbDataBuffer[Index]);
>    }
> -Exit:
> +
>    DEBUG_CODE (
>      DEBUG ((EFI_D_VERBOSE, "PtpCrbTpmCommand Receive - "));
>      for (Index = 0; Index < TpmOutSize; Index++) {
>        DEBUG ((EFI_D_VERBOSE, "%02x ", BufferOut[Index]));
>      }
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
>  
> +GoReady_Exit:
>    //
> -  // STEP 4:
> -  // Idle is any time TPM_CRB_CTRL_STS_x.Status.goIdle is 1.
> +  // Goto Ready State if command is completed succesfully and TPM support IdleBypass
> +  // If not supported. flow down to GoIdle
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 1) {
> +    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> +    return Status;
> +  }
> +
> +  //
> +  // Do not wait for state transition for TIMEOUT_C
> +  // This function will try to wait 2 TIMEOUT_C at the beginning in next call. 
> +  //
> +GoIdle_Exit:
> +
> +  //
> +  //  Return to Idle state by setting TPM_CRB_CTRL_STS_x.Status.goIdle to 1.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> +
> +  //
> +  // Only enforce Idle state transition if execution fails when CRBIndleBypass==1 
> +  // Leave regular Idle delay at the beginning of next command execution
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 1){
> +    Status = PtpCrbWaitRegisterBits (
> +               &CrbReg->CrbControlStatus,
> +               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> +               0,
> +               PTP_TIMEOUT_C
> +               );
> +  }
> +
>    return Status;
>  }
>  
>  /**
>    Send a command to TPM for execution and return response data.
> @@ -392,10 +444,32 @@ Tpm2GetPtpInterface (
>      return Tpm2PtpInterfaceFifo;
>    }
>    return Tpm2PtpInterfaceTis;
>  }
>  
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  )
> +{
> +  PTP_CRB_INTERFACE_IDENTIFIER  InterfaceId;
> +
> +  //
> +  // Check interface id
> +  //
> +  InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
> +
> +  return (UINT8)(InterfaceId.Bits.CapCRBIdleBypass);
> +}
> +
>  /**
>    Dump PTP register information.
>  
>    @param[in] Register                Pointer to PTP register.
>  **/
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 60f1c0a0e3..e24b563bdb 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -472,7 +472,17 @@
>    #  0xFF - Contains 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.
> +  #  Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and 
> +  #  diretcly move to CmdReady state. <BR>
> +  #  0x00 - Do not support IdleByPass.<BR>
> +  #  0x01 - Support IdleByPass.<BR>
> +  #  0xFF - IdleByPass State is not synced with TPM hardware.<BR>
> +  #
> +  # @Prompt IdleByass status supported by current active TPM interface.
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass|0xFF|UINT8|0x0001001F
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    SecurityPkgExtra.uni
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index c34250e423..000bc83d80 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -252,6 +252,14 @@
>  
>  #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>"
> \ No newline at end of file
> +                                                                                          "0xFF - Contains 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>"
> +                                                                                          "0x01 - Do not support IdleByPass.<BR>\n"
> +                                                                                          "0x02 - Support IdleByPass.<BR>\n"
> +                                                                                          "0xFF - IdleByPass State is not synced with TPM hardware.<BR>"
> \ No newline at end of file
> -- 
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
  2018-06-27  8:18   ` Gary Lin
@ 2018-07-03 23:25     ` Zhang, Chao B
  0 siblings, 0 replies; 6+ messages in thread
From: Zhang, Chao B @ 2018-07-03 23:25 UTC (permalink / raw)
  To: Gary Lin; +Cc: edk2-devel@lists.01.org, Yao, Jiewen, Long, Qin

Hi Gary:
   It is caused by code merge. Tks for notification. We have fixed it.

From: Gary Lin [mailto:glin@suse.com]
Sent: Wednesday, June 27, 2018 4:19 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Long, Qin <qin.long@intel.com>
Subject: Re: [edk2] [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support

On Mon, Jun 25, 2018 at 12:44:21PM +0800, Zhang, Chao B wrote:
> Directly transition from CMD completion to CMD Ready state if device
> supports IdleByPass
>
> Cc: Long Qin <qin.long@intel.com<mailto:qin.long@intel.com>>
> Cc: Yao Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Signed-off-by: Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> ---
>  .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf        |  1 +
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c        | 19 +++++
>  .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf      |  3 +-
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c    | 98 +++++++++++++++++++---
>  SecurityPkg/SecurityPkg.dec                        | 10 +++
>  SecurityPkg/SecurityPkg.uni                        | 10 ++-
>  7 files changed, 146 insertions(+), 14 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> index 3feb64df7e..e6fe563b40 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> @@ -29,10 +29,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  TPM2_PTP_INTERFACE_TYPE
>  Tpm2GetPtpInterface (
>    IN VOID *Register
>    );
>
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  );
> +
>  /**
>    This service enables the sending of commands to the TPM2.
>
>    @param[in]      InputParameterBlockSize  Size of the TPM2 input parameter block.
>    @param[in]      InputParameterBlock      Pointer to the TPM2 input parameter block.
> @@ -138,15 +150,22 @@ EFIAPI
>  Tpm2DeviceLibConstructor (
>    VOID
>    )
>  {
>    TPM2_PTP_INTERFACE_TYPE  PtpInterface;
> +  UINT8                    IdleByPass;
>
>    //
>    // Cache current active TpmInterfaceType only when needed
>    //
>    if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
>      PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>      PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
>    }
> +
> +  if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
I got a build error with PtpInterfaceCrb:

SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c: In function ‘Tpm2DeviceLibConstructor’:
SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c:165:45: error: ‘PtpInterfaceCrb’ undeclared (first use in this function); did you mean ‘PtpInterface’?
   if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
                                             ^~~~~~~~~~~~~~~
                                             PtpInterface

I assume you mean Tpm2PtpInterfaceCrb?

Cheers,

Gary Lin

> +    IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> +    PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> +  }
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> index 634bbae847..2e54a78cc0 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> @@ -53,5 +53,6 @@
>    PcdLib
>
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress            ## CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType    ## PRODUCES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> index 01f78bf0be..edcdb72a79 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> @@ -32,10 +32,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  TPM2_PTP_INTERFACE_TYPE
>  Tpm2GetPtpInterface (
>    IN VOID *Register
>    );
>
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  );
> +
>  /**
>    Dump PTP register information.
>
>    @param[in] Register                Pointer to PTP register.
>  **/
> @@ -95,10 +107,11 @@ Tpm2InstanceLibDTpmConstructor (
>    VOID
>    )
>  {
>    EFI_STATUS               Status;
>    TPM2_PTP_INTERFACE_TYPE  PtpInterface;
> +  UINT8                    IdleByPass;
>
>    Status = Tpm2RegisterTpm2DeviceLib (&mDTpm2InternalTpm2Device);
>    if ((Status == EFI_SUCCESS) || (Status == EFI_UNSUPPORTED)) {
>      //
>      // Unsupported means platform policy does not need this instance enabled.
> @@ -109,10 +122,16 @@ Tpm2InstanceLibDTpmConstructor (
>        //
>        if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
>          PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>          PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
>        }
> +
> +      if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
> +        IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> +        PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> +      }
> +
>        DumpPtpInfo ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
>      }
>      return EFI_SUCCESS;
>    }
>    return Status;
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> index 876a5a63c4..24e4c35d55 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> @@ -48,6 +48,7 @@
>    DebugLib
>    PcdLib
>
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress          ## CONSUMES
> -  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
> \ No newline at end of file
> +  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType  ## PRODUCES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass             ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1bc153a2c0..5bce9f8e02 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -172,14 +172,34 @@ PtpCrbTpmCommand (
>          DEBUG ((EFI_D_VERBOSE, "%02x ", BufferIn[Index]));
>        }
>      }
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
> -  TpmOutSize = 0;
> +  TpmOutSize         = 0;
>
>    //
>    // STEP 0:
> +  // if CapCRbIdelByPass == 0, enforce Idle state before sending command
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 0 && (MmioRead32((UINTN)&CrbReg->CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0){
> +    Status = PtpCrbWaitRegisterBits (
> +              &CrbReg->CrbControlStatus,
> +              PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> +              0,
> +              PTP_TIMEOUT_C
> +              );
> +    if (EFI_ERROR (Status)) {
> +      //
> +      // Try to goIdle to recover TPM
> +      //
> +      Status = EFI_DEVICE_ERROR;
> +      goto GoIdle_Exit;
> +    }
> +  }
> +
> +  //
> +  // STEP 1:
>    // Ready is any time the TPM is ready to receive a command, following a write
>    // of 1 by software to Request.cmdReady, as indicated by the Status field
>    // being cleared to 0.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> @@ -189,25 +209,25 @@ PtpCrbTpmCommand (
>               PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
>               PTP_TIMEOUT_C
>               );
>    if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>    Status = PtpCrbWaitRegisterBits (
>               &CrbReg->CrbControlStatus,
>               0,
>               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>               PTP_TIMEOUT_C
>               );
>    if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>
>    //
> -  // STEP 1:
> +  // STEP 2:
>    // Command Reception occurs following a Ready state between the write of the
>    // first byte of a command to the Command Buffer and the receipt of a write
>    // of 1 to Start.
>    //
>    for (Index = 0; Index < SizeIn; Index++) {
> @@ -219,11 +239,11 @@ PtpCrbTpmCommand (
>
>    MmioWrite64 ((UINTN)&CrbReg->CrbControlResponseAddrss, (UINT32)(UINTN)CrbReg->CrbDataBuffer);
>    MmioWrite32 ((UINTN)&CrbReg->CrbControlResponseSize, sizeof(CrbReg->CrbDataBuffer));
>
>    //
> -  // STEP 2:
> +  // STEP 3:
>    // Command Execution occurs after receipt of a 1 to Start and the TPM
>    // clearing Start to 0.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlStart, PTP_CRB_CONTROL_START);
>    Status = PtpCrbWaitRegisterBits (
> @@ -249,16 +269,16 @@ PtpCrbTpmCommand (
>      if (EFI_ERROR(Status)) {
>        //
>        // Still in Command Execution state. Try to goIdle, the behavior is agnostic.
>        //
>        Status = EFI_DEVICE_ERROR;
> -      goto Exit;
> +      goto GoIdle_Exit;
>      }
>    }
>
>    //
> -  // STEP 3:
> +  // STEP 4:
>    // Command Completion occurs after completion of a command (indicated by the
>    // TPM clearing TPM_CRB_CTRL_Start_x to 0) and before a write of a 1 by the
>    // software to Request.goIdle.
>    //
>
> @@ -281,40 +301,72 @@ PtpCrbTpmCommand (
>    CopyMem (&Data16, BufferOut, sizeof (UINT16));
>    // TPM2 should not use this RSP_COMMAND
>    if (SwapBytes16 (Data16) == TPM_ST_RSP_COMMAND) {
>      DEBUG ((EFI_D_ERROR, "TPM2: TPM_ST_RSP error - %x\n", TPM_ST_RSP_COMMAND));
>      Status = EFI_UNSUPPORTED;
> -    goto Exit;
> +    goto GoIdle_Exit;
>    }
>
>    CopyMem (&Data32, (BufferOut + 2), sizeof (UINT32));
>    TpmOutSize  = SwapBytes32 (Data32);
>    if (*SizeOut < TpmOutSize) {
> +    //
> +    // Command completed, but buffer is not enough
> +    //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto Exit;
> +    goto GoReady_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
>    // Continue reading the remaining data
>    //
>    for (Index = sizeof (TPM2_RESPONSE_HEADER); Index < TpmOutSize; Index++) {
>      BufferOut[Index] = MmioRead8 ((UINTN)&CrbReg->CrbDataBuffer[Index]);
>    }
> -Exit:
> +
>    DEBUG_CODE (
>      DEBUG ((EFI_D_VERBOSE, "PtpCrbTpmCommand Receive - "));
>      for (Index = 0; Index < TpmOutSize; Index++) {
>        DEBUG ((EFI_D_VERBOSE, "%02x ", BufferOut[Index]));
>      }
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
>
> +GoReady_Exit:
>    //
> -  // STEP 4:
> -  // Idle is any time TPM_CRB_CTRL_STS_x.Status.goIdle is 1.
> +  // Goto Ready State if command is completed succesfully and TPM support IdleBypass
> +  // If not supported. flow down to GoIdle
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 1) {
> +    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> +    return Status;
> +  }
> +
> +  //
> +  // Do not wait for state transition for TIMEOUT_C
> +  // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> +  //
> +GoIdle_Exit:
> +
> +  //
> +  //  Return to Idle state by setting TPM_CRB_CTRL_STS_x.Status.goIdle to 1.
>    //
>    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> +
> +  //
> +  // Only enforce Idle state transition if execution fails when CRBIndleBypass==1
> +  // Leave regular Idle delay at the beginning of next command execution
> +  //
> +  if (PcdGet8(PcdCRBIdleByPass) == 1){
> +    Status = PtpCrbWaitRegisterBits (
> +               &CrbReg->CrbControlStatus,
> +               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> +               0,
> +               PTP_TIMEOUT_C
> +               );
> +  }
> +
>    return Status;
>  }
>
>  /**
>    Send a command to TPM for execution and return response data.
> @@ -392,10 +444,32 @@ Tpm2GetPtpInterface (
>      return Tpm2PtpInterfaceFifo;
>    }
>    return Tpm2PtpInterfaceTis;
>  }
>
> +/**
> +  Return PTP CRB interface IdleByPass state.
> +
> +  @param[in] Register                Pointer to PTP register.
> +
> +  @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> +  IN VOID *Register
> +  )
> +{
> +  PTP_CRB_INTERFACE_IDENTIFIER  InterfaceId;
> +
> +  //
> +  // Check interface id
> +  //
> +  InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
> +
> +  return (UINT8)(InterfaceId.Bits.CapCRBIdleBypass);
> +}
> +
>  /**
>    Dump PTP register information.
>
>    @param[in] Register                Pointer to PTP register.
>  **/
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 60f1c0a0e3..e24b563bdb 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -472,7 +472,17 @@
>    #  0xFF - Contains 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.
> +  #  Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and
> +  #  diretcly move to CmdReady state. <BR>
> +  #  0x00 - Do not support IdleByPass.<BR>
> +  #  0x01 - Support IdleByPass.<BR>
> +  #  0xFF - IdleByPass State is not synced with TPM hardware.<BR>
> +  #
> +  # @Prompt IdleByass status supported by current active TPM interface.
> +  gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass|0xFF|UINT8|0x0001001F
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    SecurityPkgExtra.uni
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index c34250e423..000bc83d80 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -252,6 +252,14 @@
>
>  #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>"
> \ No newline at end of file
> +                                                                                          "0xFF - Contains 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>"
> +                                                                                          "0x01 - Do not support IdleByPass.<BR>\n"
> +                                                                                          "0x02 - Support IdleByPass.<BR>\n"
> +                                                                                          "0xFF - IdleByPass State is not synced with TPM hardware.<BR>"
> \ No newline at end of file
> --
> 2.16.2.windows.1
>
> _______________________________________________
> 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] 6+ messages in thread

end of thread, other threads:[~2018-07-03 23:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-25  4:44 [Patch 0/2] Add CRB IdleByPass Support Zhang, Chao B
2018-06-25  4:44 ` [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register Zhang, Chao B
2018-06-25  4:44 ` [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support Zhang, Chao B
2018-06-27  8:18   ` Gary Lin
2018-07-03 23:25     ` Zhang, Chao B
2018-06-27  0:55 ` [Patch 0/2] Add CRB IdleByPass Support Long, Qin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox