Reviewed-by: Jiewen Yao <jiewen....@intel.com> > -----Original Message----- > From: Zhang, Qi1 <qi1.zh...@intel.com> > Sent: Friday, July 29, 2022 10:26 AM > To: devel@edk2.groups.io > Cc: Zhang, Qi1 <qi1.zh...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; > Wang, Jian J <jian.j.w...@intel.com>; Patil, Swapnil > <s.keshavrao.pa...@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.zh...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Reviewed-by: Jiewen Yao <jiewen....@intel.com> > Tested-by: Swapnil Patil <s.keshavrao.pa...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
