public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Min Xu" <min.m.xu@intel.com>
To: devel@edk2.groups.io
Cc: Min M Xu <min.m.xu@intel.com>,
	Erdem Aktas <erdemaktas@google.com>,
	James Bottomley <jejb@linux.ibm.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Michael Roth <michael.roth@amd.com>
Subject: [PATCH V6 06/12] OvmfPkg: Refactor MeaureFvImage
Date: Fri,  3 Feb 2023 11:31:41 +0800	[thread overview]
Message-ID: <20230203033147.1332-7-min.m.xu@intel.com> (raw)
In-Reply-To: <20230203033147.1332-1-min.m.xu@intel.com>

From: Min M Xu <min.m.xu@intel.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243

MeasureFvImage once was implemented in PeilessStartupLib and it does
measurement and logging for Configuration FV (Cfv) image in one go,
using TpmMeasureAndLogData(). But it doesn't work in SEC.

This patch splits MeasureFvImage into 2 functions and implement them in
SecTdxHelperLib.
 - TdxHelperMeasureCfvImage
 - TdxHelperBuildGuidHobForTdxMeasurement

TdxHelperMeasureCfvImage measures the Cfv image and stores the hash value
in WorkArea. TdxHelperBuildGuidHobForTdxMeasurement builds GuidHob for the
measurement based on the hash value in WorkArea.

After these 2 functions are introduced, PeilessStartupLib should also be
updated:
 - Call these 2 functions instead of the MeasureFvImage
 - Delete the duplicated codes in PeilessStartupLib

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c  |  30 ++++-
 .../IntelTdx/TdxHelperLib/TdxMeasurementHob.c |  86 +++++++++++++
 OvmfPkg/Library/PeilessStartupLib/IntelTdx.c  | 121 ------------------
 .../PeilessStartupLib/PeilessStartup.c        |  20 ++-
 .../PeilessStartupInternal.h                  |  21 ---
 .../PeilessStartupLib/PeilessStartupLib.inf   |   4 -
 6 files changed, 124 insertions(+), 158 deletions(-)
 delete mode 100644 OvmfPkg/Library/PeilessStartupLib/IntelTdx.c

diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
index 6ca6f01aff57..1929093f9110 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
@@ -175,7 +175,35 @@ TdxHelperMeasureCfvImage (
   VOID
   )
 {
-  return EFI_UNSUPPORTED;
+  EFI_STATUS      Status;
+  UINT8           Digest[SHA384_DIGEST_SIZE];
+  OVMF_WORK_AREA  *WorkArea;
+
+  Status = HashAndExtendToRtmr (
+             0,
+             (UINT8 *)(UINTN)PcdGet32 (PcdOvmfFlashNvStorageVariableBase),
+             (UINT64)PcdGet32 (PcdCfvRawDataSize),
+             Digest,
+             SHA384_DIGEST_SIZE
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // This function is called in SEC phase and at that moment the Hob service
+  // is not available. So CfvImage measurement value is stored in workarea.
+  //
+  WorkArea = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+  if (WorkArea == NULL) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.MeasurementsBitmap |= TDX_MEASUREMENT_CFVIMG_BITMASK;
+  CopyMem (WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.CfvImgHashValue, Digest, SHA384_DIGEST_SIZE);
+
+  return EFI_SUCCESS;
 }
 
 /**
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c b/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
index 6cbc7600adb6..a4c7095cffab 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
@@ -15,6 +15,7 @@
 #include <IndustryStandard/UefiTcgPlatform.h>
 #include <Library/HobLib.h>
 #include <Library/PrintLib.h>
+#include <Library/TcgEventLogRecordLib.h>
 #include <WorkArea.h>
 
 #pragma pack(1)
@@ -29,6 +30,9 @@ typedef struct {
 
 #pragma pack()
 
+#define FV_HANDOFF_TABLE_DESC  "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef PLATFORM_FIRMWARE_BLOB2_STRUCT CFV_HANDOFF_TABLE_POINTERS2;
+
 /**
  * Build GuidHob for Tdx measurement.
  *
@@ -112,6 +116,52 @@ BuildTdxMeasurementGuidHob (
   return EFI_SUCCESS;
 }
 
+/**
+  Get the FvName from the FV header.
+
+  Causion: The FV is untrusted input.
+
+  @param[in]  FvBase            Base address of FV image.
+  @param[in]  FvLength          Length of FV image.
+
+  @return FvName pointer
+  @retval NULL   FvName is NOT found
+**/
+VOID *
+GetFvName (
+  IN EFI_PHYSICAL_ADDRESS  FvBase,
+  IN UINT64                FvLength
+  )
+{
+  EFI_FIRMWARE_VOLUME_HEADER      *FvHeader;
+  EFI_FIRMWARE_VOLUME_EXT_HEADER  *FvExtHeader;
+
+  if (FvBase >= MAX_ADDRESS) {
+    return NULL;
+  }
+
+  if (FvLength >= MAX_ADDRESS - FvBase) {
+    return NULL;
+  }
+
+  if (FvLength < sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
+    return NULL;
+  }
+
+  FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
+  if (FvHeader->ExtHeaderOffset < sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
+    return NULL;
+  }
+
+  if (FvHeader->ExtHeaderOffset + sizeof (EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
+    return NULL;
+  }
+
+  FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase + FvHeader->ExtHeaderOffset);
+
+  return &FvExtHeader->FvName;
+}
+
 /**
   Build the GuidHob for tdx measurements which were done in SEC phase.
   The measurement values are stored in WorkArea.
@@ -128,6 +178,10 @@ InternalBuildGuidHobForTdxMeasurement (
   OVMF_WORK_AREA               *WorkArea;
   VOID                         *TdHobList;
   TDX_HANDOFF_TABLE_POINTERS2  HandoffTables;
+  VOID                         *FvName;
+  CFV_HANDOFF_TABLE_POINTERS2  FvBlob2;
+  EFI_PHYSICAL_ADDRESS         FvBase;
+  UINT64                       FvLength;
   UINT8                        *HashValue;
 
   if (!TdIsEnabled ()) {
@@ -169,5 +223,37 @@ InternalBuildGuidHobForTdxMeasurement (
     return Status;
   }
 
+  //
+  // Build the GuidHob for Cfv measurement
+  //
+  if (WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.MeasurementsBitmap & TDX_MEASUREMENT_CFVIMG_BITMASK) {
+    HashValue                   = WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.CfvImgHashValue;
+    FvBase                      = (UINT64)PcdGet32 (PcdOvmfFlashNvStorageVariableBase);
+    FvLength                    = (UINT64)PcdGet32 (PcdCfvRawDataSize);
+    FvBlob2.BlobDescriptionSize = sizeof (FvBlob2.BlobDescription);
+    CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC, sizeof (FvBlob2.BlobDescription));
+    FvName = GetFvName (FvBase, FvLength);
+    if (FvName != NULL) {
+      AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription, sizeof (FvBlob2.BlobDescription), "Fv(%g)", FvName);
+    }
+
+    FvBlob2.BlobBase   = FvBase;
+    FvBlob2.BlobLength = FvLength;
+
+    Status = BuildTdxMeasurementGuidHob (
+               0,                              // RtmrIndex
+               EV_EFI_PLATFORM_FIRMWARE_BLOB2, // EventType
+               (VOID *)&FvBlob2,               // EventData
+               sizeof (FvBlob2),               // EventSize
+               HashValue,                      // HashValue
+               SHA384_DIGEST_SIZE              // HashSize
+               );
+  }
+
+  if (EFI_ERROR (Status)) {
+    ASSERT (FALSE);
+    return Status;
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
deleted file mode 100644
index ae0ffcc95da5..000000000000
--- a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/** @file
-  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <PiPei.h>
-#include <Library/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/DebugLib.h>
-#include <IndustryStandard/Tpm20.h>
-#include <IndustryStandard/UefiTcgPlatform.h>
-#include <Library/HobLib.h>
-#include <Library/PrintLib.h>
-#include <Library/TcgEventLogRecordLib.h>
-#include <Library/TpmMeasurementLib.h>
-
-#include "PeilessStartupInternal.h"
-
-#define FV_HANDOFF_TABLE_DESC  "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
-typedef PLATFORM_FIRMWARE_BLOB2_STRUCT CFV_HANDOFF_TABLE_POINTERS2;
-
-/**
-  Get the FvName from the FV header.
-
-  Causion: The FV is untrusted input.
-
-  @param[in]  FvBase            Base address of FV image.
-  @param[in]  FvLength          Length of FV image.
-
-  @return FvName pointer
-  @retval NULL   FvName is NOT found
-**/
-VOID *
-GetFvName (
-  IN EFI_PHYSICAL_ADDRESS  FvBase,
-  IN UINT64                FvLength
-  )
-{
-  EFI_FIRMWARE_VOLUME_HEADER      *FvHeader;
-  EFI_FIRMWARE_VOLUME_EXT_HEADER  *FvExtHeader;
-
-  if (FvBase >= MAX_ADDRESS) {
-    return NULL;
-  }
-
-  if (FvLength >= MAX_ADDRESS - FvBase) {
-    return NULL;
-  }
-
-  if (FvLength < sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
-    return NULL;
-  }
-
-  FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
-  if (FvHeader->ExtHeaderOffset < sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
-    return NULL;
-  }
-
-  if (FvHeader->ExtHeaderOffset + sizeof (EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
-    return NULL;
-  }
-
-  FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase + FvHeader->ExtHeaderOffset);
-
-  return &FvExtHeader->FvName;
-}
-
-/**
-  Measure FV image.
-
-  @param[in]  FvBase            Base address of FV image.
-  @param[in]  FvLength          Length of FV image.
-  @param[in]  PcrIndex          Index of PCR
-
-  @retval EFI_SUCCESS           Fv image is measured successfully
-                                or it has been already measured.
-  @retval EFI_OUT_OF_RESOURCES  No enough memory to log the new event.
-  @retval EFI_DEVICE_ERROR      The command was unsuccessful.
-
-**/
-EFI_STATUS
-EFIAPI
-MeasureFvImage (
-  IN EFI_PHYSICAL_ADDRESS  FvBase,
-  IN UINT64                FvLength,
-  IN UINT8                 PcrIndex
-  )
-{
-  EFI_STATUS                   Status;
-  CFV_HANDOFF_TABLE_POINTERS2  FvBlob2;
-  VOID                         *FvName;
-
-  //
-  // Init the log event for FV measurement
-  //
-  FvBlob2.BlobDescriptionSize = sizeof (FvBlob2.BlobDescription);
-  CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC, sizeof (FvBlob2.BlobDescription));
-  FvName = GetFvName (FvBase, FvLength);
-  if (FvName != NULL) {
-    AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription, sizeof (FvBlob2.BlobDescription), "Fv(%g)", FvName);
-  }
-
-  FvBlob2.BlobBase   = FvBase;
-  FvBlob2.BlobLength = FvLength;
-
-  Status = TpmMeasureAndLogData (
-             1,                              // PCRIndex
-             EV_EFI_PLATFORM_FIRMWARE_BLOB2, // EventType
-             (VOID *)&FvBlob2,               // EventData
-             sizeof (FvBlob2),               // EventSize
-             (UINT8 *)(UINTN)FvBase,         // HashData
-             (UINTN)(FvLength)               // HashDataLen
-             );
-
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "The FV which failed to be measured starts at: 0x%x\n", FvBase));
-    ASSERT (FALSE);
-  }
-
-  return Status;
-}
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
index 4efbc14d5921..79d3a178a65f 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
@@ -140,13 +140,11 @@ PeilessStartup (
   UINT32                      DxeCodeSize;
   TD_RETURN_DATA              TdReturnData;
   VOID                        *VmmHobList;
-  UINT8                       *CfvBase;
 
   Status      = EFI_SUCCESS;
   BootFv      = NULL;
   VmmHobList  = NULL;
   SecCoreData = (EFI_SEC_PEI_HAND_OFF *)Context;
-  CfvBase     = (UINT8 *)(UINTN)FixedPcdGet32 (PcdCfvBase);
 
   ZeroMem (&PlatformInfoHob, sizeof (PlatformInfoHob));
 
@@ -186,6 +184,15 @@ PeilessStartup (
       CpuDeadLoop ();
     }
 
+    //
+    // Measure Tdx CFV
+    //
+    Status = TdxHelperMeasureCfvImage ();
+    if (EFI_ERROR (Status)) {
+      ASSERT (FALSE);
+      CpuDeadLoop ();
+    }
+
     //
     // Build GuidHob for tdx measurement
     //
@@ -194,15 +201,6 @@ PeilessStartup (
       ASSERT (FALSE);
       CpuDeadLoop ();
     }
-
-    //
-    // Measure Tdx CFV
-    //
-    Status = MeasureFvImage ((EFI_PHYSICAL_ADDRESS)(UINTN)CfvBase, FixedPcdGet32 (PcdCfvRawDataSize), 1);
-    if (EFI_ERROR (Status)) {
-      ASSERT (FALSE);
-      CpuDeadLoop ();
-    }
   }
 
   //
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
index a2d2c1c9145b..158196271962 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
@@ -58,25 +58,4 @@ EFIAPI
 ConstructSecHobList (
   );
 
-/**
-  Measure FV image.
-
-  @param[in]  FvBase            Base address of FV image.
-  @param[in]  FvLength          Length of FV image.
-  @param[in]  PcrIndex          Index of PCR
-
-  @retval EFI_SUCCESS           Fv image is measured successfully
-                                or it has been already measured.
-  @retval EFI_OUT_OF_RESOURCES  No enough memory to log the new event.
-  @retval EFI_DEVICE_ERROR      The command was unsuccessful.
-
-**/
-EFI_STATUS
-EFIAPI
-MeasureFvImage (
-  IN EFI_PHYSICAL_ADDRESS  FvBase,
-  IN UINT64                FvLength,
-  IN UINT8                 PcrIndex
-  );
-
 #endif
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
index 5c6eb1597bea..4ced5dda9945 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
@@ -29,7 +29,6 @@
   PeilessStartup.c
   Hob.c
   DxeLoad.c
-  IntelTdx.c
   X64/VirtualMemory.c
 
 [Packages]
@@ -70,9 +69,6 @@
   gEfiNonCcFvGuid
 
 [Pcd]
-  gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase
-  gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset
-  gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize
   gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataOffset
   gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataSize
-- 
2.29.2.windows.2


  parent reply	other threads:[~2023-02-03  3:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-03  3:31 [PATCH V6 00/12] Enable Tdx measurement in OvmfPkgX64 Min Xu
2023-02-03  3:31 ` [PATCH V6 01/12] OvmfPkg: Add Tdx measurement data structure in WorkArea Min Xu
2023-02-03  3:31 ` [PATCH V6 02/12] OvmfPkg/IntelTdx: Add TdxHelperLibNull Min Xu
2023-02-03  3:31 ` [PATCH V6 03/12] OvmfPkg/IntelTdx: Add SecTdxHelperLib Min Xu
2023-02-03  3:31 ` [PATCH V6 04/12] OvmfPkg/PeilessStartupLib: Update the define of FV_HANDOFF_TABLE_POINTERS2 Min Xu
2023-02-03  3:31 ` [PATCH V6 05/12] OvmfPkg: Refactor MeasureHobList Min Xu
2023-02-03  3:31 ` Min Xu [this message]
2023-02-03  3:31 ` [PATCH V6 07/12] OvmfPkg: Refactor ProcessHobList Min Xu
2023-02-03  3:31 ` [PATCH V6 08/12] OvmfPkg/IntelTdx: Measure TdHob and Configuration FV in SecMain Min Xu
2023-02-03  8:51   ` Gerd Hoffmann
2023-02-03  3:31 ` [PATCH V6 09/12] OvmfPkg/IntelTdx: Add PeiTdxHelperLib Min Xu
2023-02-03  3:31 ` [PATCH V6 10/12] OvmfPkg/OvmfPkgX64: Measure TdHob and Configuration FV in SecMain Min Xu
2023-02-03  3:31 ` [PATCH V6 11/12] OvmfPkg/PlatformPei: Build GuidHob for Tdx measurement Min Xu
2023-02-03  3:31 ` [PATCH V6 12/12] OvmfPkg: Support Tdx measurement in OvmfPkgX64 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=20230203033147.1332-7-min.m.xu@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