From: "Zhang, Chao B" <chao.b.zhang@intel.com>
To: Gary Lin <glin@suse.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Yao, Jiewen" <jiewen.yao@intel.com>,
"Long, Qin" <qin.long@intel.com>
Subject: Re: [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
Date: Tue, 3 Jul 2018 23:25:27 +0000 [thread overview]
Message-ID: <FF72C7E4248F3C4E9BDF19D4918E90F2497FE67A@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20180627081830.gvqh75toe74btbre@GaryWorkstation>
Hi Gary:
It is caused by code merge. Tks for notification. We have fixed it.
From: Gary Lin [mailto:glin@suse.com]
Sent: Wednesday, June 27, 2018 4:19 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Long, Qin <qin.long@intel.com>
Subject: Re: [edk2] [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support
On Mon, Jun 25, 2018 at 12:44:21PM +0800, Zhang, Chao B wrote:
> Directly transition from CMD completion to CMD Ready state if device
> supports IdleByPass
>
> Cc: Long Qin <qin.long@intel.com<mailto:qin.long@intel.com>>
> Cc: Yao Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Signed-off-by: Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> ---
> .../Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c | 19 +++++
> .../Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf | 1 +
> .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c | 19 +++++
> .../Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf | 3 +-
> SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 98 +++++++++++++++++++---
> SecurityPkg/SecurityPkg.dec | 10 +++
> SecurityPkg/SecurityPkg.uni | 10 ++-
> 7 files changed, 146 insertions(+), 14 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> index 3feb64df7e..e6fe563b40 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
> @@ -29,10 +29,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> TPM2_PTP_INTERFACE_TYPE
> Tpm2GetPtpInterface (
> IN VOID *Register
> );
>
> +/**
> + Return PTP CRB interface IdleByPass state.
> +
> + @param[in] Register Pointer to PTP register.
> +
> + @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> + IN VOID *Register
> + );
> +
> /**
> This service enables the sending of commands to the TPM2.
>
> @param[in] InputParameterBlockSize Size of the TPM2 input parameter block.
> @param[in] InputParameterBlock Pointer to the TPM2 input parameter block.
> @@ -138,15 +150,22 @@ EFIAPI
> Tpm2DeviceLibConstructor (
> VOID
> )
> {
> TPM2_PTP_INTERFACE_TYPE PtpInterface;
> + UINT8 IdleByPass;
>
> //
> // Cache current active TpmInterfaceType only when needed
> //
> if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
> PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
> }
> +
> + if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
I got a build error with PtpInterfaceCrb:
SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c: In function ‘Tpm2DeviceLibConstructor’:
SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c:165:45: error: ‘PtpInterfaceCrb’ undeclared (first use in this function); did you mean ‘PtpInterface’?
if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
^~~~~~~~~~~~~~~
PtpInterface
I assume you mean Tpm2PtpInterfaceCrb?
Cheers,
Gary Lin
> + IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> + PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> + }
> +
> return EFI_SUCCESS;
> }
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> index 634bbae847..2e54a78cc0 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> @@ -53,5 +53,6 @@
> PcdLib
>
> [Pcd]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType ## PRODUCES
> + gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> index 01f78bf0be..edcdb72a79 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
> @@ -32,10 +32,22 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> TPM2_PTP_INTERFACE_TYPE
> Tpm2GetPtpInterface (
> IN VOID *Register
> );
>
> +/**
> + Return PTP CRB interface IdleByPass state.
> +
> + @param[in] Register Pointer to PTP register.
> +
> + @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> + IN VOID *Register
> + );
> +
> /**
> Dump PTP register information.
>
> @param[in] Register Pointer to PTP register.
> **/
> @@ -95,10 +107,11 @@ Tpm2InstanceLibDTpmConstructor (
> VOID
> )
> {
> EFI_STATUS Status;
> TPM2_PTP_INTERFACE_TYPE PtpInterface;
> + UINT8 IdleByPass;
>
> Status = Tpm2RegisterTpm2DeviceLib (&mDTpm2InternalTpm2Device);
> if ((Status == EFI_SUCCESS) || (Status == EFI_UNSUPPORTED)) {
> //
> // Unsupported means platform policy does not need this instance enabled.
> @@ -109,10 +122,16 @@ Tpm2InstanceLibDTpmConstructor (
> //
> if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
> PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
> }
> +
> + if (PcdGet8(PcdActiveTpmInterfaceType) == PtpInterfaceCrb && PcdGet8(PcdCRBIdleByPass) == 0xFF) {
> + IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> + PcdSet8S(PcdCRBIdleByPass, IdleByPass);
> + }
> +
> DumpPtpInfo ((VOID *) (UINTN) PcdGet64 (PcdTpmBaseAddress));
> }
> return EFI_SUCCESS;
> }
> return Status;
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> index 876a5a63c4..24e4c35d55 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> @@ -48,6 +48,7 @@
> DebugLib
> PcdLib
>
> [Pcd]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## CONSUMES
> - gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType ## PRODUCES
> \ No newline at end of file
> + gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType ## PRODUCES
> + gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass ## PRODUCES
> \ No newline at end of file
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1bc153a2c0..5bce9f8e02 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -172,14 +172,34 @@ PtpCrbTpmCommand (
> DEBUG ((EFI_D_VERBOSE, "%02x ", BufferIn[Index]));
> }
> }
> DEBUG ((EFI_D_VERBOSE, "\n"));
> );
> - TpmOutSize = 0;
> + TpmOutSize = 0;
>
> //
> // STEP 0:
> + // if CapCRbIdelByPass == 0, enforce Idle state before sending command
> + //
> + if (PcdGet8(PcdCRBIdleByPass) == 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)) {
> + //
> + // 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);
> @@ -189,25 +209,25 @@ PtpCrbTpmCommand (
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
> PTP_TIMEOUT_C
> );
> if (EFI_ERROR (Status)) {
> Status = EFI_DEVICE_ERROR;
> - goto Exit;
> + 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 Exit;
> + goto GoIdle_Exit;
> }
>
> //
> - // STEP 1:
> + // STEP 2:
> // Command Reception occurs following a Ready state between the write of the
> // first byte of a command to the Command Buffer and the receipt of a write
> // of 1 to Start.
> //
> for (Index = 0; Index < SizeIn; Index++) {
> @@ -219,11 +239,11 @@ PtpCrbTpmCommand (
>
> MmioWrite64 ((UINTN)&CrbReg->CrbControlResponseAddrss, (UINT32)(UINTN)CrbReg->CrbDataBuffer);
> MmioWrite32 ((UINTN)&CrbReg->CrbControlResponseSize, sizeof(CrbReg->CrbDataBuffer));
>
> //
> - // STEP 2:
> + // STEP 3:
> // Command Execution occurs after receipt of a 1 to Start and the TPM
> // clearing Start to 0.
> //
> MmioWrite32((UINTN)&CrbReg->CrbControlStart, PTP_CRB_CONTROL_START);
> Status = PtpCrbWaitRegisterBits (
> @@ -249,16 +269,16 @@ PtpCrbTpmCommand (
> if (EFI_ERROR(Status)) {
> //
> // Still in Command Execution state. Try to goIdle, the behavior is agnostic.
> //
> Status = EFI_DEVICE_ERROR;
> - goto Exit;
> + goto GoIdle_Exit;
> }
> }
>
> //
> - // STEP 3:
> + // STEP 4:
> // Command Completion occurs after completion of a command (indicated by the
> // TPM clearing TPM_CRB_CTRL_Start_x to 0) and before a write of a 1 by the
> // software to Request.goIdle.
> //
>
> @@ -281,40 +301,72 @@ PtpCrbTpmCommand (
> CopyMem (&Data16, BufferOut, sizeof (UINT16));
> // TPM2 should not use this RSP_COMMAND
> if (SwapBytes16 (Data16) == TPM_ST_RSP_COMMAND) {
> DEBUG ((EFI_D_ERROR, "TPM2: TPM_ST_RSP error - %x\n", TPM_ST_RSP_COMMAND));
> Status = EFI_UNSUPPORTED;
> - goto Exit;
> + goto GoIdle_Exit;
> }
>
> CopyMem (&Data32, (BufferOut + 2), sizeof (UINT32));
> TpmOutSize = SwapBytes32 (Data32);
> if (*SizeOut < TpmOutSize) {
> + //
> + // Command completed, but buffer is not enough
> + //
> Status = EFI_BUFFER_TOO_SMALL;
> - goto Exit;
> + goto GoReady_Exit;
> }
> *SizeOut = TpmOutSize;
> //
> // Continue reading the remaining data
> //
> for (Index = sizeof (TPM2_RESPONSE_HEADER); Index < TpmOutSize; Index++) {
> BufferOut[Index] = MmioRead8 ((UINTN)&CrbReg->CrbDataBuffer[Index]);
> }
> -Exit:
> +
> DEBUG_CODE (
> DEBUG ((EFI_D_VERBOSE, "PtpCrbTpmCommand Receive - "));
> for (Index = 0; Index < TpmOutSize; Index++) {
> DEBUG ((EFI_D_VERBOSE, "%02x ", BufferOut[Index]));
> }
> DEBUG ((EFI_D_VERBOSE, "\n"));
> );
>
> +GoReady_Exit:
> //
> - // STEP 4:
> - // Idle is any time TPM_CRB_CTRL_STS_x.Status.goIdle is 1.
> + // Goto Ready State if command is completed succesfully and TPM support IdleBypass
> + // If not supported. flow down to GoIdle
> + //
> + if (PcdGet8(PcdCRBIdleByPass) == 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.
> + //
> +GoIdle_Exit:
> +
> + //
> + // Return to Idle state by setting TPM_CRB_CTRL_STS_x.Status.goIdle to 1.
> //
> MmioWrite32((UINTN)&CrbReg->CrbControlRequest, PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
> +
> + //
> + // Only enforce Idle state transition if execution fails when CRBIndleBypass==1
> + // Leave regular Idle delay at the beginning of next command execution
> + //
> + if (PcdGet8(PcdCRBIdleByPass) == 1){
> + Status = PtpCrbWaitRegisterBits (
> + &CrbReg->CrbControlStatus,
> + PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
> + 0,
> + PTP_TIMEOUT_C
> + );
> + }
> +
> return Status;
> }
>
> /**
> Send a command to TPM for execution and return response data.
> @@ -392,10 +444,32 @@ Tpm2GetPtpInterface (
> return Tpm2PtpInterfaceFifo;
> }
> return Tpm2PtpInterfaceTis;
> }
>
> +/**
> + Return PTP CRB interface IdleByPass state.
> +
> + @param[in] Register Pointer to PTP register.
> +
> + @return PTP CRB interface IdleByPass state.
> +**/
> +UINT8
> +Tpm2GetIdleByPass (
> + IN VOID *Register
> + )
> +{
> + PTP_CRB_INTERFACE_IDENTIFIER InterfaceId;
> +
> + //
> + // Check interface id
> + //
> + InterfaceId.Uint32 = MmioRead32 ((UINTN)&((PTP_CRB_REGISTERS *)Register)->InterfaceId);
> +
> + return (UINT8)(InterfaceId.Bits.CapCRBIdleBypass);
> +}
> +
> /**
> Dump PTP register information.
>
> @param[in] Register Pointer to PTP register.
> **/
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 60f1c0a0e3..e24b563bdb 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -472,7 +472,17 @@
> # 0xFF - Contains no current active TPM interface type.<BR>
> #
> # @Prompt current active TPM interface type.
> gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType|0xFF|UINT8|0x0001001E
>
> + ## This PCD records IdleByass status supported by current active TPM interface.
> + # Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and
> + # diretcly move to CmdReady state. <BR>
> + # 0x00 - Do not support IdleByPass.<BR>
> + # 0x01 - Support IdleByPass.<BR>
> + # 0xFF - IdleByPass State is not synced with TPM hardware.<BR>
> + #
> + # @Prompt IdleByass status supported by current active TPM interface.
> + gEfiSecurityPkgTokenSpaceGuid.PcdCRBIdleByPass|0xFF|UINT8|0x0001001F
> +
> [UserExtensions.TianoCore."ExtraFiles"]
> SecurityPkgExtra.uni
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index c34250e423..000bc83d80 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -252,6 +252,14 @@
>
> #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdActiveTpmInterfaceType_HELP #language en-US "This PCD indicates current active TPM interface type.\n"
> "0x00 - FIFO interface as defined in TIS 1.3 is active.<BR>\n"
> "0x01 - FIFO interface as defined in PTP for TPM 2.0 is active.<BR>\n"
> "0x02 - CRB interface is active.<BR>\n"
> - "0xFF - Contains no current active TPM interface type<BR>"
> \ No newline at end of file
> + "0xFF - Contains no current active TPM interface type<BR>"
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdCRBIdleByPass_PROMPT #language en-US "IdleByass status supported by current active TPM interface."
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdCRBIdleByPass_HELP #language en-US "This PCD records IdleByass status supported by current active TPM interface.\n"
> + "Accodingt to TCG PTP spec 1.3, TPM with CRB interface can skip idle state and diretcly move to CmdReady state. <BR>"
> + "0x01 - Do not support IdleByPass.<BR>\n"
> + "0x02 - Support IdleByPass.<BR>\n"
> + "0xFF - IdleByPass State is not synced with TPM hardware.<BR>"
> \ No newline at end of file
> --
> 2.16.2.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
next prev parent reply other threads:[~2018-07-03 23:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-25 4:44 [Patch 0/2] Add CRB IdleByPass Support Zhang, Chao B
2018-06-25 4:44 ` [Patch 1/2] Add CapCRBIdleBypass definition to interface ID register. It complies with existing register Zhang, Chao B
2018-06-25 4:44 ` [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support Zhang, Chao B
2018-06-27 8:18 ` Gary Lin
2018-07-03 23:25 ` Zhang, Chao B [this message]
2018-06-27 0:55 ` [Patch 0/2] Add CRB IdleByPass Support Long, Qin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=FF72C7E4248F3C4E9BDF19D4918E90F2497FE67A@SHSMSX101.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox