Date
1 - 5 of 5
[PATCH V4 06/12] OvmfPkg/PeilessStartupLib: Build GuidHob for Tdx measurement
|
Min Xu
From: Min M Xu <min.m.xu@...>
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@...> Cc: James Bottomley <jejb@...> Cc: Jiewen Yao <jiewen.yao@...> Cc: Gerd Hoffmann <kraxel@...> Cc: Tom Lendacky <thomas.lendacky@...> Cc: Michael Roth <michael.roth@...> Signed-off-by: Min Xu <min.m.xu@...> --- 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 |
|
|
|
Gerd Hoffmann
On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:
From: Min M Xu <min.m.xu@...>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 |
|
|
|
Min Xu
On January 27, 2023 3:54 PM, Gerd Hoffmann wrote:
On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:When tdx-measurement is being enabled in OvmfPkgX64, we find below functions are needed.From: Min M Xu <min.m.xu@...>What is the reason to create a new TdxHelperLib btw.? - 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 |
|
|
|
Gerd Hoffmann
On Fri, Jan 27, 2023 at 11:30:46AM +0000, Xu, Min M wrote:
On January 27, 2023 3:54 PM, Gerd Hoffmann wrote:Ok, makes sense to me.On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:When tdx-measurement is being enabled in OvmfPkgX64, we find below functions are needed.From: Min M Xu <min.m.xu@...>What is the reason to create a new TdxHelperLib btw.? thanks, Gerd |
|
|
|
Min Xu
On January 27, 2023 3:54 PM, Gerd Hoffmann wrote:
On Fri, Jan 27, 2023 at 08:11:00AM +0800, Min Xu wrote:Thanks for the suggestion. The patches will be re-organized in the next version.From: Min M Xu <min.m.xu@...>But you don't use them anywhere? The point of splitting the patches is not Thanks Min |
|
|