Qi, A few comments below.
Regards, Jian > -----Original Message----- > From: Zhang, Qi1 <qi1.zh...@intel.com> > Sent: Friday, July 17, 2020 4:50 PM > To: devel@edk2.groups.io > Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; > Zhang, Qi1 <qi1.zh...@intel.com>; Kumar, Rahul1 <rahul1.ku...@intel.com> > Subject: [PATCH v4 4/6] SecurityPkg/Tcg2: Add TcgPpi > > From: Jiewen Yao <jiewen....@intel.com> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2841 > > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Qi Zhang <qi1.zh...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Signed-off-by: Jiewen Yao <jiewen....@intel.com> > --- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 110 +++++++++++++++++++++------- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 3 +- > 2 files changed, 86 insertions(+), 27 deletions(-) > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 19b8e4b318..592f760057 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -1,7 +1,7 @@ > /** @file > > Initialize TPM2 device and measure FVs before handing off control to DXE. > > > > -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR> > > +Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR> > > Copyright (c) 2017, Microsoft Corporation. All rights reserved. <BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -17,6 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Ppi/EndOfPeiPhase.h> > > #include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h> > > #include <Ppi/FirmwareVolumeInfoPrehashedFV.h> > > +#include <Ppi/Tcg.h> > > > > #include <Guid/TcgEventHob.h> > > #include <Guid/MeasuredFvHob.h> > > @@ -66,6 +67,48 @@ EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList > = { > NULL > > }; > > > > +/** > > + Do a hash operation on a data buffer, extend a specific TPM PCR with the > hash result, > > + and build a GUIDed HOB recording the event which will be passed to the DXE > phase and > > + added into the Event Log. > > + > > + @param[in] This Indicates the calling context > > + @param[in] Flags Bitmap providing additional information. > > + @param[in] HashData If BIT0 of Flags is 0, it is physical > address of the > > + start of the data buffer to be hashed, > extended, and logged. > > + If BIT0 of Flags is 1, it is physical > address of the > > + start of the pre-hash data buffter to be > extended, and logged. > > + The pre-hash data format is > TPML_DIGEST_VALUES. > > + @param[in] HashDataLen The length, in bytes, of the buffer > referenced by > HashData. > > + @param[in] NewEventHdr Pointer to a TCG_PCR_EVENT_HDR data > structure. > > + @param[in] NewEventData Pointer to the new event data. > > + > > + @retval EFI_SUCCESS Operation completed successfully. > > + @retval EFI_OUT_OF_RESOURCES No enough memory to log the new event. > > + @retval EFI_DEVICE_ERROR The command was unsuccessful. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +HashLogExtendEvent ( > > + IN EDKII_TCG_PPI *This, > > + IN UINT64 Flags, > > + IN UINT8 *HashData, > > + IN UINTN HashDataLen, > > + IN TCG_PCR_EVENT_HDR *NewEventHdr, > > + IN UINT8 *NewEventData > > + ); > > + > > +EDKII_TCG_PPI mEdkiiTcgPpi = { > > + HashLogExtendEvent > > +}; > > + > > +EFI_PEI_PPI_DESCRIPTOR mTcgPpiList = { > > + EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, > > + &gEdkiiTcgPpiGuid, > > + &mEdkiiTcgPpi > > +}; > > + > > // > > // Number of firmware blobs to grow by each time we run out of room > > // > > @@ -375,9 +418,13 @@ LogHashEvent ( > and build a GUIDed HOB recording the event which will be passed to the DXE > phase and > > added into the Event Log. > > > > + @param[in] This Indicates the calling context > > @param[in] Flags Bitmap providing additional information. > > - @param[in] HashData Physical address of the start of the data > buffer > > - to be hashed, extended, and logged. > > + @param[in] HashData If BIT0 of Flags is 0, it is physical > address of the > > + start of the data buffer to be hashed, > extended, and logged. > > + If BIT0 of Flags is 1, it is physical > address of the > > + start of the pre-hash data buffter to be > extended, and logged. > > + The pre-hash data format is > TPML_DIGEST_VALUES. > > @param[in] HashDataLen The length, in bytes, of the buffer > referenced by > HashData. > > @param[in] NewEventHdr Pointer to a TCG_PCR_EVENT_HDR data > structure. > > @param[in] NewEventData Pointer to the new event data. > > @@ -388,7 +435,9 @@ LogHashEvent ( > > > **/ > > EFI_STATUS > > +EFIAPI > > HashLogExtendEvent ( > > + IN EDKII_TCG_PPI *This, > > IN UINT64 Flags, > > IN UINT8 *HashData, > > IN UINTN HashDataLen, > > @@ -403,16 +452,23 @@ HashLogExtendEvent ( > return EFI_DEVICE_ERROR; > > } > > > > - Status = HashAndExtend ( > > - NewEventHdr->PCRIndex, > > - HashData, > > - HashDataLen, > > + if(Flags & EDKII_TCG_PRE_HASH) { > > + ZeroMem (&DigestList, sizeof(DigestList)); > > + CopyMem(&DigestList, HashData, sizeof(DigestList)); Missing a space before '('. > > + Status = Tpm2PcrExtend( Missing a space before '('. > > + 0, > > &DigestList > > ); > > + } else { > > + Status = HashAndExtend ( > > + NewEventHdr->PCRIndex, > > + HashData, > > + HashDataLen, > > + &DigestList > > + ); > > + } > > if (!EFI_ERROR (Status)) { > > - if ((Flags & EFI_TCG2_EXTEND_ONLY) == 0) { > > - Status = LogHashEvent (&DigestList, NewEventHdr, NewEventData); > > - } > > + Status = LogHashEvent (&DigestList, NewEventHdr, NewEventData); > > } > > > > if (Status == EFI_DEVICE_ERROR) { > > @@ -452,6 +508,7 @@ MeasureCRTMVersion ( > TcgEventHdr.EventSize = (UINT32) StrSize((CHAR16*)PcdGetPtr > (PcdFirmwareVersionString)); > > > > return HashLogExtendEvent ( > > + &mEdkiiTcgPpi, > > 0, > > (UINT8*)PcdGetPtr (PcdFirmwareVersionString), > > TcgEventHdr.EventSize, > > @@ -651,27 +708,22 @@ MeasureFvImage ( > // FV pre-hash algos comply with current TPM hash requirement > > // Skip hashing step in measure, only extend DigestList to PCR and log > event > > // > > - Status = Tpm2PcrExtend( > > - 0, > > - &DigestList > > + Status = HashLogExtendEvent ( > > + &mEdkiiTcgPpi, > > + EDKII_TCG_PRE_HASH, > > + (UINT8*) &DigestList, // HashData > > + (UINTN) sizeof(DigestList), // HashDataLen > > + &TcgEventHdr, // EventHdr > > + EventData // EventData > > ); > > - > > - if (!EFI_ERROR(Status)) { > > - Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData); > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by > Tcg2Pei starts at: 0x%x\n", FvBase)); > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by > Tcg2Pei has the size: 0x%x\n", FvLength)); > > - } else if (Status == EFI_DEVICE_ERROR) { > > - BuildGuidHob (&gTpmErrorHobGuid,0); > > - REPORT_STATUS_CODE ( > > - EFI_ERROR_CODE | EFI_ERROR_MINOR, > > - (PcdGet32 (PcdStatusCodeSubClassTpmDevice) | > EFI_P_EC_INTERFACE_ERROR) > > - ); > > - } Please explain the purpose of the removal of this part of code in commit message. It looks that it's not a relevant change to this patch series. > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by > Tcg2Pei starts at: 0x%x\n", FvBase)); > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by > Tcg2Pei has the size: 0x%x\n", FvLength)); > > } else { > > // > > // Hash the FV, extend digest to the TPM and log TCG event > > // > > Status = HashLogExtendEvent ( > > + &mEdkiiTcgPpi, > > 0, > > (UINT8*) (UINTN) FvBase, // HashData > > (UINTN) FvLength, // HashDataLen > > @@ -849,6 +901,12 @@ PeimEntryMP ( > { > > EFI_STATUS Status; > > > > + // > > + // install Tcg Services > > + // > > + Status = PeiServicesInstallPpi (&mTcgPpiList); > > + ASSERT_EFI_ERROR (Status); > > + > > if (PcdGet8 (PcdTpm2ScrtmPolicy) == 1) { > > Status = MeasureCRTMVersion (); > > } > > @@ -893,7 +951,7 @@ MeasureSeparatorEventWithError ( > TcgEvent.PCRIndex = PCRIndex; > > TcgEvent.EventType = EV_SEPARATOR; > > TcgEvent.EventSize = (UINT32)sizeof (EventData); > > - return HashLogExtendEvent(0,(UINT8 *)&EventData, TcgEvent.EventSize, > &TcgEvent,(UINT8 *)&EventData); > > + return HashLogExtendEvent(&mEdkiiTcgPpi, 0, (UINT8 *)&EventData, > TcgEvent.EventSize, &TcgEvent,(UINT8 *)&EventData); > > } > > > > /** > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > index 3d361e8859..f64b29f1ae 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > @@ -8,7 +8,7 @@ > # > > # This module will initialize TPM device, measure reported FVs and BIOS > version. > > # > > -# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR> > > # Copyright (c) 2017, Microsoft Corporation. All rights reserved. <BR> > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -72,6 +72,7 @@ > gPeiTpmInitializationDonePpiGuid ## > PRODUCES > > gEfiEndOfPeiSignalPpiGuid ## > SOMETIMES_CONSUMES > ## NOTIFY > > gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid ## > SOMETIMES_CONSUMES > > + gEdkiiTcgPpiGuid ## > PRODUCES > > > > [Pcd] > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ## > SOMETIMES_CONSUMES > > -- > 2.26.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62972): https://edk2.groups.io/g/devel/message/62972 Mute This Topic: https://groups.io/mt/75608832/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
