* [PATCH v3] SecurityPkg: Add retry mechanism for tpm command
@ 2022-07-28 8:51 Qi Zhang
2022-07-28 12:50 ` Yao, Jiewen
0 siblings, 1 reply; 2+ messages in thread
From: Qi Zhang @ 2022-07-28 8:51 UTC (permalink / raw)
To: devel; +Cc: Qi Zhang, Jiewen Yao, Jian J Wang, Swapnil Patil
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 | 107 +++++++++++-------
1 file changed, 68 insertions(+), 39 deletions(-)
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 1d99beaa10..6b5994fde2 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -33,6 +33,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
//
#define TPMCMDBUFLENGTH 0x500
+//
+// Max retry count
+//
+#define RETRY_CNT_MAX 3
+
/**
Check whether TPM PTP register exist.
@@ -153,6 +158,7 @@ PtpCrbTpmCommand (
UINT32 TpmOutSize;
UINT16 Data16;
UINT32 Data32;
+ UINT8 RetryCnt;
DEBUG_CODE_BEGIN ();
UINTN DebugSize;
@@ -179,53 +185,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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] SecurityPkg: Add retry mechanism for tpm command
2022-07-28 8:51 [PATCH v3] SecurityPkg: Add retry mechanism for tpm command Qi Zhang
@ 2022-07-28 12:50 ` Yao, Jiewen
0 siblings, 0 replies; 2+ messages in thread
From: Yao, Jiewen @ 2022-07-28 12:50 UTC (permalink / raw)
To: Zhang, Qi1, devel@edk2.groups.io; +Cc: Wang, Jian J, Patil, Swapnil
Hey
Please add the spec related info to the code as comments.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, July 28, 2022 4:51 PM
> 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 v3] 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 | 107 +++++++++++-------
> 1 file changed, 68 insertions(+), 39 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1d99beaa10..6b5994fde2 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -33,6 +33,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> //
>
> #define TPMCMDBUFLENGTH 0x500
>
>
>
> +//
>
> +// Max retry count
>
> +//
>
> +#define RETRY_CNT_MAX 3
>
> +
>
> /**
>
> Check whether TPM PTP register exist.
>
>
>
> @@ -153,6 +158,7 @@ PtpCrbTpmCommand (
> UINT32 TpmOutSize;
>
> UINT16 Data16;
>
> UINT32 Data32;
>
> + UINT8 RetryCnt;
>
>
>
> DEBUG_CODE_BEGIN ();
>
> UINTN DebugSize;
>
> @@ -179,53 +185,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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-28 12:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-28 8:51 [PATCH v3] SecurityPkg: Add retry mechanism for tpm command Qi Zhang
2022-07-28 12:50 ` Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox