Date
1 - 5 of 5
[PATCH] SecurityPkg: Add retry mechanism for tpm command
Michael D Kinney
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
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-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Wednesday, July 27, 2022 5:41 PM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: Wang, Jian J <jian.j.wang@...>
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@...>
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-----
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@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>
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@...>-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Wednesday, July 27, 2022 7:36 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Yao, Jiewen
<jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>
Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command
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 //
#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
Qi Zhang
Retry count is suggested in the spec.
PtpCrbWaitRegisterBits() already has delay.
Thanks!
Qi Zhang
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
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-----
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@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>
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@...>-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Wednesday, July 27, 2022 7:36 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Yao, Jiewen
<jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>
Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command
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 //
#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
Michael D Kinney
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
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-----
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@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>
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@...>-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Wednesday, July 27, 2022 7:36 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Yao, Jiewen <jiewen.yao@...>;
Wang, Jian J <jian.j.wang@...>
Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command
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
//
#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
Yao, Jiewen
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@...>
toggle quoted message
Show quoted text
For the code, reviewed-by: Jiewen Yao <Jiewen.yao@...>
-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Wednesday, July 27, 2022 7:36 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Yao, Jiewen <jiewen.yao@...>;
Wang, Jian J <jian.j.wang@...>
Subject: [PATCH] SecurityPkg: Add retry mechanism for tpm command
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
//
#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
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-=
+ &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
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