public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: devel@edk2.groups.io, min.m.xu@intel.com
Cc: Michael D Kinney <michael.d.kinney@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Zhiguang Liu <zhiguang.liu@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Jian J Wang <jian.j.wang@intel.com>, nd <nd@arm.com>,
	Joey Gouly <Joey.Gouly@arm.com>
Subject: Re: [edk2-devel] [PATCH V2 2/3] SecurityPkg: Support TdProtocol in DxeTpm2MeasureBootLib
Date: Tue, 19 Oct 2021 14:22:33 +0100	[thread overview]
Message-ID: <8f972fd2-30fb-9eb2-0ec4-f10031faff04@arm.com> (raw)
In-Reply-To: <09c7d8dd1c1856d8d0295064c8b833dd3d85e8c4.1633661591.git.min.m.xu@intel.com>

Hi Min, Jiewen,

Thank you for this patch.

I think this patch would need updating based on the changes done to 
patch 1/3.

Other than that I have some general feedback marked inline as [SAMI].

Regards,

Sami Mujawar


On 08/10/2021 06:21 AM, Min Xu via groups.io wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> DxeTpm2MeasureBootLib supports TPM2 based measure boot. After
> Td protocol is introduced, TD based measure boot needs to be supported
> in DxeTpm2MeasureBootLib as well.
>
> There are 2 major changes in this commit.
>
> 1. MEASURE_BOOT_PROTOCOLS is defined to store the instances of TCG2
> protocol and TD protocol. In the DxeTpm2MeasureBootHandler above 2
> measure boot protocol instances will be located. Then the located
> protocol instances will be called to do the measure boot.
>
> 2. TdEvent is similar to Tcg2Event except the MrIndex and PcrIndex.
> CreateTdEventFromTcg2Event is used to create the TdEvent based on the
> Tcg2Event.
>
> Above 2 changes make the minimize changes to the existing code.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   .../DxeTpm2MeasureBootLib.c                   | 346 ++++++++++++++----
>   .../DxeTpm2MeasureBootLib.inf                 |   1 +
>   2 files changed, 279 insertions(+), 68 deletions(-)
>
> diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> index 92eac715800f..f523a1a7a9d6 100644
> --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> @@ -41,6 +41,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Library/PeCoffLib.h>
>   #include <Library/SecurityManagementLib.h>
>   #include <Library/HobLib.h>
> +#include <Protocol/TdProtocol.h>
> +
> +typedef struct {
> +  EFI_TCG2_PROTOCOL     *Tcg2Protocol;
> +  EFI_TD_PROTOCOL       *TdProtocol;
> +} MEASURE_BOOT_PROTOCOLS;
>   
>   //
>   // Flag to check GPT partition. It only need be measured once.
> @@ -55,6 +61,56 @@ UINTN                             mTcg2ImageSize;
>   EFI_HANDLE                        mTcg2CacheMeasuredHandle  = NULL;
>   MEASURED_HOB_DATA                 *mTcg2MeasuredHobData     = NULL;
>   
> +/**
> +  Create TdEvent from Tcg2Event.
> +
> +  TdEvent is similar to Tcg2Event except the MrIndex.
> +
> +  @param  TdProtocol  Pointer to the located Td protocol instance.
> +  @param  Tcg2Event   Pointer to the Tcg2Event.
> +  @param  EventSize   Size of the Event.
> +
> +  @retval Pointer to the created TdEvent.
> +**/
> +EFI_TD_EVENT *
> +CreateTdEventFromTcg2Event (
> +  IN  EFI_TD_PROTOCOL *TdProtocol,
> +  IN  EFI_TCG2_EVENT  *Tcg2Event,
> +  IN  UINT32          EventSize
> +  )
> +{
> +  EFI_TD_EVENT    *TdEvent;
> +  UINT32          MrIndex;
> +  EFI_STATUS      Status;
> +
> +  TdEvent = NULL;
> +  if (Tcg2Event == NULL || TdProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return NULL;
> +  }
> +
> +  Status = TdProtocol->MapPcrToMrIndex (TdProtocol, Tcg2Event->Header.PCRIndex, &MrIndex);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Cannot map PcrIndex(%d) to MrIndex\n", Tcg2Event->Header.PCRIndex));
> +    return NULL;
> +  }
> +
> +  TdEvent = (EFI_TD_EVENT *)AllocateZeroPool (Tcg2Event->Size);
> +  if (TdEvent == NULL) {
> +    ASSERT (FALSE);
> +    return NULL;
> +  }
> +
> +  TdEvent->Size                 = Tcg2Event->Size;
> +  TdEvent->Header.HeaderSize    = Tcg2Event->Header.HeaderSize;
> +  TdEvent->Header.HeaderVersion = Tcg2Event->Header.HeaderVersion;
> +  TdEvent->Header.MrIndex       = MrIndex;
> +  TdEvent->Header.EventType     = Tcg2Event->Header.EventType;
> +  CopyMem (TdEvent->Event, Tcg2Event->Event, EventSize);
> +
> +  return TdEvent;
> +}
> +
>   /**
>     Reads contents of a PE/COFF image in memory buffer.
>   
> @@ -109,7 +165,7 @@ DxeTpm2MeasureBootLibImageRead (
>     Caution: This function may receive untrusted input.
>     The GPT partition table is external input, so this function should parse partition data carefully.
>   
> -  @param Tcg2Protocol            Pointer to the located TCG2 protocol instance.
> +  @param MeasureBootProtocols    Pointer to the located MeasureBoot protocol instances (i.e. TCG2/Td protocol).
>     @param GptHandle               Handle that GPT partition was installed.
>   
>     @retval EFI_SUCCESS            Successfully measure GPT table.
> @@ -121,8 +177,8 @@ DxeTpm2MeasureBootLibImageRead (
>   EFI_STATUS
>   EFIAPI
>   Tcg2MeasureGptTable (
> -  IN  EFI_TCG2_PROTOCOL  *Tcg2Protocol,
> -  IN  EFI_HANDLE         GptHandle
> +  IN  MEASURE_BOOT_PROTOCOLS  *MeasureBootProtocols,
> +  IN  EFI_HANDLE              GptHandle
>     )
>   {
>     EFI_STATUS                        Status;
> @@ -134,13 +190,24 @@ Tcg2MeasureGptTable (
>     UINTN                             NumberOfPartition;
>     UINT32                            Index;
>     EFI_TCG2_EVENT                    *Tcg2Event;
> +  EFI_TD_EVENT                      *TdEvent;
>     EFI_GPT_DATA                      *GptData;
>     UINT32                            EventSize;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> +  EFI_TD_PROTOCOL                   *TdProtocol;
>   
>     if (mTcg2MeasureGptCount > 0) {
>       return EFI_SUCCESS;
>     }
>   
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  TdProtocol    = MeasureBootProtocols->TdProtocol;
> +
> +  if (Tcg2Protocol == NULL && TdProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>     Status = gBS->HandleProtocol (GptHandle, &gEfiBlockIoProtocolGuid, (VOID**)&BlockIo);
>     if (EFI_ERROR (Status)) {
>       return EFI_UNSUPPORTED;
> @@ -149,6 +216,7 @@ Tcg2MeasureGptTable (
>     if (EFI_ERROR (Status)) {
>       return EFI_UNSUPPORTED;
>     }
> +
>     //
>     // Read the EFI Partition Table Header
>     //
> @@ -156,6 +224,15 @@ Tcg2MeasureGptTable (
>     if (PrimaryHeader == NULL) {
>       return EFI_OUT_OF_RESOURCES;
>     }
> +
> +  //
> +  // PrimaryHeader->SizeOfPartitionEntry should not be zero
> +  //
> +  if (PrimaryHeader->SizeOfPartitionEntry == 0) {
> +    DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry should not be zero!\n"));
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
[SAMI] I think this check is at an incorrect location. Should this be 
after the ReadDisk() below? Also, PrimaryHeader would need to be freed 
in the error scenario above.
> +
>     Status = DiskIo->ReadDisk (
>                        DiskIo,
>                        BlockIo->Media->MediaId,
> @@ -164,7 +241,7 @@ Tcg2MeasureGptTable (
>                        (UINT8 *)PrimaryHeader
>                        );
>     if (EFI_ERROR (Status)) {
> -    DEBUG ((EFI_D_ERROR, "Failed to Read Partition Table Header!\n"));
> +    DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
>       FreePool (PrimaryHeader);
>       return EFI_DEVICE_ERROR;
>     }
> @@ -201,16 +278,18 @@ Tcg2MeasureGptTable (
>       PartitionEntry = (EFI_PARTITION_ENTRY *)((UINT8 *)PartitionEntry + PrimaryHeader->SizeOfPartitionEntry);
>     }
>   
> +  TdEvent = NULL;
> +  Tcg2Event = NULL;
> +
>     //
> -  // Prepare Data for Measurement
> +  // Prepare Data for Measurement (TdProtocol and Tcg2Protocol)
>     //
>     EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
>                           + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
>     Tcg2Event = (EFI_TCG2_EVENT *) AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event));
>     if (Tcg2Event == NULL) {
> -    FreePool (PrimaryHeader);
> -    FreePool (EntryPtr);
> -    return EFI_OUT_OF_RESOURCES;
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Exit;
>     }
>   
>     Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event);
> @@ -242,23 +321,56 @@ Tcg2MeasureGptTable (
>       PartitionEntry =(EFI_PARTITION_ENTRY *)((UINT8 *)PartitionEntry + PrimaryHeader->SizeOfPartitionEntry);
>     }
>   
> +  if (TdProtocol != NULL) {
> +    TdEvent = CreateTdEventFromTcg2Event (TdProtocol, Tcg2Event, EventSize);
> +    if (TdEvent == NULL) {
> +      goto Exit;
[SAMI] I think Status should be set to reflect an appropriate error code 
here. Also would it be possible to create this event just before calling 
TdProtocol->HashLogExtendEvent at line 351?
I am trying to understand why is this done differently in 
Tcg2MeasurePeImage() i.e. The TdEvent is created and extended in the 
same if (TdProtocol != NULL) block.
[/SAMI]
> +    }
> +  }
> +
> +  //
> +  // Measure the GPT data by Tcg2Protocol
> +  //
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               Tcg2Event
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasureGptTable - %r\n", Status));
> +  }
> +
> +  //
> +  // Measure the GPT data by TdProtocol
>     //
> -  // Measure the GPT data
> -  //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             0,
> -             (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> -             (UINT64) EventSize,
> -             Tcg2Event
> -             );
> -  if (!EFI_ERROR (Status)) {
> -    mTcg2MeasureGptCount++;
> +  if (TdProtocol != NULL) {
> +    Status = TdProtocol->HashLogExtendEvent (
> +               TdProtocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               TdEvent
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Td MeasureGptTable - %r\n", Status));
>     }
>   
> +Exit:
>     FreePool (PrimaryHeader);
>     FreePool (EntryPtr);
> -  FreePool (Tcg2Event);
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +  if (TdEvent != NULL) {
> +    FreePool (TdEvent);
> +  }
>   
>     return Status;
>   }
> @@ -271,12 +383,12 @@ Tcg2MeasureGptTable (
>     PE/COFF image is external input, so this function will validate its data structure
>     within this image buffer before use.
>   
> -  @param[in] Tcg2Protocol   Pointer to the located TCG2 protocol instance.
> -  @param[in] ImageAddress   Start address of image buffer.
> -  @param[in] ImageSize      Image size
> -  @param[in] LinkTimeBase   Address that the image is loaded into memory.
> -  @param[in] ImageType      Image subsystem type.
> -  @param[in] FilePath       File path is corresponding to the input image.
> +  @param[in] MeasureBootProtocols   Pointer to the located MeasureBoot protocol instances.
> +  @param[in] ImageAddress           Start address of image buffer.
> +  @param[in] ImageSize              Image size
> +  @param[in] LinkTimeBase           Address that the image is loaded into memory.
> +  @param[in] ImageType              Image subsystem type.
> +  @param[in] FilePath               File path is corresponding to the input image.
>   
>     @retval EFI_SUCCESS            Successfully measure image.
>     @retval EFI_OUT_OF_RESOURCES   No enough resource to measure image.
> @@ -287,7 +399,7 @@ Tcg2MeasureGptTable (
>   EFI_STATUS
>   EFIAPI
>   Tcg2MeasurePeImage (
> -  IN  EFI_TCG2_PROTOCOL         *Tcg2Protocol,
> +  IN  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols,
>     IN  EFI_PHYSICAL_ADDRESS      ImageAddress,
>     IN  UINTN                     ImageSize,
>     IN  UINTN                     LinkTimeBase,
> @@ -300,9 +412,22 @@ Tcg2MeasurePeImage (
>     EFI_IMAGE_LOAD_EVENT              *ImageLoad;
>     UINT32                            FilePathSize;
>     UINT32                            EventSize;
> +  EFI_TD_EVENT                      *TdEvent;
> +  EFI_TD_PROTOCOL                   *TdProtocol;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
>   
>     Status        = EFI_UNSUPPORTED;
>     ImageLoad     = NULL;
> +  TdEvent       = NULL;
> +
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  TdProtocol    = MeasureBootProtocols->TdProtocol;
> +
> +  if (Tcg2Protocol == NULL && TdProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>     FilePathSize  = (UINT32) GetDevicePathSize (FilePath);
>   
>     //
> @@ -334,7 +459,7 @@ Tcg2MeasurePeImage (
>         break;
>       default:
>         DEBUG ((
> -        EFI_D_ERROR,
> +        DEBUG_ERROR,
>           "Tcg2MeasurePeImage: Unknown subsystem type %d",
>           ImageType
>           ));
> @@ -352,28 +477,124 @@ Tcg2MeasurePeImage (
>     //
>     // Log the PE data
>     //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             PE_COFF_IMAGE,
> -             ImageAddress,
> -             ImageSize,
> -             Tcg2Event
> -             );
> -  if (Status == EFI_VOLUME_FULL) {
> -    //
> -    // Volume full here means the image is hashed and its result is extended to PCR.
> -    // But the event log can't be saved since log area is full.
> -    // Just return EFI_SUCCESS in order not to block the image load.
> -    //
> -    Status = EFI_SUCCESS;
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               Tcg2Event
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasurePeImage - %r\n", Status));
> +  }
> +
> +  if (TdProtocol != NULL) {
> +    TdEvent = CreateTdEventFromTcg2Event (TdProtocol, Tcg2Event, EventSize);
> +    if (TdEvent == NULL) {
> +      goto Finish;
[SAMI] I think Status should be set to reflect an appropriate error code 
here.
> +    }
> +
> +    Status = TdProtocol->HashLogExtendEvent (
> +               TdProtocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               TdEvent
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Td MeasurePeImage - %r\n", Status));
>     }
>   
>   Finish:
> -  FreePool (Tcg2Event);
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +
> +  if (TdEvent != NULL) {
> +    FreePool (TdEvent);
> +  }
>   
>     return Status;
>   }
>   
> +/**
> +  Get the measure boot protocols.
> +
> +  There are 2 measure boot, TCG2 protocol based and Td protocol based.
> +
> +  @param  MeasureBootProtocols  Pointer to the located measure boot protocol instances.
> +
> +  @retval EFI_SUCCESS           Sucessfully locate the measure boot protocol instances (at least one instance).
> +  @retval EFI_UNSUPPORTED       Measure boot is not supported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetMeasureBootProtocols (
> +  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  EFI_TD_PROTOCOL                     *TdProtocol;
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY    Tcg2ProtocolCapability;
> +  EFI_TD_BOOT_SERVICE_CAPABILITY      TdProtocolCapability;
> +
> +  TdProtocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiTdProtocolGuid, NULL, (VOID **) &TdProtocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // TdTcg2 protocol is not installed.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "TdProtocol is not installed. - %r\n", Status));
> +  } else {
> +    TdProtocolCapability.Size = sizeof (TdProtocolCapability);
> +    Status = TdProtocol->GetCapability (TdProtocol, &TdProtocolCapability);
> +    if (EFI_ERROR (Status) || !TdProtocolCapability.TdPresentFlag) {
> +      DEBUG ((DEBUG_ERROR, "TdPresentFlag=FALSE. %r\n", Status));
> +      TdProtocol = NULL;
> +    }
> +  }
> +
> +  Tcg2Protocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Tcg2 protocol is not installed. So, TPM2 is not present.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "Tcg2Protocol is not installed. - %r\n", Status));
> +  } else {
> +    Tcg2ProtocolCapability.Size = (UINT8) sizeof (Tcg2ProtocolCapability);
> +    Status = Tcg2Protocol->GetCapability (Tcg2Protocol, &Tcg2ProtocolCapability);
> +    if (EFI_ERROR (Status) || (!Tcg2ProtocolCapability.TPMPresentFlag)) {
> +      //
> +      // TPM device doesn't work or activate.
> +      //
> +      DEBUG ((DEBUG_ERROR, "TPMPresentFlag=FALSE %r\n", Status));
> +      Tcg2Protocol = NULL;
> +    }
> +  }
> +
> +  MeasureBootProtocols->Tcg2Protocol = Tcg2Protocol;
> +  MeasureBootProtocols->TdProtocol = TdProtocol;
> +
> +  return (Tcg2Protocol == NULL && TdProtocol == NULL) ? EFI_UNSUPPORTED: EFI_SUCCESS;
> +}
> +
>   /**
>     The security handler is used to abstract platform-specific policy
>     from the DXE core response to an attempt to use a file that returns a
> @@ -422,9 +643,8 @@ DxeTpm2MeasureBootHandler (
>     IN  BOOLEAN                          BootPolicy
>     )
>   {
> -  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  MEASURE_BOOT_PROTOCOLS              MeasureBootProtocols;
>     EFI_STATUS                          Status;
> -  EFI_TCG2_BOOT_SERVICE_CAPABILITY    ProtocolCapability;
>     EFI_DEVICE_PATH_PROTOCOL            *DevicePathNode;
>     EFI_DEVICE_PATH_PROTOCOL            *OrigDevicePathNode;
>     EFI_HANDLE                          Handle;
> @@ -435,28 +655,19 @@ DxeTpm2MeasureBootHandler (
>     EFI_PHYSICAL_ADDRESS                FvAddress;
>     UINT32                              Index;
>   
> -  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +  MeasureBootProtocols.Tcg2Protocol = NULL;
> +  MeasureBootProtocols.TdProtocol   = NULL;
> +
> +  Status = GetMeasureBootProtocols(&MeasureBootProtocols);
> +
>     if (EFI_ERROR (Status)) {
> -    //
> -    // Tcg2 protocol is not installed. So, TPM2 is not present.
> -    // Don't do any measurement, and directly return EFI_SUCCESS.
> -    //
[SAMI] It may be helpful to retain the oirginal comment with slight 
rewording.
> -    DEBUG ((EFI_D_VERBOSE, "DxeTpm2MeasureBootHandler - Tcg2 - %r\n", Status));
> +    DEBUG ((DEBUG_INFO, "None of Tcg2Protocol/TdProtocol is installed.\n"));
>       return EFI_SUCCESS;
>     }
>   
> -  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
> -  Status = Tcg2Protocol->GetCapability (
> -                           Tcg2Protocol,
> -                           &ProtocolCapability
> -                           );
> -  if (EFI_ERROR (Status) || (!ProtocolCapability.TPMPresentFlag)) {
> -    //
> -    // TPM device doesn't work or activate.
> -    //
> -    DEBUG ((EFI_D_ERROR, "DxeTpm2MeasureBootHandler (%r) - TPMPresentFlag - %x\n", Status, ProtocolCapability.TPMPresentFlag));
> -    return EFI_SUCCESS;
> -  }
> +  DEBUG ((DEBUG_INFO, "Tcg2Protocol = %p, TdProtocol = %p\n",
> +                      MeasureBootProtocols.Tcg2Protocol,
> +                      MeasureBootProtocols.TdProtocol));
>   
>     //
>     // Copy File Device Path
> @@ -502,8 +713,8 @@ DxeTpm2MeasureBootHandler (
>               //
>               // Measure GPT disk.
>               //
> -            Status = Tcg2MeasureGptTable (Tcg2Protocol, Handle);
> -            DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasureGptTable - %r\n", Status));
> +            Status = Tcg2MeasureGptTable (&MeasureBootProtocols, Handle);
> +
>               if (!EFI_ERROR (Status)) {
>                 //
>                 // GPT disk check done.
> @@ -647,14 +858,13 @@ DxeTpm2MeasureBootHandler (
>       // Measure PE image into TPM log.
>       //
>       Status = Tcg2MeasurePeImage (
> -               Tcg2Protocol,
> +               &MeasureBootProtocols,
>                  (EFI_PHYSICAL_ADDRESS) (UINTN) FileBuffer,
>                  FileSize,
>                  (UINTN) ImageContext.ImageAddress,
>                  ImageContext.ImageType,
>                  DevicePathNode
>                  );
> -    DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasurePeImage - %r\n", Status));
>     }
>   
>     //
> @@ -665,7 +875,7 @@ Finish:
>       FreePool (OrigDevicePathNode);
>     }
>   
> -  DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
> +  DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
>   
>     return Status;
>   }
> diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> index 2506abbe7c8b..29b62c3ba8fa 100644
> --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> @@ -61,6 +61,7 @@
>   
>   [Protocols]
>     gEfiTcg2ProtocolGuid                  ## SOMETIMES_CONSUMES
> +  gEfiTdProtocolGuid
>     gEfiFirmwareVolumeBlockProtocolGuid   ## SOMETIMES_CONSUMES
>     gEfiBlockIoProtocolGuid               ## SOMETIMES_CONSUMES
>     gEfiDiskIoProtocolGuid                ## SOMETIMES_CONSUMES


  reply	other threads:[~2021-10-19 13:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08  5:21 [PATCH V2 0/3] Introduce TdProtocol into EDK2 Min Xu
2021-10-08  5:21 ` [PATCH V2 1/3] MdePkg: Introduce TdProtocol for TD-Guest firmware Min Xu
2021-10-11  1:37   ` 回复: " gaoliming
2021-10-19 13:21   ` [edk2-devel] " Sami Mujawar
2021-10-19 14:40     ` Yao, Jiewen
2021-10-20  9:26       ` Sami Mujawar
2021-10-08  5:21 ` [PATCH V2 2/3] SecurityPkg: Support TdProtocol in DxeTpm2MeasureBootLib Min Xu
2021-10-19 13:22   ` Sami Mujawar [this message]
2021-10-27  5:19     ` [edk2-devel] " Min Xu
2021-11-01 13:35       ` Sami Mujawar
2021-10-08  5:21 ` [PATCH V2 3/3] SecurityPkg: Support TdProtocol in DxeTpmMeasurementLib Min Xu
2021-10-19 13:24   ` [edk2-devel] " Sami Mujawar
2021-10-12 15:26 ` [edk2-devel] [PATCH V2 0/3] Introduce TdProtocol into EDK2 Sami Mujawar
2021-10-14  5:41   ` Min Xu
2021-10-14 11:59     ` Yao, Jiewen
     [not found]     ` <16ADE3D948B3147A.7007@groups.io>
2021-10-14 13:43       ` Yao, Jiewen
2021-10-18 12:59         ` Sami Mujawar
2021-10-18 13:06           ` Yao, Jiewen
2021-10-19  9:51             ` Sami Mujawar
2021-10-19 13:06               ` Min Xu

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=8f972fd2-30fb-9eb2-0ec4-f10031faff04@arm.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