public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
@ 2021-07-16 21:18 Rodrigo Gonzalez del Cueto
  2021-07-30 17:18 ` [edk2-devel] " Rodrigo Gonzalez del Cueto
  2021-08-09  1:27 ` Yao, Jiewen
  0 siblings, 2 replies; 8+ messages in thread
From: Rodrigo Gonzalez del Cueto @ 2021-07-16 21:18 UTC (permalink / raw)
  To: devel; +Cc: Rodrigo Gonzalez del Cueto, Jian J Wang, Jiewen Yao

To follow the TCG CRB protocol specification, on every CRB TPM command
completion the TPM should return to Idle state, regardless of the
CRB Idle Bypass capability reported by the TPM device.

See: TCG PC Client Device Driver Design Principles for TPM 2.0,
Version 1.0, Rev 0.27

Signed-off-by: Rodrigo Gonzalez del Cueto <rodrigo.gonzalez.del.cueto@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
---
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index f1f8091683..34e3874a5b 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -310,7 +310,7 @@ PtpCrbTpmCommand (
     // Command completed, but buffer is not enough
     //
     Status = EFI_BUFFER_TOO_SMALL;
-    goto GoReady_Exit;
+    goto GoIdle_Exit;
   }
   *SizeOut = TpmOutSize;
   //
@@ -328,16 +328,6 @@ PtpCrbTpmCommand (
     DEBUG ((EFI_D_VERBOSE, "\n"));
   );
 
-GoReady_Exit:
-  //
-  // Goto Ready State if command is completed successfully and TPM support IdleBypass
-  // If not supported. flow down to GoIdle
-  //
-  if (GetCachedIdleByPass () == 1) {
-    MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
-    return Status;
-  }
-
   //
   // Do not wait for state transition for TIMEOUT_C
   // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
