.../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 107 +++++++++++------- 1 file changed, 68 insertions(+), 39 deletions(-)
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>
---
.../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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91889): https://edk2.groups.io/g/devel/message/91889
Mute This Topic: https://groups.io/mt/92647147/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thanks. Please add Bugzilla ID and add tested-by tag by the people who performed the test. For the code, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> > -----Original Message----- > From: Zhang, Qi1 <qi1.zhang@intel.com> > Sent: Wednesday, July 27, 2022 7:36 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> > Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command > > 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> > --- > .../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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91890): https://edk2.groups.io/g/devel/message/91890 Mute This Topic: https://groups.io/mt/92647147/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen > Sent: Wednesday, July 27, 2022 5:07 AM > To: Zhang, Qi1 <qi1.zhang@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com> > Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Add retry mechanism for tpm command > > Thanks. Please add Bugzilla ID and add tested-by tag by the people who performed the test. > > For the code, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> > > > -----Original Message----- > > From: Zhang, Qi1 <qi1.zhang@intel.com> > > Sent: Wednesday, July 27, 2022 7:36 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> > > Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command > > > > 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> > > --- > > .../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 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91900): https://edk2.groups.io/g/devel/message/91900 Mute This Topic: https://groups.io/mt/92647147/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Retry count is suggested in the spec. PtpCrbWaitRegisterBits() already has delay. Thanks! Qi Zhang -----Original Message----- From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Thursday, July 28, 2022 12:38 AM To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> Cc: Wang, Jian J <jian.j.wang@intel.com> 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----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Wednesday, July 27, 2022 5:07 AM > To: Zhang, Qi1 <qi1.zhang@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com> > Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Add retry mechanism for > tpm command > > Thanks. Please add Bugzilla ID and add tested-by tag by the people who performed the test. > > For the code, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> > > > -----Original Message----- > > From: Zhang, Qi1 <qi1.zhang@intel.com> > > Sent: Wednesday, July 27, 2022 7:36 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> > > Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command > > > > 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> > > --- > > .../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 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91909): https://edk2.groups.io/g/devel/message/91909 Mute This Topic: https://groups.io/mt/92647147/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
The commit message and comments for the #defines for the retry count and timeouts should state that the values are required by spec and name the spec. Thanks, Mike > -----Original Message----- > From: Zhang, Qi1 <qi1.zhang@intel.com> > Sent: Wednesday, July 27, 2022 5:41 PM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com> > Cc: Wang, Jian J <jian.j.wang@intel.com> > Subject: RE: [edk2-devel] [PATCH] SecurityPkg: Add retry mechanism for tpm command > > Retry count is suggested in the spec. > > PtpCrbWaitRegisterBits() already has delay. > > Thanks! > Qi Zhang > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, July 28, 2022 12:38 AM > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > Cc: Wang, Jian J <jian.j.wang@intel.com> > 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----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > > Jiewen > > Sent: Wednesday, July 27, 2022 5:07 AM > > To: Zhang, Qi1 <qi1.zhang@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.wang@intel.com> > > Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Add retry mechanism for > > tpm command > > > > Thanks. Please add Bugzilla ID and add tested-by tag by the people who performed the test. > > > > For the code, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> > > > > > -----Original Message----- > > > From: Zhang, Qi1 <qi1.zhang@intel.com> > > > Sent: Wednesday, July 27, 2022 7:36 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> > > > Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command > > > > > > 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> > > > --- > > > .../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 > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91912): https://edk2.groups.io/g/devel/message/91912 Mute This Topic: https://groups.io/mt/92647147/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.