Date
1 - 5 of 5
[PATCH] SecurityPkg: Add retry mechanism for tpm command
|
Qi Zhang
Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> --- .../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 //=0D #define TPMCMDBUFLENGTH 0x500=0D =0D +//=0D +// Max retry count=0D +//=0D +#define RETRY_CNT_MAX 3=0D +=0D /**=0D Check whether TPM PTP register exist.=0D =0D @@ -153,6 +158,7 @@ PtpCrbTpmCommand ( UINT32 TpmOutSize;=0D UINT16 Data16;=0D UINT32 Data32;=0D + UINT8 RetryCnt;=0D =0D DEBUG_CODE_BEGIN ();=0D UINTN DebugSize;=0D @@ -179,53 +185,76 @@ PtpCrbTpmCommand ( DEBUG_CODE_END ();=0D TpmOutSize =3D 0;=0D =0D - //=0D - // STEP 0:=0D - // if CapCRbIdelByPass =3D=3D 0, enforce Idle state before sending comma= nd=0D - //=0D - if ((GetCachedIdleByPass () =3D=3D 0) && ((MmioRead32 ((UINTN)&CrbReg->C= rbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) =3D=3D 0)) {=0D + RetryCnt =3D 0;=0D + while (TRUE) {=0D + //=0D + // STEP 0:=0D + // if CapCRbIdelByPass =3D=3D 0, enforce Idle state before sending com= mand=0D + //=0D + if ((GetCachedIdleByPass () =3D=3D 0) && ((MmioRead32 ((UINTN)&CrbReg-= CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) =3D=3D 0)) {=0D+ Status =3D PtpCrbWaitRegisterBits (=0D + &CrbReg->CrbControlStatus,=0D + PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,=0D + 0,=0D + PTP_TIMEOUT_C=0D + );=0D + if (EFI_ERROR (Status)) {=0D + RetryCnt++;=0D + if (RetryCnt < RETRY_CNT_MAX) {=0D + MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_= AREA_REQUEST_GO_IDLE);=0D + continue;=0D + } else {=0D + //=0D + // Try to goIdle to recover TPM=0D + //=0D + Status =3D EFI_DEVICE_ERROR;=0D + goto GoIdle_Exit;=0D + }=0D + }=0D + }=0D +=0D + //=0D + // STEP 1:=0D + // Ready is any time the TPM is ready to receive a command, following = a write=0D + // of 1 by software to Request.cmdReady, as indicated by the Status fi= eld=0D + // being cleared to 0.=0D + //=0D + MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_R= EQUEST_COMMAND_READY);=0D Status =3D PtpCrbWaitRegisterBits (=0D - &CrbReg->CrbControlStatus,=0D - PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,=0D + &CrbReg->CrbControlRequest,=0D 0,=0D + PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,=0D PTP_TIMEOUT_C=0D );=0D if (EFI_ERROR (Status)) {=0D - //=0D - // Try to goIdle to recover TPM=0D - //=0D - Status =3D EFI_DEVICE_ERROR;=0D - goto GoIdle_Exit;=0D + RetryCnt++;=0D + if (RetryCnt < RETRY_CNT_MAX) {=0D + MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AR= EA_REQUEST_GO_IDLE);=0D + continue;=0D + } else {=0D + Status =3D EFI_DEVICE_ERROR;=0D + goto GoIdle_Exit;=0D + }=0D }=0D - }=0D =0D - //=0D - // STEP 1:=0D - // Ready is any time the TPM is ready to receive a command, following a = write=0D - // of 1 by software to Request.cmdReady, as indicated by the Status fiel= d=0D - // being cleared to 0.=0D - //=0D - MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQ= UEST_COMMAND_READY);=0D - Status =3D PtpCrbWaitRegisterBits (=0D - &CrbReg->CrbControlRequest,=0D - 0,=0D - PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,=0D - PTP_TIMEOUT_C=0D - );=0D - if (EFI_ERROR (Status)) {=0D - Status =3D EFI_DEVICE_ERROR;=0D - goto GoIdle_Exit;=0D - }=0D + Status =3D PtpCrbWaitRegisterBits (=0D + &CrbReg->CrbControlStatus,=0D + 0,=0D + PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,=0D + PTP_TIMEOUT_C=0D + );=0D + if (EFI_ERROR (Status)) {=0D + RetryCnt++;=0D + if (RetryCnt < RETRY_CNT_MAX) {=0D + MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AR= EA_REQUEST_GO_IDLE);=0D + continue;=0D + } else {=0D + Status =3D EFI_DEVICE_ERROR;=0D + goto GoIdle_Exit;=0D + }=0D + }=0D =0D - Status =3D PtpCrbWaitRegisterBits (=0D - &CrbReg->CrbControlStatus,=0D - 0,=0D - PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,=0D - PTP_TIMEOUT_C=0D - );=0D - if (EFI_ERROR (Status)) {=0D - Status =3D EFI_DEVICE_ERROR;=0D - goto GoIdle_Exit;=0D + break;=0D }=0D =0D //=0D --=20 2.26.2.windows.1 |
|
|
|
Yao, Jiewen
Thanks. Please add Bugzilla ID and add tested-by tag by the people who performed the test.
toggle quoted message
Show quoted text
For the code, reviewed-by: Jiewen Yao <Jiewen.yao@...> -----Original Message----- |
|
|
|
Michael D Kinney
Why is 3 the correct retry count?
toggle quoted message
Show quoted text
Do we need this to be configurable? Is a delay required between retries? What specific state is a DTPM getting into that requires this retry mechanism? Can that state be detected? Thanks, Mike -----Original Message----- |
|
|
|
Qi Zhang
Retry count is suggested in the spec.
toggle quoted message
Show quoted text
PtpCrbWaitRegisterBits() already has delay. Thanks! Qi Zhang -----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...> Sent: Thursday, July 28, 2022 12:38 AM To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>; Zhang, Qi1 <qi1.zhang@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Wang, Jian J <jian.j.wang@...> Subject: RE: [edk2-devel] [PATCH] SecurityPkg: Add retry mechanism for tpm command Why is 3 the correct retry count? Do we need this to be configurable? Is a delay required between retries? What specific state is a DTPM getting into that requires this retry mechanism? Can that state be detected? Thanks, Mike -----Original Message----- |
|
|
|
Michael D Kinney
The commit message and comments for the #defines
toggle quoted message
Show quoted text
for the retry count and timeouts should state that the values are required by spec and name the spec. Thanks, Mike -----Original Message----- |
|
|