From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=195.135.221.5; helo=smtp.nue.novell.com; envelope-from=glin@suse.com; receiver=edk2-devel@lists.01.org Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 372B9211F887A for ; Wed, 27 Jun 2018 01:19:08 -0700 (PDT) Received: from emea4-mta.ukb.novell.com ([10.120.13.87]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Wed, 27 Jun 2018 10:19:06 +0200 Received: from GaryWorkstation (nwb-a10-snat.microfocus.com [10.120.13.202]) by emea4-mta.ukb.novell.com with ESMTP (TLS encrypted); Wed, 27 Jun 2018 09:18:35 +0100 Date: Wed, 27 Jun 2018 16:18:30 +0800 From: Gary Lin To: "Zhang, Chao B" Cc: edk2-devel@lists.01.org, Yao Jiewen , Long Qin Message-ID: <20180627081830.gvqh75toe74btbre@GaryWorkstation> References: <20180625044421.2028-1-chao.b.zhang@intel.com> <20180625044421.2028-3-chao.b.zhang@intel.com> MIME-Version: 1.0 In-Reply-To: <20180625044421.2028-3-chao.b.zhang@intel.com> User-Agent: NeoMutt/20170912 (1.9.0) Subject: Re: [Patch 2/2] SecurityPkg: Tpm2DeviceLib: Enable CapCRBIdleBypass support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jun 2018 08:19:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 > Cc: Yao Jiewen > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chao Zhang > Signed-off-by: Zhang, Chao B > --- > .../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.
> # > # @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.
> + # 0x00 - Do not support IdleByPass.
> + # 0x01 - Support IdleByPass.
> + # 0xFF - IdleByPass State is not synced with TPM hardware.
> + # > + # @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.
\n" > "0x01 - FIFO interface as defined in PTP for TPM 2.0 is active.
\n" > "0x02 - CRB interface is active.
\n" > - "0xFF - Contains no current active TPM interface type
" > \ No newline at end of file > + "0xFF - Contains no current active TPM interface type
" > + > +#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.
" > + "0x01 - Do not support IdleByPass.
\n" > + "0x02 - Support IdleByPass.
\n" > + "0xFF - IdleByPass State is not synced with TPM hardware.
" > \ No newline at end of file > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >