1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH v4] SecurityPkg: Add retry mechanism for tpm command

Qi Zhang posted 1 patch 1 year, 9 months ago
Failed in applying to current master (apply log)
.../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c       | 108 +++++++++++-------
1 file changed, 69 insertions(+), 39 deletions(-)
[edk2-devel] [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
Posted by Qi Zhang 1 year, 9 months ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980

As per TCG PC Client Device Driver Design Principle document,
if tpm commands fails due to timeout condition, then it should
have retry mechanism (3 retry attempts).
Existing implementation of PtpCrbTpmCommand does not have retry
mechanism if it fails with EFI_TIMEOUT.

See TCG PC Client Device Driver Design Principles for TPM 2.0
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1_r4_211104_final.pdf
Vision 1.1, Revision 0.04
Section 7.2.1

Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Swapnil Patil <S.Keshavrao.Patil@dell.com>
---
 .../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c       | 108 +++++++++++-------
 1 file changed, 69 insertions(+), 39 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 1d99beaa10..840265292a 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -33,6 +33,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 #define TPMCMDBUFLENGTH  0x500
 
+//
+// Max retry count according to Spec TCG PC Client Device Driver Design Principles
+// for TPM2.0, Version 1.1, Revision 0.04, Section 7.2.1
+//
+#define RETRY_CNT_MAX  3
+
 /**
   Check whether TPM PTP register exist.
 
@@ -153,6 +159,7 @@ PtpCrbTpmCommand (
   UINT32      TpmOutSize;
   UINT16      Data16;
   UINT32      Data32;
+  UINT8       RetryCnt;
 
   DEBUG_CODE_BEGIN ();
   UINTN  DebugSize;
@@ -179,53 +186,76 @@ PtpCrbTpmCommand (
   DEBUG_CODE_END ();
   TpmOutSize = 0;
 
-  //
-  // STEP 0:
-  // if CapCRbIdelByPass == 0, enforce Idle state before sending command
-  //
-  if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)&CrbReg->CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
+  RetryCnt = 0;
+  while (TRUE) {
+    //
+    // STEP 0:
+    // if CapCRbIdelByPass == 0, enforce Idle state before sending command
+    //
+    if ((GetCachedIdleByPass () == 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)) {
+        RetryCnt++;
+        if (RetryCnt < RETRY_CNT_MAX) {
+          MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+          continue;
+        } else {
+          //
+          // 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);
     Status = PtpCrbWaitRegisterBits (
-               &CrbReg->CrbControlStatus,
-               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+               &CrbReg->CrbControlRequest,
                0,
+               PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
                PTP_TIMEOUT_C
                );
     if (EFI_ERROR (Status)) {
-      //
-      // Try to goIdle to recover TPM
-      //
-      Status = EFI_DEVICE_ERROR;
-      goto GoIdle_Exit;
+      RetryCnt++;
+      if (RetryCnt < RETRY_CNT_MAX) {
+        MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+        continue;
+      } else {
+        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);
-  Status = PtpCrbWaitRegisterBits (
-             &CrbReg->CrbControlRequest,
-             0,
-             PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
-             PTP_TIMEOUT_C
-             );
-  if (EFI_ERROR (Status)) {
-    Status = EFI_DEVICE_ERROR;
-    goto GoIdle_Exit;
-  }
+    Status = PtpCrbWaitRegisterBits (
+               &CrbReg->CrbControlStatus,
+               0,
+               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+               PTP_TIMEOUT_C
+               );
+    if (EFI_ERROR (Status)) {
+      RetryCnt++;
+      if (RetryCnt < RETRY_CNT_MAX) {
+        MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+        continue;
+      } else {
+        Status = EFI_DEVICE_ERROR;
+        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 GoIdle_Exit;
+    break;
   }
 
   //
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91955): https://edk2.groups.io/g/devel/message/91955
Mute This Topic: https://groups.io/mt/92683623/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
Posted by Yao, Jiewen 1 year, 9 months ago 
Reviewed-by: Jiewen Yao <Jiewen.yao@Intel.com>

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Friday, July 29, 2022 10:26 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Patil, Swapnil
> <s.keshavrao.patil@dell.com>
> Subject: [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980
> 
> As per TCG PC Client Device Driver Design Principle document,
> if tpm commands fails due to timeout condition, then it should
> have retry mechanism (3 retry attempts).
> Existing implementation of PtpCrbTpmCommand does not have retry
> mechanism if it fails with EFI_TIMEOUT.
> 
> See TCG PC Client Device Driver Design Principles for TPM 2.0
> https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1
> _r4_211104_final.pdf
> Vision 1.1, Revision 0.04
> Section 7.2.1
> 
> Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> Tested-by: Swapnil Patil <S.Keshavrao.Patil@dell.com>
> ---
>  .../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c       | 108 +++++++++++-------
>  1 file changed, 69 insertions(+), 39 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1d99beaa10..840265292a 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -33,6 +33,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
> 
>  #define TPMCMDBUFLENGTH  0x500
> 
> 
> 
> +//
> 
> +// Max retry count according to Spec TCG PC Client Device Driver Design
> Principles
> 
> +// for TPM2.0, Version 1.1, Revision 0.04, Section 7.2.1
> 
> +//
> 
> +#define RETRY_CNT_MAX  3
> 
> +
> 
>  /**
> 
>    Check whether TPM PTP register exist.
> 
> 
> 
> @@ -153,6 +159,7 @@ PtpCrbTpmCommand (
>    UINT32      TpmOutSize;
> 
>    UINT16      Data16;
> 
>    UINT32      Data32;
> 
> +  UINT8       RetryCnt;
> 
> 
> 
>    DEBUG_CODE_BEGIN ();
> 
>    UINTN  DebugSize;
> 
> @@ -179,53 +186,76 @@ PtpCrbTpmCommand (
>    DEBUG_CODE_END ();
> 
>    TpmOutSize = 0;
> 
> 
> 
> -  //
> 
> -  // STEP 0:
> 
> -  // if CapCRbIdelByPass == 0, enforce Idle state before sending command
> 
> -  //
> 
> -  if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)&CrbReg-
> >CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
> 
> +  RetryCnt = 0;
> 
> +  while (TRUE) {
> 
> +    //
> 
> +    // STEP 0:
> 
> +    // if CapCRbIdelByPass == 0, enforce Idle state before sending command
> 
> +    //
> 
> +    if ((GetCachedIdleByPass () == 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)) {
> 
> +        RetryCnt++;
> 
> +        if (RetryCnt < RETRY_CNT_MAX) {
> 
> +          MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> 
> +          continue;
> 
> +        } else {
> 
> +          //
> 
> +          // 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);
> 
>      Status = PtpCrbWaitRegisterBits (
> 
> -               &CrbReg->CrbControlStatus,
> 
> -               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> 
> +               &CrbReg->CrbControlRequest,
> 
>                 0,
> 
> +               PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
> 
>                 PTP_TIMEOUT_C
> 
>                 );
> 
>      if (EFI_ERROR (Status)) {
> 
> -      //
> 
> -      // Try to goIdle to recover TPM
> 
> -      //
> 
> -      Status = EFI_DEVICE_ERROR;
> 
> -      goto GoIdle_Exit;
> 
> +      RetryCnt++;
> 
> +      if (RetryCnt < RETRY_CNT_MAX) {
> 
> +        MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> 
> +        continue;
> 
> +      } else {
> 
> +        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);
> 
> -  Status = PtpCrbWaitRegisterBits (
> 
> -             &CrbReg->CrbControlRequest,
> 
> -             0,
> 
> -             PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
> 
> -             PTP_TIMEOUT_C
> 
> -             );
> 
> -  if (EFI_ERROR (Status)) {
> 
> -    Status = EFI_DEVICE_ERROR;
> 
> -    goto GoIdle_Exit;
> 
> -  }
> 
> +    Status = PtpCrbWaitRegisterBits (
> 
> +               &CrbReg->CrbControlStatus,
> 
> +               0,
> 
> +               PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> 
> +               PTP_TIMEOUT_C
> 
> +               );
> 
> +    if (EFI_ERROR (Status)) {
> 
> +      RetryCnt++;
> 
> +      if (RetryCnt < RETRY_CNT_MAX) {
> 
> +        MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> 
> +        continue;
> 
> +      } else {
> 
> +        Status = EFI_DEVICE_ERROR;
> 
> +        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 GoIdle_Exit;
> 
> +    break;
> 
>    }
> 
> 
> 
>    //
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91964): https://edk2.groups.io/g/devel/message/91964
Mute This Topic: https://groups.io/mt/92683623/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.