-- 
2.31.1.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
  2021-07-16 21:18 [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion Rodrigo Gonzalez del Cueto
@ 2021-07-30 17:18 ` Rodrigo Gonzalez del Cueto
  2021-08-09  1:27 ` Yao, Jiewen
  1 sibling, 0 replies; 8+ messages in thread
From: Rodrigo Gonzalez del Cueto @ 2021-07-30 17:18 UTC (permalink / raw)
  To: Rodrigo Gonzalez del Cueto, devel

[-- Attachment #1: Type: text/plain, Size: 112 bytes --]

Missed adding the Bugzilla reference to the patch.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3463

[-- Attachment #2: Type: text/html, Size: 120 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
  2021-07-16 21:18 [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion Rodrigo Gonzalez del Cueto
  2021-07-30 17:18 ` [edk2-devel] " Rodrigo Gonzalez del Cueto
@ 2021-08-09  1:27 ` Yao, Jiewen
  2021-10-29 21:33   ` Rodrigo Gonzalez del Cueto
  1 sibling, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2021-08-09  1:27 UTC (permalink / raw)
  To: Gonzalez Del Cueto, Rodrigo, devel@edk2.groups.io; +Cc: Wang, Jian J

Would you please tell us how many TPM2 chip you have tested?

I think we need consider the compatibility of exiting TPM2 chips, to make sure the code still work.


Thank you
Yao Jiewen

> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>
> Sent: Saturday, July 17, 2021 5:18 AM
> To: devel@edk2.groups.io
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command
> completion.
> 
> To follow the TCG CRB protocol specification, on every CRB TPM command
> completion the TPM should return to Idle state, regardless of the
> CRB Idle Bypass capability reported by the TPM device.
> 
> See: TCG PC Client Device Driver Design Principles for TPM 2.0,
> Version 1.0, Rev 0.27
> 
> Signed-off-by: Rodrigo Gonzalez del Cueto
> <rodrigo.gonzalez.del.cueto@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index f1f8091683..34e3874a5b 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -310,7 +310,7 @@ PtpCrbTpmCommand (
>      // Command completed, but buffer is not enough
>      //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto GoReady_Exit;
> +    goto GoIdle_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
> @@ -328,16 +328,6 @@ PtpCrbTpmCommand (
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
> 
> -GoReady_Exit:
> -  //
> -  // Goto Ready State if command is completed successfully and TPM support
> IdleBypass
> -  // If not supported. flow down to GoIdle
> -  //
> -  if (GetCachedIdleByPass () == 1) {
> -    MmioWrite32((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> -    return Status;
> -  }
> -
>    //
>    // Do not wait for state transition for TIMEOUT_C
>    // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> --
> 2.31.1.windows.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
  2021-08-09  1:27 ` Yao, Jiewen
@ 2021-10-29 21:33   ` Rodrigo Gonzalez del Cueto
  2021-11-04 13:40     ` Yao, Jiewen
       [not found]     ` <16B45B96D9D719E5.17679@groups.io>
  0 siblings, 2 replies; 8+ messages in thread
From: Rodrigo Gonzalez del Cueto @ 2021-10-29 21:33 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wang, Jian J

[-- Attachment #1: Type: text/plain, Size: 3192 bytes --]

Hi Jiewen,

I have tested the proposed CRB protocol fix with three different TPM configurations I have available which support the CRB interface: Intel PTT, STMicro and Nuvoton. Under these CRB configurations I didn't observe any issues arising from the proposed change aligning with the TCG CRB protocol definition.

I verified the BIOS flows were unaffected and completed without errors and that the OS was still able to interact with the TPM.

Regards,
-Rodrigo
________________________________
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Sunday, August 8, 2021 6:27 PM
To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: RE: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Would you please tell us how many TPM2 chip you have tested?

I think we need consider the compatibility of exiting TPM2 chips, to make sure the code still work.


Thank you
Yao Jiewen

> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>
> Sent: Saturday, July 17, 2021 5:18 AM
> To: devel@edk2.groups.io
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command
> completion.
>
> To follow the TCG CRB protocol specification, on every CRB TPM command
> completion the TPM should return to Idle state, regardless of the
> CRB Idle Bypass capability reported by the TPM device.
>
> See: TCG PC Client Device Driver Design Principles for TPM 2.0,
> Version 1.0, Rev 0.27
>
> Signed-off-by: Rodrigo Gonzalez del Cueto
> <rodrigo.gonzalez.del.cueto@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index f1f8091683..34e3874a5b 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -310,7 +310,7 @@ PtpCrbTpmCommand (
>      // Command completed, but buffer is not enough
>      //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto GoReady_Exit;
> +    goto GoIdle_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
> @@ -328,16 +328,6 @@ PtpCrbTpmCommand (
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
>
> -GoReady_Exit:
> -  //
> -  // Goto Ready State if command is completed successfully and TPM support
> IdleBypass
> -  // If not supported. flow down to GoIdle
> -  //
> -  if (GetCachedIdleByPass () == 1) {
> -    MmioWrite32((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> -    return Status;
> -  }
> -
>    //
>    // Do not wait for state transition for TIMEOUT_C
>    // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> --
> 2.31.1.windows.1


[-- Attachment #2: Type: text/html, Size: 6056 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
  2021-10-29 21:33   ` Rodrigo Gonzalez del Cueto
@ 2021-11-04 13:40     ` Yao, Jiewen
       [not found]     ` <16B45B96D9D719E5.17679@groups.io>
  1 sibling, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2021-11-04 13:40 UTC (permalink / raw)
  To: Gonzalez Del Cueto, Rodrigo, devel@edk2.groups.io; +Cc: Wang, Jian J

[-- Attachment #1: Type: text/plain, Size: 3994 bytes --]

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>
Sent: Saturday, October 30, 2021 5:34 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Hi Jiewen,

I have tested the proposed CRB protocol fix with three different TPM configurations I have available which support the CRB interface: Intel PTT, STMicro and Nuvoton. Under these CRB configurations I didn't observe any issues arising from the proposed change aligning with the TCG CRB protocol definition.

I verified the BIOS flows were unaffected and completed without errors and that the OS was still able to interact with the TPM.

Regards,
-Rodrigo
________________________________
From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Sunday, August 8, 2021 6:27 PM
To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
Subject: RE: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Would you please tell us how many TPM2 chip you have tested?

I think we need consider the compatibility of exiting TPM2 chips, to make sure the code still work.


Thank you
Yao Jiewen

> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>
> Sent: Saturday, July 17, 2021 5:18 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>;
> Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command
> completion.
>
> To follow the TCG CRB protocol specification, on every CRB TPM command
> completion the TPM should return to Idle state, regardless of the
> CRB Idle Bypass capability reported by the TPM device.
>
> See: TCG PC Client Device Driver Design Principles for TPM 2.0,
> Version 1.0, Rev 0.27
>
> Signed-off-by: Rodrigo Gonzalez del Cueto
> <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index f1f8091683..34e3874a5b 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -310,7 +310,7 @@ PtpCrbTpmCommand (
>      // Command completed, but buffer is not enough
>      //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto GoReady_Exit;
> +    goto GoIdle_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
> @@ -328,16 +328,6 @@ PtpCrbTpmCommand (
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
>
> -GoReady_Exit:
> -  //
> -  // Goto Ready State if command is completed successfully and TPM support
> IdleBypass
> -  // If not supported. flow down to GoIdle
> -  //
> -  if (GetCachedIdleByPass () == 1) {
> -    MmioWrite32((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> -    return Status;
> -  }
> -
>    //
>    // Do not wait for state transition for TIMEOUT_C
>    // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> --
> 2.31.1.windows.1

[-- Attachment #2: Type: text/html, Size: 8709 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.
       [not found]     ` <16B45B96D9D719E5.17679@groups.io>
@ 2021-11-04 14:05       ` Yao, Jiewen
  0 siblings, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2021-11-04 14:05 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, Gonzalez Del Cueto, Rodrigo
  Cc: Wang, Jian J

[-- Attachment #1: Type: text/plain, Size: 4613 bytes --]

CI failed: https://github.com/tianocore/edk2/pull/2173

Would you please try CI by yourself?

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: Thursday, November 4, 2021 9:40 PM
To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com<mailto:Jiewen.yao@intel.com>>

From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>
Sent: Saturday, October 30, 2021 5:34 AM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
Subject: Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Hi Jiewen,

I have tested the proposed CRB protocol fix with three different TPM configurations I have available which support the CRB interface: Intel PTT, STMicro and Nuvoton. Under these CRB configurations I didn't observe any issues arising from the proposed change aligning with the TCG CRB protocol definition.

I verified the BIOS flows were unaffected and completed without errors and that the OS was still able to interact with the TPM.

Regards,
-Rodrigo
________________________________
From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Sunday, August 8, 2021 6:27 PM
To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
Subject: RE: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion.

Would you please tell us how many TPM2 chip you have tested?

I think we need consider the compatibility of exiting TPM2 chips, to make sure the code still work.


Thank you
Yao Jiewen

> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>
> Sent: Saturday, July 17, 2021 5:18 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>;
> Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command
> completion.
>
> To follow the TCG CRB protocol specification, on every CRB TPM command
> completion the TPM should return to Idle state, regardless of the
> CRB Idle Bypass capability reported by the TPM device.
>
> See: TCG PC Client Device Driver Design Principles for TPM 2.0,
> Version 1.0, Rev 0.27
>
> Signed-off-by: Rodrigo Gonzalez del Cueto
> <rodrigo.gonzalez.del.cueto@intel.com<mailto:rodrigo.gonzalez.del.cueto@intel.com>>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index f1f8091683..34e3874a5b 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -310,7 +310,7 @@ PtpCrbTpmCommand (
>      // Command completed, but buffer is not enough
>      //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto GoReady_Exit;
> +    goto GoIdle_Exit;
>    }
>    *SizeOut = TpmOutSize;
>    //
> @@ -328,16 +328,6 @@ PtpCrbTpmCommand (
>      DEBUG ((EFI_D_VERBOSE, "\n"));
>    );
>
> -GoReady_Exit:
> -  //
> -  // Goto Ready State if command is completed successfully and TPM support
> IdleBypass
> -  // If not supported. flow down to GoIdle
> -  //
> -  if (GetCachedIdleByPass () == 1) {
> -    MmioWrite32((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> -    return Status;
> -  }
> -
>    //
>    // Do not wait for state transition for TIMEOUT_C
>    // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> --
> 2.31.1.windows.1


[-- Attachment #2: Type: text/html, Size: 9926 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion
@ 2021-12-17  2:47 Rodrigo Gonzalez del Cueto
  2021-12-17 15:09 ` Yao, Jiewen
  0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Gonzalez del Cueto @ 2021-12-17  2:47 UTC (permalink / raw)
  To: devel; +Cc: Rodrigo Gonzalez del Cueto, Jian J Wang, Jiewen Yao

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3463

In V2: Fixed patch format and uncrustify cleanup

In V1: To follow the TCG CRB protocol specification, on every CRB TPM
 command completion the TPM should return to Idle state, regardless of
the CRB Idle Bypass capability reported by the TPM device.

See: TCG PC Client Device Driver Design Principles for TPM 2.0,
Version 1.0, Rev 0.27

Signed-off-by: Rodrigo Gonzalez del Cueto <rodrigo.gonzalez.del.cueto@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
---
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 40ab998004..1d99beaa10 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -1,7 +1,7 @@
 /** @file
   PTP (Platform TPM Profile) CRB (Command Response Buffer) interface used by dTPM2.0 library.
 
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
 Copyright (c), Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -320,7 +320,7 @@ PtpCrbTpmCommand (
     // Command completed, but buffer is not enough
     //
     Status = EFI_BUFFER_TOO_SMALL;
-    goto GoReady_Exit;
+    goto GoIdle_Exit;
   }
 
   *SizeOut = TpmOutSize;
@@ -340,16 +340,6 @@ PtpCrbTpmCommand (
   DEBUG ((DEBUG_VERBOSE, "\n"));
   DEBUG_CODE_END ();
 
-GoReady_Exit:
-  //
-  // Goto Ready State if command is completed successfully and TPM support IdleBypass
-  // If not supported. flow down to GoIdle
-  //
-  if (GetCachedIdleByPass () == 1) {
-    MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
-    return Status;
-  }
-
   //
   // Do not wait for state transition for TIMEOUT_C
   // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion
  2021-12-17  2:47 Rodrigo Gonzalez del Cueto
@ 2021-12-17 15:09 ` Yao, Jiewen
  0 siblings, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2021-12-17 15:09 UTC (permalink / raw)
  To: Gonzalez Del Cueto, Rodrigo, devel@edk2.groups.io; +Cc: Wang, Jian J

Pushed ab5ab2f60348138a4b7b1c95ad6f5d0954fb96f1

> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>
> Sent: Friday, December 17, 2021 10:48 AM
> To: devel@edk2.groups.io
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] SecurityPkg: TPM must go to Idle state on CRB command
> completion
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3463
> 
> In V2: Fixed patch format and uncrustify cleanup
> 
> In V1: To follow the TCG CRB protocol specification, on every CRB TPM
>  command completion the TPM should return to Idle state, regardless of
> the CRB Idle Bypass capability reported by the TPM device.
> 
> See: TCG PC Client Device Driver Design Principles for TPM 2.0,
> Version 1.0, Rev 0.27
> 
> Signed-off-by: Rodrigo Gonzalez del Cueto
> <rodrigo.gonzalez.del.cueto@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 40ab998004..1d99beaa10 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -1,7 +1,7 @@
>  /** @file
>    PTP (Platform TPM Profile) CRB (Command Response Buffer) interface used by
> dTPM2.0 library.
> 
> -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
>  Copyright (c), Microsoft Corporation.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -320,7 +320,7 @@ PtpCrbTpmCommand (
>      // Command completed, but buffer is not enough
>      //
>      Status = EFI_BUFFER_TOO_SMALL;
> -    goto GoReady_Exit;
> +    goto GoIdle_Exit;
>    }
> 
>    *SizeOut = TpmOutSize;
> @@ -340,16 +340,6 @@ PtpCrbTpmCommand (
>    DEBUG ((DEBUG_VERBOSE, "\n"));
>    DEBUG_CODE_END ();
> 
> -GoReady_Exit:
> -  //
> -  // Goto Ready State if command is completed successfully and TPM support
> IdleBypass
> -  // If not supported. flow down to GoIdle
> -  //
> -  if (GetCachedIdleByPass () == 1) {
> -    MmioWrite32 ((UINTN)&CrbReg->CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
> -    return Status;
> -  }
> -
>    //
>    // Do not wait for state transition for TIMEOUT_C
>    // This function will try to wait 2 TIMEOUT_C at the beginning in next call.
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-12-17 15:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-16 21:18 [PATCH] SecurityPkg: TPM must go to Idle state on CRB command completion Rodrigo Gonzalez del Cueto
2021-07-30 17:18 ` [edk2-devel] " Rodrigo Gonzalez del Cueto
2021-08-09  1:27 ` Yao, Jiewen
2021-10-29 21:33   ` Rodrigo Gonzalez del Cueto
2021-11-04 13:40     ` Yao, Jiewen
     [not found]     ` <16B45B96D9D719E5.17679@groups.io>
2021-11-04 14:05       ` [edk2-devel] " Yao, Jiewen
  -- strict thread matches above, loose matches on Subject: below --
2021-12-17  2:47 Rodrigo Gonzalez del Cueto
2021-12-17 15:09 ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox