From: Min M Xu <min.m.xu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243
2 new functions are added in PeilessStartupLib/IntelTdx.c.
- BuildTdxMeasurementGuidHob
- InternalBuildGuidHobForTdxMeasurement
These 2 functions build GuidHob for Tdx measurement. These 2 functions
are to be moved to TdxHelperLib in the following patch. That is to make
the code more reviewable.
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>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/Library/PeilessStartupLib/IntelTdx.c | 180 ++++++++++++++++++
.../PeilessStartupLib/PeilessStartupLib.inf | 2 +
2 files changed, 182 insertions(+)
diff --git a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
index 4e8dca3d7712..8c2a031ee9c7 100644
--- a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
+++ b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
@@ -13,6 +13,7 @@
#include <Library/PrintLib.h>
#include <Library/TcgEventLogRecordLib.h>
#include <Library/TpmMeasurementLib.h>
+#include <WorkArea.h>
#include "PeilessStartupInternal.h"
@@ -90,6 +91,89 @@ MeasureHobList (
return Status;
}
+/**
+ * Build GuidHob for Tdx measurement.
+ *
+ * Tdx measurement includes the measurement of TdHob and CFV. They're measured
+ * and extended to RTMR registers in SEC phase. Because at that moment the Hob
+ * service are not available. So the values of the measurement are saved in
+ * workarea and will be built into GuidHob after the Hob service is ready.
+ *
+ * @param RtmrIndex RTMR index
+ * @param EventType Event type
+ * @param EventData Event data
+ * @param EventSize Size of event data
+ * @param HashValue Hash value
+ * @param HashSize Size of hash
+ *
+ * @retval EFI_SUCCESS Successfully build the GuidHobs
+ * @retval Others Other error as indicated
+ */
+STATIC
+EFI_STATUS
+BuildTdxMeasurementGuidHob (
+ UINT32 RtmrIndex,
+ UINT32 EventType,
+ UINT8 *EventData,
+ UINT32 EventSize,
+ UINT8 *HashValue,
+ UINT32 HashSize
+ )
+{
+ VOID *EventHobData;
+ UINT8 *Ptr;
+ TPML_DIGEST_VALUES *TdxDigest;
+
+ if (HashSize != SHA384_DIGEST_SIZE) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ #define TDX_DIGEST_VALUE_LEN (sizeof (UINT32) + sizeof (TPMI_ALG_HASH) + SHA384_DIGEST_SIZE)
+
+ EventHobData = BuildGuidHob (
+ &gCcEventEntryHobGuid,
+ sizeof (TCG_PCRINDEX) + sizeof (TCG_EVENTTYPE) +
+ TDX_DIGEST_VALUE_LEN +
+ sizeof (UINT32) + EventSize
+ );
+
+ if (EventHobData == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Ptr = (UINT8 *)EventHobData;
+
+ //
+ // There are 2 types of measurement registers in TDX: MRTD and RTMR[0-3].
+ // According to UEFI Spec 2.10 Section 38.4.1, RTMR[0-3] is mapped to MrIndex[1-4].
+ // So RtmrIndex must be increased by 1 before the event log is created.
+ //
+ RtmrIndex++;
+ CopyMem (Ptr, &RtmrIndex, sizeof (UINT32));
+ Ptr += sizeof (UINT32);
+
+ CopyMem (Ptr, &EventType, sizeof (TCG_EVENTTYPE));
+ Ptr += sizeof (TCG_EVENTTYPE);
+
+ TdxDigest = (TPML_DIGEST_VALUES *)Ptr;
+ TdxDigest->count = 1;
+ TdxDigest->digests[0].hashAlg = TPM_ALG_SHA384;
+ CopyMem (
+ TdxDigest->digests[0].digest.sha384,
+ HashValue,
+ SHA384_DIGEST_SIZE
+ );
+ Ptr += TDX_DIGEST_VALUE_LEN;
+
+ CopyMem (Ptr, &EventSize, sizeof (UINT32));
+ Ptr += sizeof (UINT32);
+
+ CopyMem (Ptr, (VOID *)EventData, EventSize);
+ Ptr += EventSize;
+
+ return EFI_SUCCESS;
+}
+
/**
Get the FvName from the FV header.
@@ -136,6 +220,102 @@ GetFvName (
return &FvExtHeader->FvName;
}
+/**
+ Build the GuidHob for tdx measurements which were done in SEC phase.
+ The measurement values are stored in WorkArea.
+
+ @retval EFI_SUCCESS The GuidHob is built successfully
+ @retval Others Other errors as indicated
+**/
+EFI_STATUS
+InternalBuildGuidHobForTdxMeasurement (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ 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 ()) {
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+ }
+
+ WorkArea = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+ if (WorkArea == NULL) {
+ return EFI_ABORTED;
+ }
+
+ Status = EFI_SUCCESS;
+
+ //
+ // Build the GuidHob for TdHob measurement
+ //
+ TdHobList = (VOID *)(UINTN)FixedPcdGet32 (PcdOvmfSecGhcbBase);
+ if (WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.MeasurementsBitmap & TDX_MEASUREMENT_TDHOB_BITMASK) {
+ HashValue = WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.TdHobHashValue;
+ HandoffTables.TableDescriptionSize = sizeof (HandoffTables.TableDescription);
+ CopyMem (HandoffTables.TableDescription, HANDOFF_TABLE_DESC, sizeof (HandoffTables.TableDescription));
+ HandoffTables.NumberOfTables = 1;
+ CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), &gUefiOvmfPkgTokenSpaceGuid);
+ HandoffTables.TableEntry[0].VendorTable = TdHobList;
+
+ Status = BuildTdxMeasurementGuidHob (
+ 0, // RtmrIndex
+ EV_EFI_HANDOFF_TABLES2, // EventType
+ (UINT8 *)(UINTN)&HandoffTables, // EventData
+ sizeof (HandoffTables), // EventSize
+ HashValue, // HashValue
+ SHA384_DIGEST_SIZE // HashSize
+ );
+ }
+
+ if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ 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;
+}
+
/**
Measure FV image.
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
index 5c6eb1597bea..f9012ccd68d0 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
@@ -89,3 +89,5 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
--
2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99113): https://edk2.groups.io/g/devel/message/99113
Mute This Topic: https://groups.io/mt/96556337/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243
>
> 2 new functions are added in PeilessStartupLib/IntelTdx.c.
> - BuildTdxMeasurementGuidHob
> - InternalBuildGuidHobForTdxMeasurement
>
> These 2 functions build GuidHob for Tdx measurement.
But you don't use them anywhere? The point of splitting the patches is
not only to simplify review, but also to simplify testing (and in case a
bug shows up later finding it with bisecting).
So, current state of the code is:
There are MeasureHobList() + MeasureFvImage(), doing measurement and
logging in one go, using TpmMeasureAndLogData(). Problem is this
doesn't work in SEC, so you want split.
So, you add TdxHelperMeasureTdHob (doing the measurement part of
MeasureHobList) and TdxHelperMeasureCfvImage (likewise doing the
measurement part of MeasureFvImage) and logging both is handled by
TdxHelperBuildGuidHobForTdxMeasurement().
So I think the series should have:
(1) One or more patches doing cleanups (like reusing the struct).
(2) A patch removing MeasureHobList and adding TdxHelperMeasureTdHob
with the first half of TdxHelperBuildGuidHobForTdxMeasurement
(3) A patch removing MeasureFvImage and adding TdxHelperMeasureCfvImage
with the second half of TdxHelperBuildGuidHobForTdxMeasurement
(4) A patch moving the code from PlatformInitLib to TdxHelperLib.
(5) A patch moving the calls to TdxHelperMeasureTdHob and
TdxHelperMeasureCfvImage to SEC.
(6) A patch adding the Tdxhelper* calls to OvmfPkgX64.
What is the reason to create a new TdxHelperLib btw.?
Are there any problems with the code being in PlatformInitLib?
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99125): https://edk2.groups.io/g/devel/message/99125
Mute This Topic: https://groups.io/mt/96556337/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On January 27, 2023 3:54 PM, Gerd Hoffmann wrote: > On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote: > > From: Min M Xu <min.m.xu@intel.com> > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243 > > > > 2 new functions are added in PeilessStartupLib/IntelTdx.c. > > - BuildTdxMeasurementGuidHob > > - InternalBuildGuidHobForTdxMeasurement > > > > These 2 functions build GuidHob for Tdx measurement. > > But you don't use them anywhere? The point of splitting the patches is not > only to simplify review, but also to simplify testing (and in case a bug shows > up later finding it with bisecting). > > So, current state of the code is: > > There are MeasureHobList() + MeasureFvImage(), doing measurement and > logging in one go, using TpmMeasureAndLogData(). Problem is this doesn't > work in SEC, so you want split. > > So, you add TdxHelperMeasureTdHob (doing the measurement part of > MeasureHobList) and TdxHelperMeasureCfvImage (likewise doing the > measurement part of MeasureFvImage) and logging both is handled by > TdxHelperBuildGuidHobForTdxMeasurement(). > > So I think the series should have: > > (1) One or more patches doing cleanups (like reusing the struct). > (2) A patch removing MeasureHobList and adding TdxHelperMeasureTdHob > with the first half of TdxHelperBuildGuidHobForTdxMeasurement > (3) A patch removing MeasureFvImage and adding > TdxHelperMeasureCfvImage > with the second half of TdxHelperBuildGuidHobForTdxMeasurement > (4) A patch moving the code from PlatformInitLib to TdxHelperLib. > (5) A patch moving the calls to TdxHelperMeasureTdHob and > TdxHelperMeasureCfvImage to SEC. > (6) A patch adding the Tdxhelper* calls to OvmfPkgX64. Thanks for the suggestion. The patches will be re-organized in the next version. Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99209): https://edk2.groups.io/g/devel/message/99209 Mute This Topic: https://groups.io/mt/96556337/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On January 27, 2023 3:54 PM, Gerd Hoffmann wrote: > On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote: > > From: Min M Xu <min.m.xu@intel.com> > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243 > > > What is the reason to create a new TdxHelperLib btw.? > Are there any problems with the code being in PlatformInitLib? > When tdx-measurement is being enabled in OvmfPkgX64, we find below functions are needed. - ProcessTdHob - MeasureTdHob - MeasureCfvImage - BuildGuidHobForTdxMeasurement The first one was implemented in PlatformInitLib. The others were implemented in PeilessStartupLib. These 4 functions should be implemented in one lib so that they could be called in both OvmfPkgX64 and IntelTdxX64. So there are below 2 options 1) Implement all these 4 functions in PlatformInitLib 2) Implement all these 4 functions in a new TdxHelperLib We choose option-2 (a new TdxHelperLib). 1. TdxHelperLib contains all the tdx specific helper functions as the lib name indicates. 2. We can avoid PlatformInitLib getting bigger and bigger by adding more and more functions. (these functions can be implemented in a separate lib) 3. Furthermore, PlatformTdxPublishRamRegions in PlatformInitLib can be moved to TdxHelperLib as well (we will submit a separate patch-set later). So that we can have a general-purpose PlatformInitLib. Based on above consideration, we create a new TdxHelperLib. Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99153): https://edk2.groups.io/g/devel/message/99153 Mute This Topic: https://groups.io/mt/96556337/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Fri, Jan 27, 2023 at 11:30:46AM +0000, Xu, Min M wrote: > On January 27, 2023 3:54 PM, Gerd Hoffmann wrote: > > On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote: > > > From: Min M Xu <min.m.xu@intel.com> > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243 > > > > > What is the reason to create a new TdxHelperLib btw.? > > Are there any problems with the code being in PlatformInitLib? > > > When tdx-measurement is being enabled in OvmfPkgX64, we find below functions are needed. > - ProcessTdHob > - MeasureTdHob > - MeasureCfvImage > - BuildGuidHobForTdxMeasurement > > The first one was implemented in PlatformInitLib. The others were implemented in PeilessStartupLib. These 4 functions should be implemented in one lib so that they could be called in both OvmfPkgX64 and IntelTdxX64. > So there are below 2 options > 1) Implement all these 4 functions in PlatformInitLib > 2) Implement all these 4 functions in a new TdxHelperLib > > We choose option-2 (a new TdxHelperLib). > 1. TdxHelperLib contains all the tdx specific helper functions as the lib name indicates. > 2. We can avoid PlatformInitLib getting bigger and bigger by adding more and more functions. (these functions can be implemented in a separate lib) > 3. Furthermore, PlatformTdxPublishRamRegions in PlatformInitLib can be moved to TdxHelperLib as well (we will submit a separate patch-set later). So that we can have a general-purpose PlatformInitLib. > > Based on above consideration, we create a new TdxHelperLib. Ok, makes sense to me. thanks, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99154): https://edk2.groups.io/g/devel/message/99154 Mute This Topic: https://groups.io/mt/96556337/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.