Date   

[PATCH V5 12/13] OvmfPkg/PlatformPei: Build GuidHob for Tdx measurement

Min Xu
 

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

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

TdxHelperBuildGuidHobForTdxMeasurement is called in PlatformPei to build
GuidHob for Tdx measurement.

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@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 ++++-
OvmfPkg/CloudHv/CloudHvX64.dsc | 5 ++++-
OvmfPkg/Microvm/MicrovmX64.dsc | 5 ++++-
OvmfPkg/OvmfPkgX64.dsc | 5 ++++-
OvmfPkg/PlatformPei/IntelTdx.c | 3 +++
5 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 36100f5fdc11..1cebd6b4bcc2 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -570,7 +570,10 @@
}
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

- OvmfPkg/PlatformPei/PlatformPei.inf
+ OvmfPkg/PlatformPei/PlatformPei.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
+ }
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
UefiCpuPkg/CpuMpPei/CpuMpPei.inf
OvmfPkg/AmdSev/SecretPei/SecretPei.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 7326417eab62..fc5e73158a71 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -678,7 +678,10 @@
}
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

- OvmfPkg/PlatformPei/PlatformPei.inf
+ OvmfPkg/PlatformPei/PlatformPei.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
+ }
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
<LibraryClasses>
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 2d53b5c2950d..1161e1f39bf2 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -679,7 +679,10 @@
}
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

- OvmfPkg/PlatformPei/PlatformPei.inf
+ OvmfPkg/PlatformPei/PlatformPei.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
+ }
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
UefiCpuPkg/CpuMpPei/CpuMpPei.inf

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d87013a4422c..a13299c18cfd 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -746,7 +746,10 @@
}
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

- OvmfPkg/PlatformPei/PlatformPei.inf
+ OvmfPkg/PlatformPei/PlatformPei.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf
+ }
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
<LibraryClasses>
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/PlatformPei/IntelTdx.c b/OvmfPkg/PlatformPei/IntelTdx.c
index 3c1ddbfafd80..3d625cabd844 100644
--- a/OvmfPkg/PlatformPei/IntelTdx.c
+++ b/OvmfPkg/PlatformPei/IntelTdx.c
@@ -18,6 +18,7 @@
#include <Library/QemuFwCfgLib.h>
#include <Library/PeiServicesLib.h>
#include <Library/TdxLib.h>
+#include <Library/TdxHelperLib.h>
#include <Library/PlatformInitLib.h>
#include <WorkArea.h>
#include <ConfidentialComputingGuestAttr.h>
@@ -39,6 +40,8 @@ IntelTdxInitialize (
return;
}

+ TdxHelperBuildGuidHobForTdxMeasurement ();
+
PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrIntelTdx);
ASSERT_RETURN_ERROR (PcdStatus);

--
2.29.2.windows.2


[PATCH V5 11/13] OvmfPkg/OvmfPkgX64: Measure TdHob and Configuration FV in SecMain

Min Xu
 

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

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

TdHob and Configuration FV (Cfv) are external inputs from VMM. From the
security perspective, they should be measured before they're consumed.
This patch measures TdHob and Cfv and stores the measurement values in
WorkArea.

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/Sec/SecMain.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index a27dc9406b70..4bb3b641701e 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -760,6 +760,19 @@ SecCoreStartupWithStack (

#if defined (TDX_GUEST_SUPPORTED)
if (CcProbe () == CcGuestTypeIntelTdx) {
+ //
+ // From the security perspective all the external input should be measured before
+ // it is consumed. TdHob and Configuration FV (Cfv) image are passed from VMM
+ // and should be measured here.
+ //
+ if (EFI_ERROR (TdxHelperMeasureTdHob ())) {
+ CpuDeadLoop ();
+ }
+
+ if (EFI_ERROR (TdxHelperMeasureCfvImage ())) {
+ CpuDeadLoop ();
+ }
+
//
// For Td guests, the memory map info is in TdHobLib. It should be processed
// first so that the memory is accepted. Otherwise access to the unaccepted
--
2.29.2.windows.2


[PATCH V5 10/13] OvmfPkg/IntelTdx: Add PeiTdxHelperLib

Min Xu
 

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

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

TdxHelperLib provides below helper functions for a td-guest.
- TdxHelperProcessTdHob
- TdxHelperMeasureTdHob
- TdxHelperMeasureCfvImage
- TdxHelperBuildGuidHobForTdxMeasurement

PeiTdxHelperLib is the PEI instance of TdxHelperLib. It implements 1
function for tdx in PEI phase. Other functions are not supported in
PEI phase.
- TdxHelperBuildGuidHobForTdxMeasurement builds GuidHob for tdx
measurement in PEI phase.

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@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c | 91 +++++++++++++++++++
.../IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf | 48 ++++++++++
2 files changed, 139 insertions(+)
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf

diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c
new file mode 100644
index 000000000000..91ab53ed14ad
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c
@@ -0,0 +1,91 @@
+/** @file
+ TdxHelper Functions which are used in PEI phase
+
+ Copyright (c) 2022 - 2023, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <PiPei.h>
+
+/**
+ 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
+ );
+
+/**
+ In Tdx guest, some information need to be passed from host VMM to guest
+ firmware. For example, the memory resource, etc. These information are
+ prepared by host VMM and put in TdHob which is described in TdxMetadata.
+ TDVF processes the TdHob to accept memories.
+
+ @retval EFI_SUCCESS Successfully process the TdHob
+ @retval Others Other error as indicated
+**/
+EFI_STATUS
+EFIAPI
+TdxHelperProcessTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ In Tdx guest, TdHob is passed from host VMM to guest firmware and it contains
+ the information of the memory resource. From the security perspective before
+ it is consumed, it should be measured and extended.
+ *
+ * @retval EFI_SUCCESS Successfully measure the TdHob
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * In Tdx guest, Configuration FV (CFV) is treated as external input because it
+ * may contain the data provided by VMM. From the sucurity perspective Cfv image
+ * should be measured before it is consumed.
+ *
+ * @retval EFI_SUCCESS Successfully measure the CFV image
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureCfvImage (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ 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
+EFIAPI
+TdxHelperBuildGuidHobForTdxMeasurement (
+ VOID
+ )
+{
+ return InternalBuildGuidHobForTdxMeasurement ();
+}
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf b/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf
new file mode 100644
index 000000000000..ad3b6c1da62b
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf
@@ -0,0 +1,48 @@
+## @file
+# TdxHelperLib PEI instance
+#
+# This module provides Tdx helper functions in PEI phase.
+# Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = PeiTdxHelperLib
+ FILE_GUID = 4d22289d-3bde-4501-a737-7719f3215065
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxHelperLib|PEIM
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = X64
+#
+
+[Sources]
+ PeiTdxHelper.c
+ TdxMeasurementHob.c
+
+[Packages]
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+ SecurityPkg/SecurityPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ HobLib
+ PcdLib
+
+[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize
+
+[Guids]
+ gCcEventEntryHobGuid
--
2.29.2.windows.2


[PATCH V5 09/13] OvmfPkg/PeilessStartupLib: Delete the duplicated tdx measurement

Min Xu
 

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

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

After TdHob and Configuration FV (Cfv) are measured in SecMain, the
same measurements in PeilessStartupLib should be deleted.

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/IntelTdx/IntelTdxX64.dsc | 3 ---
.../PeilessStartupLib/PeilessStartup.c | 20 +------------------
.../PeilessStartupLib/PeilessStartupLib.inf | 2 --
3 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 920f1c6080d4..41de2e942817 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -548,11 +548,8 @@
OvmfPkg/IntelTdx/Sec/SecMain.inf {
<LibraryClasses>
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
- TpmMeasurementLib|SecurityPkg/Library/SecTpmMeasurementLib/SecTpmMeasurementLibTdx.inf
NULL|OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SecCryptLib.inf
- HashLib|SecurityPkg/Library/HashLibTdx/HashLibTdx.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
}

#
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
index 79d3a178a65f..164aa2d61911 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
@@ -176,25 +176,7 @@ PeilessStartup (

if (TdIsEnabled ()) {
//
- // Measure HobList
- //
- Status = TdxHelperMeasureTdHob ();
- if (EFI_ERROR (Status)) {
- ASSERT (FALSE);
- CpuDeadLoop ();
- }
-
- //
- // Measure Tdx CFV
- //
- Status = TdxHelperMeasureCfvImage ();
- if (EFI_ERROR (Status)) {
- ASSERT (FALSE);
- CpuDeadLoop ();
- }
-
- //
- // Build GuidHob for tdx measurement
+ // Build GuidHob for the tdx measurements which were done in SEC phase.
//
Status = TdxHelperBuildGuidHobForTdxMeasurement ();
if (EFI_ERROR (Status)) {
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
index 4ced5dda9945..e77ad7bc921e 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
@@ -56,8 +56,6 @@
PrePiLib
QemuFwCfgLib
PlatformInitLib
- HashLib
- TpmMeasurementLib

[Guids]
gEfiHobMemoryAllocModuleGuid
--
2.29.2.windows.2


[PATCH V5 08/13] OvmfPkg/IntelTdx: Measure TdHob and Configuration FV in SecMain

Min Xu
 

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

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

TdHob and Configuration FV (Cfv) are external inputs from VMM. From the
security perspective, they should be measured before they're consumed.
This patch measures TdHob and Cfv and stores the measurement values in
WorkArea.

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/IntelTdx/Sec/SecMain.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/OvmfPkg/IntelTdx/Sec/SecMain.c b/OvmfPkg/IntelTdx/Sec/SecMain.c
index 41bd5c66ba29..ccb217b709a0 100644
--- a/OvmfPkg/IntelTdx/Sec/SecMain.c
+++ b/OvmfPkg/IntelTdx/Sec/SecMain.c
@@ -62,6 +62,19 @@ SecCoreStartupWithStack (
volatile UINT8 *Table;

if (CcProbe () == CcGuestTypeIntelTdx) {
+ //
+ // From the security perspective all the external input should be measured before
+ // it is consumed. TdHob and Configuration FV (Cfv) image are passed from VMM
+ // and should be measured here.
+ //
+ if (EFI_ERROR (TdxHelperMeasureTdHob ())) {
+ CpuDeadLoop ();
+ }
+
+ if (EFI_ERROR (TdxHelperMeasureCfvImage ())) {
+ CpuDeadLoop ();
+ }
+
//
// For Td guests, the memory map info is in TdHobLib. It should be processed
// first so that the memory is accepted. Otherwise access to the unaccepted
--
2.29.2.windows.2


[PATCH V5 07/13] OvmfPkg: Refactor ProcessHobList

Min Xu
 

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

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

ProcessHobList once was implemented in PlatformInitLib and it walks thru
TdHob list and accept un-accepted memories.

This patch moves the codes to SecTdxHelperLib and rename ProcessHobList
as TdxHelperProcessTdHob

After TdxHelperProcessTdHob is introduced, below changes are applied:
- Call TdxHelperProcessTdHob instead of ProcessHobList in SecMain.c
(in both OvmfPkgX64/Sec and IntelTdx/Sec).
- Delete the duplicated codes in PlatformInitLib

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/Include/Library/PlatformInitLib.h | 17 -
OvmfPkg/IntelTdx/Sec/SecMain.c | 4 +-
OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c | 754 ++++++++++++++++-
OvmfPkg/Library/PlatformInitLib/IntelTdx.c | 768 ------------------
.../Library/PlatformInitLib/IntelTdxNull.c | 20 -
.../PlatformInitLib/PlatformInitLib.inf | 1 -
OvmfPkg/OvmfPkgX64.dsc | 3 +-
OvmfPkg/Sec/SecMain.c | 4 +-
8 files changed, 759 insertions(+), 812 deletions(-)

diff --git a/OvmfPkg/Include/Library/PlatformInitLib.h b/OvmfPkg/Include/Library/PlatformInitLib.h
index 051b31191194..57b18b94d9b8 100644
--- a/OvmfPkg/Include/Library/PlatformInitLib.h
+++ b/OvmfPkg/Include/Library/PlatformInitLib.h
@@ -210,23 +210,6 @@ PlatformMaxCpuCountInitialization (
IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob
);

-/**
- In Tdx guest, some information need to be passed from host VMM to guest
- firmware. For example, the memory resource, etc. These information are
- prepared by host VMM and put in HobList which is described in TdxMetadata.
-
- Information in HobList is treated as external input. From the security
- perspective before it is consumed, it should be validated.
-
- @retval EFI_SUCCESS Successfully process the hoblist
- @retval Others Other error as indicated
-**/
-EFI_STATUS
-EFIAPI
-ProcessTdxHobList (
- VOID
- );
-
/**
In Tdx guest, the system memory is passed in TdHob by host VMM. So
the major task of PlatformTdxPublishRamRegions is to walk thru the
diff --git a/OvmfPkg/IntelTdx/Sec/SecMain.c b/OvmfPkg/IntelTdx/Sec/SecMain.c
index ab01ec9ab19c..41bd5c66ba29 100644
--- a/OvmfPkg/IntelTdx/Sec/SecMain.c
+++ b/OvmfPkg/IntelTdx/Sec/SecMain.c
@@ -24,7 +24,7 @@
#include <Library/LocalApicLib.h>
#include <Library/CpuExceptionHandlerLib.h>
#include <IndustryStandard/Tdx.h>
-#include <Library/PlatformInitLib.h>
+#include <Library/TdxHelperLib.h>
#include <Library/CcProbeLib.h>
#include <Library/PeilessStartupLib.h>

@@ -67,7 +67,7 @@ SecCoreStartupWithStack (
// first so that the memory is accepted. Otherwise access to the unaccepted
// memory will trigger tripple fault.
//
- if (ProcessTdxHobList () != EFI_SUCCESS) {
+ if (TdxHelperProcessTdHob () != EFI_SUCCESS) {
CpuDeadLoop ();
}
}
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
index 1929093f9110..3372cee2f720 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
@@ -17,11 +17,20 @@
#include <IndustryStandard/IntelTdx.h>
#include <IndustryStandard/Tpm20.h>
#include <Library/TdxLib.h>
+#include <Library/TdxMailboxLib.h>
+#include <Library/SynchronizationLib.h>
#include <Pi/PrePiHob.h>
#include <WorkArea.h>
#include <ConfidentialComputingGuestAttr.h>
#include <Library/TdxHelperLib.h>

+#define ALIGNED_2MB_MASK 0x1fffff
+#define MEGABYTE_SHIFT 20
+
+#define ACCEPT_CHUNK_SIZE SIZE_32MB
+#define AP_STACK_SIZE SIZE_16KB
+#define APS_STACK_SIZE(CpusNum) (ALIGN_VALUE(CpusNum*AP_STACK_SIZE, SIZE_2MB))
+
/**
Build the GuidHob for tdx measurements which were done in SEC phase.
The measurement values are stored in WorkArea.
@@ -34,6 +43,720 @@ InternalBuildGuidHobForTdxMeasurement (
VOID
);

+/**
+ This function will be called to accept pages. Only BSP accepts pages.
+
+ TDCALL(ACCEPT_PAGE) supports the accept page size of 4k and 2M. To
+ simplify the implementation, the Memory to be accpeted is splitted
+ into 3 parts:
+ ----------------- <-- StartAddress1 (not 2M aligned)
+ | part 1 | Length1 < 2M
+ |---------------| <-- StartAddress2 (2M aligned)
+ | | Length2 = Integer multiples of 2M
+ | part 2 |
+ | |
+ |---------------| <-- StartAddress3
+ | part 3 | Length3 < 2M
+ |---------------|
+
+ @param[in] PhysicalAddress Start physical adress
+ @param[in] PhysicalEnd End physical address
+
+ @retval EFI_SUCCESS Accept memory successfully
+ @retval Others Other errors as indicated
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BspAcceptMemoryResourceRange (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN EFI_PHYSICAL_ADDRESS PhysicalEnd
+ )
+{
+ EFI_STATUS Status;
+ UINT32 AcceptPageSize;
+ UINT64 StartAddress1;
+ UINT64 StartAddress2;
+ UINT64 StartAddress3;
+ UINT64 TotalLength;
+ UINT64 Length1;
+ UINT64 Length2;
+ UINT64 Length3;
+ UINT64 Pages;
+
+ AcceptPageSize = FixedPcdGet32 (PcdTdxAcceptPageSize);
+ TotalLength = PhysicalEnd - PhysicalAddress;
+ StartAddress1 = 0;
+ StartAddress2 = 0;
+ StartAddress3 = 0;
+ Length1 = 0;
+ Length2 = 0;
+ Length3 = 0;
+
+ if (TotalLength == 0) {
+ return EFI_SUCCESS;
+ }
+
+ if (ALIGN_VALUE (PhysicalAddress, SIZE_2MB) != PhysicalAddress) {
+ StartAddress1 = PhysicalAddress;
+ Length1 = ALIGN_VALUE (PhysicalAddress, SIZE_2MB) - PhysicalAddress;
+ if (Length1 >= TotalLength) {
+ Length1 = TotalLength;
+ }
+
+ PhysicalAddress += Length1;
+ TotalLength -= Length1;
+ }
+
+ if (TotalLength > SIZE_2MB) {
+ StartAddress2 = PhysicalAddress;
+ Length2 = TotalLength & ~(UINT64)ALIGNED_2MB_MASK;
+ PhysicalAddress += Length2;
+ TotalLength -= Length2;
+ }
+
+ if (TotalLength) {
+ StartAddress3 = PhysicalAddress;
+ Length3 = TotalLength;
+ }
+
+ Status = EFI_SUCCESS;
+ if (Length1 > 0) {
+ Pages = Length1 / SIZE_4KB;
+ Status = TdAcceptPages (StartAddress1, Pages, SIZE_4KB);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ if (Length2 > 0) {
+ Pages = Length2 / AcceptPageSize;
+ Status = TdAcceptPages (StartAddress2, Pages, AcceptPageSize);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ if (Length3 > 0) {
+ Pages = Length3 / SIZE_4KB;
+ Status = TdAcceptPages (StartAddress3, Pages, SIZE_4KB);
+ ASSERT (!EFI_ERROR (Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ return Status;
+}
+
+/**
+ * This function is called by BSP and APs to accept memory.
+ * Note:
+ * The input PhysicalStart/PhysicalEnd indicates the whole memory region
+ * to be accepted. BSP or AP only accepts one piece in the whole memory region.
+ *
+ * @param CpuIndex vCPU index
+ * @param CpusNum Total vCPU number of a Tdx guest
+ * @param PhysicalStart Start address of a memory region which is to be accepted
+ * @param PhysicalEnd End address of a memory region which is to be accepted
+ *
+ * @retval EFI_SUCCESS Successfully accept the memory
+ * @retval Other Other errors as indicated
+ */
+STATIC
+EFI_STATUS
+EFIAPI
+BspApAcceptMemoryResourceRange (
+ UINT32 CpuIndex,
+ UINT32 CpusNum,
+ EFI_PHYSICAL_ADDRESS PhysicalStart,
+ EFI_PHYSICAL_ADDRESS PhysicalEnd
+ )
+{
+ UINT64 Status;
+ UINT64 Pages;
+ UINT64 Stride;
+ UINT64 AcceptPageSize;
+ EFI_PHYSICAL_ADDRESS PhysicalAddress;
+
+ AcceptPageSize = (UINT64)(UINTN)FixedPcdGet32 (PcdTdxAcceptPageSize);
+
+ Status = EFI_SUCCESS;
+ Stride = (UINTN)CpusNum * ACCEPT_CHUNK_SIZE;
+ PhysicalAddress = PhysicalStart + ACCEPT_CHUNK_SIZE * (UINTN)CpuIndex;
+
+ while (!EFI_ERROR (Status) && PhysicalAddress < PhysicalEnd) {
+ Pages = MIN (ACCEPT_CHUNK_SIZE, PhysicalEnd - PhysicalAddress) / AcceptPageSize;
+ Status = TdAcceptPages (PhysicalAddress, Pages, (UINT32)(UINTN)AcceptPageSize);
+ ASSERT (!EFI_ERROR (Status));
+ PhysicalAddress += Stride;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ * This function is called by APs to accept memory.
+ *
+ * @param CpuIndex vCPU index of an AP
+ * @param PhysicalStart Start address of a memory region which is to be accepted
+ * @param PhysicalEnd End address of a memory region which is to be accepted
+ *
+ * @retval EFI_SUCCESS Successfully accept the memory
+ * @retval Others Other errors as indicated
+ */
+STATIC
+EFI_STATUS
+EFIAPI
+ApAcceptMemoryResourceRange (
+ UINT32 CpuIndex,
+ EFI_PHYSICAL_ADDRESS PhysicalStart,
+ EFI_PHYSICAL_ADDRESS PhysicalEnd
+ )
+{
+ UINT64 Status;
+ TD_RETURN_DATA TdReturnData;
+
+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ if (Status != TDX_EXIT_REASON_SUCCESS) {
+ ASSERT (FALSE);
+ return EFI_ABORTED;
+ }
+
+ if ((CpuIndex == 0) || (CpuIndex >= TdReturnData.TdInfo.NumVcpus)) {
+ ASSERT (FALSE);
+ return EFI_ABORTED;
+ }
+
+ return BspApAcceptMemoryResourceRange (CpuIndex, TdReturnData.TdInfo.NumVcpus, PhysicalStart, PhysicalEnd);
+}
+
+/**
+ * This function is called by BSP. It coordinates BSP/APs to accept memory together.
+ *
+ * @param PhysicalStart Start address of a memory region which is to be accepted
+ * @param PhysicalEnd End address of a memory region which is to be accepted
+ * @param APsStackAddress APs stack address
+ * @param CpusNum Total vCPU number of the Tdx guest
+ *
+ * @retval EFI_SUCCESS Successfully accept the memory
+ * @retval Others Other errors as indicated
+ */
+STATIC
+EFI_STATUS
+EFIAPI
+MpAcceptMemoryResourceRange (
+ IN EFI_PHYSICAL_ADDRESS PhysicalStart,
+ IN EFI_PHYSICAL_ADDRESS PhysicalEnd,
+ IN OUT EFI_PHYSICAL_ADDRESS APsStackAddress,
+ IN UINT32 CpusNum
+ )
+{
+ UINT64 Length;
+ EFI_STATUS Status;
+
+ Length = PhysicalEnd - PhysicalStart;
+
+ DEBUG ((DEBUG_INFO, "MpAccept : 0x%llx - 0x%llx (0x%llx)\n", PhysicalStart, PhysicalEnd, Length));
+
+ if (Length == 0) {
+ return EFI_SUCCESS;
+ }
+
+ //
+ // The start address is not 2M aligned. BSP first accept the part which is not 2M aligned.
+ //
+ if (ALIGN_VALUE (PhysicalStart, SIZE_2MB) != PhysicalStart) {
+ Length = MIN (ALIGN_VALUE (PhysicalStart, SIZE_2MB) - PhysicalStart, Length);
+ Status = BspAcceptMemoryResourceRange (PhysicalStart, PhysicalStart + Length);
+ ASSERT (Status == EFI_SUCCESS);
+
+ PhysicalStart += Length;
+ Length = PhysicalEnd - PhysicalStart;
+ }
+
+ if (Length == 0) {
+ return EFI_SUCCESS;
+ }
+
+ //
+ // BSP will accept the memory by itself if the memory is not big enough compared with a chunk.
+ //
+ if (Length <= ACCEPT_CHUNK_SIZE) {
+ return BspAcceptMemoryResourceRange (PhysicalStart, PhysicalEnd);
+ }
+
+ //
+ // Now APs are asked to accept the memory together.
+ //
+ MpSerializeStart ();
+
+ MpSendWakeupCommand (
+ MpProtectedModeWakeupCommandAcceptPages,
+ (UINT64)(UINTN)ApAcceptMemoryResourceRange,
+ PhysicalStart,
+ PhysicalEnd,
+ APsStackAddress,
+ AP_STACK_SIZE
+ );
+
+ //
+ // Now BSP does its job.
+ //
+ BspApAcceptMemoryResourceRange (0, CpusNum, PhysicalStart, PhysicalEnd);
+
+ MpSerializeEnd ();
+
+ return EFI_SUCCESS;
+}
+
+/**
+ BSP accept a small piece of memory which will be used as APs stack.
+
+ @param[in] VmmHobList The Hoblist pass the firmware
+ @param[in] APsStackSize APs stack size
+ @param[out] PhysicalAddressEnd The physical end address of accepted memory in phase-1
+
+ @retval EFI_SUCCESS Process the HobList successfully
+ @retval Others Other errors as indicated
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AcceptMemoryForAPsStack (
+ IN CONST VOID *VmmHobList,
+ IN UINT32 APsStackSize,
+ OUT EFI_PHYSICAL_ADDRESS *PhysicalAddressEnd
+ )
+{
+ EFI_STATUS Status;
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_PHYSICAL_ADDRESS PhysicalEnd;
+ EFI_PHYSICAL_ADDRESS PhysicalStart;
+ UINT64 ResourceLength;
+ BOOLEAN MemoryRegionFound;
+
+ ASSERT (VmmHobList != NULL);
+
+ Status = EFI_SUCCESS;
+ Hob.Raw = (UINT8 *)VmmHobList;
+ MemoryRegionFound = FALSE;
+
+ DEBUG ((DEBUG_INFO, "AcceptMemoryForAPsStack with APsStackSize=0x%x\n", APsStackSize));
+
+ //
+ // Parse the HOB list until end of list or matching type is found.
+ //
+ while (!END_OF_HOB_LIST (Hob) && !MemoryRegionFound) {
+ if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
+ DEBUG ((DEBUG_INFO, "\nResourceType: 0x%x\n", Hob.ResourceDescriptor->ResourceType));
+
+ if (Hob.ResourceDescriptor->ResourceType == BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
+ ResourceLength = Hob.ResourceDescriptor->ResourceLength;
+ PhysicalStart = Hob.ResourceDescriptor->PhysicalStart;
+ PhysicalEnd = PhysicalStart + ResourceLength;
+
+ DEBUG ((DEBUG_INFO, "ResourceAttribute: 0x%x\n", Hob.ResourceDescriptor->ResourceAttribute));
+ DEBUG ((DEBUG_INFO, "PhysicalStart: 0x%llx\n", PhysicalStart));
+ DEBUG ((DEBUG_INFO, "ResourceLength: 0x%llx\n", ResourceLength));
+ DEBUG ((DEBUG_INFO, "Owner: %g\n\n", &Hob.ResourceDescriptor->Owner));
+
+ if (ResourceLength >= APsStackSize) {
+ MemoryRegionFound = TRUE;
+ if (ResourceLength > ACCEPT_CHUNK_SIZE) {
+ PhysicalEnd = Hob.ResourceDescriptor->PhysicalStart + APsStackSize;
+ }
+ }
+
+ Status = BspAcceptMemoryResourceRange (
+ Hob.ResourceDescriptor->PhysicalStart,
+ PhysicalEnd
+ );
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ }
+ }
+
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ }
+
+ ASSERT (MemoryRegionFound);
+ *PhysicalAddressEnd = PhysicalEnd;
+
+ return Status;
+}
+
+/**
+ BSP and APs work togeter to accept memory which is under the address of 4G.
+
+ @param[in] VmmHobList The Hoblist pass the firmware
+ @param[in] CpusNum Number of vCPUs
+ @param[in] APsStackStartAddres Start address of APs stack
+ @param[in] PhysicalAddressStart Start physical address which to be accepted
+
+ @retval EFI_SUCCESS Process the HobList successfully
+ @retval Others Other errors as indicated
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AcceptMemory (
+ IN CONST VOID *VmmHobList,
+ IN UINT32 CpusNum,
+ IN EFI_PHYSICAL_ADDRESS APsStackStartAddress,
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddressStart
+ )
+{
+ EFI_STATUS Status;
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_PHYSICAL_ADDRESS PhysicalStart;
+ EFI_PHYSICAL_ADDRESS PhysicalEnd;
+ EFI_PHYSICAL_ADDRESS AcceptMemoryEndAddress;
+
+ Status = EFI_SUCCESS;
+ AcceptMemoryEndAddress = BASE_4GB;
+
+ ASSERT (VmmHobList != NULL);
+ Hob.Raw = (UINT8 *)VmmHobList;
+
+ DEBUG ((DEBUG_INFO, "AcceptMemory under address of 4G\n"));
+
+ //
+ // Parse the HOB list until end of list or matching type is found.
+ //
+ while (!END_OF_HOB_LIST (Hob)) {
+ if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
+ if (Hob.ResourceDescriptor->ResourceType == BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
+ PhysicalStart = Hob.ResourceDescriptor->PhysicalStart;
+ PhysicalEnd = PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
+
+ if (PhysicalEnd <= PhysicalAddressStart) {
+ // this memory region has been accepted. Skipped it.
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ continue;
+ }
+
+ if (PhysicalStart >= AcceptMemoryEndAddress) {
+ // this memory region is not to be accepted. And we're done.
+ break;
+ }
+
+ if (PhysicalStart >= PhysicalAddressStart) {
+ // this memory region has not been acceted.
+ } else if ((PhysicalStart < PhysicalAddressStart) && (PhysicalEnd > PhysicalAddressStart)) {
+ // part of the memory region has been accepted.
+ PhysicalStart = PhysicalAddressStart;
+ }
+
+ // then compare the PhysicalEnd with AcceptMemoryEndAddress
+ if (PhysicalEnd >= AcceptMemoryEndAddress) {
+ PhysicalEnd = AcceptMemoryEndAddress;
+ }
+
+ DEBUG ((DEBUG_INFO, "ResourceAttribute: 0x%x\n", Hob.ResourceDescriptor->ResourceAttribute));
+ DEBUG ((DEBUG_INFO, "PhysicalStart: 0x%llx\n", Hob.ResourceDescriptor->PhysicalStart));
+ DEBUG ((DEBUG_INFO, "ResourceLength: 0x%llx\n", Hob.ResourceDescriptor->ResourceLength));
+ DEBUG ((DEBUG_INFO, "Owner: %g\n\n", &Hob.ResourceDescriptor->Owner));
+
+ // Now we're ready to accept memory [PhysicalStart, PhysicalEnd)
+ if (CpusNum == 1) {
+ Status = BspAcceptMemoryResourceRange (PhysicalStart, PhysicalEnd);
+ } else {
+ Status = MpAcceptMemoryResourceRange (
+ PhysicalStart,
+ PhysicalEnd,
+ APsStackStartAddress,
+ CpusNum
+ );
+ }
+
+ if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ break;
+ }
+
+ if (PhysicalEnd == AcceptMemoryEndAddress) {
+ break;
+ }
+ }
+ }
+
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ }
+
+ return Status;
+}
+
+/**
+ Check the value whether in the valid list.
+
+ @param[in] Value A value
+ @param[in] ValidList A pointer to valid list
+ @param[in] ValidListLength Length of valid list
+
+ @retval TRUE The value is in valid list.
+ @retval FALSE The value is not in valid list.
+
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsInValidList (
+ IN UINT32 Value,
+ IN UINT32 *ValidList,
+ IN UINT32 ValidListLength
+ )
+{
+ UINT32 index;
+
+ if (ValidList == NULL) {
+ return FALSE;
+ }
+
+ for (index = 0; index < ValidListLength; index++) {
+ if (ValidList[index] == Value) {
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+}
+
+/**
+ Check the integrity of VMM Hob List.
+
+ @param[in] VmmHobList A pointer to Hob List
+
+ @retval TRUE The Hob List is valid.
+ @retval FALSE The Hob List is invalid.
+
+**/
+STATIC
+BOOLEAN
+EFIAPI
+ValidateHobList (
+ IN CONST VOID *VmmHobList
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ UINT32 EFI_BOOT_MODE_LIST[] = {
+ BOOT_WITH_FULL_CONFIGURATION,
+ BOOT_WITH_MINIMAL_CONFIGURATION,
+ BOOT_ASSUMING_NO_CONFIGURATION_CHANGES,
+ BOOT_WITH_FULL_CONFIGURATION_PLUS_DIAGNOSTICS,
+ BOOT_WITH_DEFAULT_SETTINGS,
+ BOOT_ON_S4_RESUME,
+ BOOT_ON_S5_RESUME,
+ BOOT_WITH_MFG_MODE_SETTINGS,
+ BOOT_ON_S2_RESUME,
+ BOOT_ON_S3_RESUME,
+ BOOT_ON_FLASH_UPDATE,
+ BOOT_IN_RECOVERY_MODE
+ };
+
+ UINT32 EFI_RESOURCE_TYPE_LIST[] = {
+ EFI_RESOURCE_SYSTEM_MEMORY,
+ EFI_RESOURCE_MEMORY_MAPPED_IO,
+ EFI_RESOURCE_IO,
+ EFI_RESOURCE_FIRMWARE_DEVICE,
+ EFI_RESOURCE_MEMORY_MAPPED_IO_PORT,
+ EFI_RESOURCE_MEMORY_RESERVED,
+ EFI_RESOURCE_IO_RESERVED,
+ BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED
+ };
+
+ if (VmmHobList == NULL) {
+ DEBUG ((DEBUG_ERROR, "HOB: HOB data pointer is NULL\n"));
+ return FALSE;
+ }
+
+ Hob.Raw = (UINT8 *)VmmHobList;
+
+ //
+ // Parse the HOB list until end of list or matching type is found.
+ //
+ while (!END_OF_HOB_LIST (Hob)) {
+ if (Hob.Header->Reserved != (UINT32)0) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob header Reserved filed should be zero\n"));
+ return FALSE;
+ }
+
+ if (Hob.Header->HobLength == 0) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob header LEANGTH should not be zero\n"));
+ return FALSE;
+ }
+
+ switch (Hob.Header->HobType) {
+ case EFI_HOB_TYPE_HANDOFF:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_HANDOFF_INFO_TABLE)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_HANDOFF));
+ return FALSE;
+ }
+
+ if (IsInValidList (Hob.HandoffInformationTable->BootMode, EFI_BOOT_MODE_LIST, ARRAY_SIZE (EFI_BOOT_MODE_LIST)) == FALSE) {
+ DEBUG ((DEBUG_ERROR, "HOB: Unknow HandoffInformationTable BootMode type. Type: 0x%08x\n", Hob.HandoffInformationTable->BootMode));
+ return FALSE;
+ }
+
+ if ((Hob.HandoffInformationTable->EfiFreeMemoryTop % 4096) != 0) {
+ DEBUG ((DEBUG_ERROR, "HOB: HandoffInformationTable EfiFreeMemoryTop address must be 4-KB aligned to meet page restrictions of UEFI.\
+ Address: 0x%016lx\n", Hob.HandoffInformationTable->EfiFreeMemoryTop));
+ return FALSE;
+ }
+
+ break;
+
+ case EFI_HOB_TYPE_RESOURCE_DESCRIPTOR:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_RESOURCE_DESCRIPTOR)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_RESOURCE_DESCRIPTOR));
+ return FALSE;
+ }
+
+ if (IsInValidList (Hob.ResourceDescriptor->ResourceType, EFI_RESOURCE_TYPE_LIST, ARRAY_SIZE (EFI_RESOURCE_TYPE_LIST)) == FALSE) {
+ DEBUG ((DEBUG_ERROR, "HOB: Unknow ResourceDescriptor ResourceType type. Type: 0x%08x\n", Hob.ResourceDescriptor->ResourceType));
+ return FALSE;
+ }
+
+ if ((Hob.ResourceDescriptor->ResourceAttribute & (~(EFI_RESOURCE_ATTRIBUTE_PRESENT |
+ EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+ EFI_RESOURCE_ATTRIBUTE_TESTED |
+ EFI_RESOURCE_ATTRIBUTE_READ_PROTECTED |
+ EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTED |
+ EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTED |
+ EFI_RESOURCE_ATTRIBUTE_PERSISTENT |
+ EFI_RESOURCE_ATTRIBUTE_SINGLE_BIT_ECC |
+ EFI_RESOURCE_ATTRIBUTE_MULTIPLE_BIT_ECC |
+ EFI_RESOURCE_ATTRIBUTE_ECC_RESERVED_1 |
+ EFI_RESOURCE_ATTRIBUTE_ECC_RESERVED_2 |
+ EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
+ EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
+ EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
+ EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
+ EFI_RESOURCE_ATTRIBUTE_16_BIT_IO |
+ EFI_RESOURCE_ATTRIBUTE_32_BIT_IO |
+ EFI_RESOURCE_ATTRIBUTE_64_BIT_IO |
+ EFI_RESOURCE_ATTRIBUTE_UNCACHED_EXPORTED |
+ EFI_RESOURCE_ATTRIBUTE_READ_PROTECTABLE |
+ EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE |
+ EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTABLE |
+ EFI_RESOURCE_ATTRIBUTE_PERSISTABLE |
+ EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED |
+ EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE |
+ EFI_RESOURCE_ATTRIBUTE_MORE_RELIABLE))) != 0)
+ {
+ DEBUG ((DEBUG_ERROR, "HOB: Unknow ResourceDescriptor ResourceAttribute type. Type: 0x%08x\n", Hob.ResourceDescriptor->ResourceAttribute));
+ return FALSE;
+ }
+
+ break;
+
+ // EFI_HOB_GUID_TYPE is variable length data, so skip check
+ case EFI_HOB_TYPE_GUID_EXTENSION:
+ break;
+
+ case EFI_HOB_TYPE_FV:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV));
+ return FALSE;
+ }
+
+ break;
+
+ case EFI_HOB_TYPE_FV2:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME2)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV2));
+ return FALSE;
+ }
+
+ break;
+
+ case EFI_HOB_TYPE_FV3:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME3)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV3));
+ return FALSE;
+ }
+
+ break;
+
+ case EFI_HOB_TYPE_CPU:
+ if (Hob.Header->HobLength != sizeof (EFI_HOB_CPU)) {
+ DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_CPU));
+ return FALSE;
+ }
+
+ for (UINT32 index = 0; index < 6; index++) {
+ if (Hob.Cpu->Reserved[index] != 0) {
+ DEBUG ((DEBUG_ERROR, "HOB: Cpu Reserved field will always be set to zero.\n"));
+ return FALSE;
+ }
+ }
+
+ break;
+
+ default:
+ DEBUG ((DEBUG_ERROR, "HOB: Hob type is not know. Type: 0x%04x\n", Hob.Header->HobType));
+ return FALSE;
+ }
+
+ // Get next HOB
+ Hob.Raw = (UINT8 *)(Hob.Raw + Hob.Header->HobLength);
+ }
+
+ return TRUE;
+}
+
+/**
+ Processing the incoming HobList for the TDX
+
+ Firmware must parse list, and accept the pages of memory before their can be
+ use by the guest.
+
+ @param[in] VmmHobList The Hoblist pass the firmware
+
+ @retval EFI_SUCCESS Process the HobList successfully
+ @retval Others Other errors as indicated
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ProcessHobList (
+ IN CONST VOID *VmmHobList
+ )
+{
+ EFI_STATUS Status;
+ UINT32 CpusNum;
+ EFI_PHYSICAL_ADDRESS PhysicalEnd;
+ EFI_PHYSICAL_ADDRESS APsStackStartAddress;
+
+ CpusNum = GetCpusNum ();
+
+ //
+ // If there are mutli-vCPU in a TDX guest, accept memory is split into 2 phases.
+ // Phase-1 accepts a small piece of memory by BSP. This piece of memory
+ // is used to setup AP's stack.
+ // After that phase-2 accepts a big piece of memory by BSP/APs.
+ //
+ // TDVF supports 4K and 2M accept-page-size. The memory which can be accpeted
+ // in 2M accept-page-size must be 2M aligned and multiple 2M. So we align
+ // APsStackSize to 2M size aligned.
+ //
+ if (CpusNum > 1) {
+ Status = AcceptMemoryForAPsStack (VmmHobList, APS_STACK_SIZE (CpusNum), &PhysicalEnd);
+ ASSERT (Status == EFI_SUCCESS);
+ APsStackStartAddress = PhysicalEnd - APS_STACK_SIZE (CpusNum);
+ } else {
+ PhysicalEnd = 0;
+ APsStackStartAddress = 0;
+ }
+
+ Status = AcceptMemory (VmmHobList, CpusNum, APsStackStartAddress, PhysicalEnd);
+ ASSERT (Status == EFI_SUCCESS);
+
+ return Status;
+}
+
/**
In Tdx guest, some information need to be passed from host VMM to guest
firmware. For example, the memory resource, etc. These information are
@@ -49,7 +772,36 @@ TdxHelperProcessTdHob (
VOID
)
{
- return EFI_UNSUPPORTED;
+ EFI_STATUS Status;
+ VOID *TdHob;
+ TD_RETURN_DATA TdReturnData;
+
+ TdHob = (VOID *)(UINTN)FixedPcdGet32 (PcdOvmfSecGhcbBase);
+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "Intel Tdx Started with (GPAW: %d, Cpus: %d)\n",
+ TdReturnData.TdInfo.Gpaw,
+ TdReturnData.TdInfo.NumVcpus
+ ));
+
+ //
+ // Validate HobList
+ //
+ if (ValidateHobList (TdHob) == FALSE) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Process Hoblist to accept memory
+ //
+ Status = ProcessHobList (TdHob);
+
+ return Status;
}

/**
diff --git a/OvmfPkg/Library/PlatformInitLib/IntelTdx.c b/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
index 6cb63139cba0..ada8592ddd5a 100644
--- a/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
+++ b/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
@@ -16,779 +16,11 @@
#include <Library/MemoryAllocationLib.h>
#include <IndustryStandard/Tdx.h>
#include <IndustryStandard/IntelTdx.h>
-#include <IndustryStandard/QemuFwCfg.h>
-#include <Library/QemuFwCfgLib.h>
#include <Library/PeiServicesLib.h>
-#include <Library/TdxLib.h>
-#include <Library/TdxMailboxLib.h>
-#include <Library/SynchronizationLib.h>
#include <Pi/PrePiHob.h>
#include <WorkArea.h>
#include <ConfidentialComputingGuestAttr.h>

-#define ALIGNED_2MB_MASK 0x1fffff
-#define MEGABYTE_SHIFT 20
-
-#define ACCEPT_CHUNK_SIZE SIZE_32MB
-#define AP_STACK_SIZE SIZE_16KB
-#define APS_STACK_SIZE(CpusNum) (ALIGN_VALUE(CpusNum*AP_STACK_SIZE, SIZE_2MB))
-
-/**
- This function will be called to accept pages. Only BSP accepts pages.
-
- TDCALL(ACCEPT_PAGE) supports the accept page size of 4k and 2M. To
- simplify the implementation, the Memory to be accpeted is splitted
- into 3 parts:
- ----------------- <-- StartAddress1 (not 2M aligned)
- | part 1 | Length1 < 2M
- |---------------| <-- StartAddress2 (2M aligned)
- | | Length2 = Integer multiples of 2M
- | part 2 |
- | |
- |---------------| <-- StartAddress3
- | part 3 | Length3 < 2M
- |---------------|
-
- @param[in] PhysicalAddress Start physical adress
- @param[in] PhysicalEnd End physical address
-
- @retval EFI_SUCCESS Accept memory successfully
- @retval Others Other errors as indicated
-**/
-EFI_STATUS
-EFIAPI
-BspAcceptMemoryResourceRange (
- IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
- IN EFI_PHYSICAL_ADDRESS PhysicalEnd
- )
-{
- EFI_STATUS Status;
- UINT32 AcceptPageSize;
- UINT64 StartAddress1;
- UINT64 StartAddress2;
- UINT64 StartAddress3;
- UINT64 TotalLength;
- UINT64 Length1;
- UINT64 Length2;
- UINT64 Length3;
- UINT64 Pages;
-
- AcceptPageSize = FixedPcdGet32 (PcdTdxAcceptPageSize);
- TotalLength = PhysicalEnd - PhysicalAddress;
- StartAddress1 = 0;
- StartAddress2 = 0;
- StartAddress3 = 0;
- Length1 = 0;
- Length2 = 0;
- Length3 = 0;
-
- if (TotalLength == 0) {
- return EFI_SUCCESS;
- }
-
- if (ALIGN_VALUE (PhysicalAddress, SIZE_2MB) != PhysicalAddress) {
- StartAddress1 = PhysicalAddress;
- Length1 = ALIGN_VALUE (PhysicalAddress, SIZE_2MB) - PhysicalAddress;
- if (Length1 >= TotalLength) {
- Length1 = TotalLength;
- }
-
- PhysicalAddress += Length1;
- TotalLength -= Length1;
- }
-
- if (TotalLength > SIZE_2MB) {
- StartAddress2 = PhysicalAddress;
- Length2 = TotalLength & ~(UINT64)ALIGNED_2MB_MASK;
- PhysicalAddress += Length2;
- TotalLength -= Length2;
- }
-
- if (TotalLength) {
- StartAddress3 = PhysicalAddress;
- Length3 = TotalLength;
- }
-
- Status = EFI_SUCCESS;
- if (Length1 > 0) {
- Pages = Length1 / SIZE_4KB;
- Status = TdAcceptPages (StartAddress1, Pages, SIZE_4KB);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- }
-
- if (Length2 > 0) {
- Pages = Length2 / AcceptPageSize;
- Status = TdAcceptPages (StartAddress2, Pages, AcceptPageSize);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- }
-
- if (Length3 > 0) {
- Pages = Length3 / SIZE_4KB;
- Status = TdAcceptPages (StartAddress3, Pages, SIZE_4KB);
- ASSERT (!EFI_ERROR (Status));
- if (EFI_ERROR (Status)) {
- return Status;
- }
- }
-
- return Status;
-}
-
-/**
- * This function is called by BSP and APs to accept memory.
- * Note:
- * The input PhysicalStart/PhysicalEnd indicates the whole memory region
- * to be accepted. BSP or AP only accepts one piece in the whole memory region.
- *
- * @param CpuIndex vCPU index
- * @param CpusNum Total vCPU number of a Tdx guest
- * @param PhysicalStart Start address of a memory region which is to be accepted
- * @param PhysicalEnd End address of a memory region which is to be accepted
- *
- * @retval EFI_SUCCESS Successfully accept the memory
- * @retval Other Other errors as indicated
- */
-STATIC
-EFI_STATUS
-EFIAPI
-BspApAcceptMemoryResourceRange (
- UINT32 CpuIndex,
- UINT32 CpusNum,
- EFI_PHYSICAL_ADDRESS PhysicalStart,
- EFI_PHYSICAL_ADDRESS PhysicalEnd
- )
-{
- UINT64 Status;
- UINT64 Pages;
- UINT64 Stride;
- UINT64 AcceptPageSize;
- EFI_PHYSICAL_ADDRESS PhysicalAddress;
-
- AcceptPageSize = (UINT64)(UINTN)FixedPcdGet32 (PcdTdxAcceptPageSize);
-
- Status = EFI_SUCCESS;
- Stride = (UINTN)CpusNum * ACCEPT_CHUNK_SIZE;
- PhysicalAddress = PhysicalStart + ACCEPT_CHUNK_SIZE * (UINTN)CpuIndex;
-
- while (!EFI_ERROR (Status) && PhysicalAddress < PhysicalEnd) {
- Pages = MIN (ACCEPT_CHUNK_SIZE, PhysicalEnd - PhysicalAddress) / AcceptPageSize;
- Status = TdAcceptPages (PhysicalAddress, Pages, (UINT32)(UINTN)AcceptPageSize);
- ASSERT (!EFI_ERROR (Status));
- PhysicalAddress += Stride;
- }
-
- return EFI_SUCCESS;
-}
-
-/**
- * This function is called by APs to accept memory.
- *
- * @param CpuIndex vCPU index of an AP
- * @param PhysicalStart Start address of a memory region which is to be accepted
- * @param PhysicalEnd End address of a memory region which is to be accepted
- *
- * @retval EFI_SUCCESS Successfully accept the memory
- * @retval Others Other errors as indicated
- */
-STATIC
-EFI_STATUS
-EFIAPI
-ApAcceptMemoryResourceRange (
- UINT32 CpuIndex,
- EFI_PHYSICAL_ADDRESS PhysicalStart,
- EFI_PHYSICAL_ADDRESS PhysicalEnd
- )
-{
- UINT64 Status;
- TD_RETURN_DATA TdReturnData;
-
- Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
- if (Status != TDX_EXIT_REASON_SUCCESS) {
- ASSERT (FALSE);
- return EFI_ABORTED;
- }
-
- if ((CpuIndex == 0) || (CpuIndex >= TdReturnData.TdInfo.NumVcpus)) {
- ASSERT (FALSE);
- return EFI_ABORTED;
- }
-
- return BspApAcceptMemoryResourceRange (CpuIndex, TdReturnData.TdInfo.NumVcpus, PhysicalStart, PhysicalEnd);
-}
-
-/**
- * This function is called by BSP. It coordinates BSP/APs to accept memory together.
- *
- * @param PhysicalStart Start address of a memory region which is to be accepted
- * @param PhysicalEnd End address of a memory region which is to be accepted
- * @param APsStackAddress APs stack address
- * @param CpusNum Total vCPU number of the Tdx guest
- *
- * @retval EFI_SUCCESS Successfully accept the memory
- * @retval Others Other errors as indicated
- */
-EFI_STATUS
-EFIAPI
-MpAcceptMemoryResourceRange (
- IN EFI_PHYSICAL_ADDRESS PhysicalStart,
- IN EFI_PHYSICAL_ADDRESS PhysicalEnd,
- IN OUT EFI_PHYSICAL_ADDRESS APsStackAddress,
- IN UINT32 CpusNum
- )
-{
- UINT64 Length;
- EFI_STATUS Status;
-
- Length = PhysicalEnd - PhysicalStart;
-
- DEBUG ((DEBUG_INFO, "MpAccept : 0x%llx - 0x%llx (0x%llx)\n", PhysicalStart, PhysicalEnd, Length));
-
- if (Length == 0) {
- return EFI_SUCCESS;
- }
-
- //
- // The start address is not 2M aligned. BSP first accept the part which is not 2M aligned.
- //
- if (ALIGN_VALUE (PhysicalStart, SIZE_2MB) != PhysicalStart) {
- Length = MIN (ALIGN_VALUE (PhysicalStart, SIZE_2MB) - PhysicalStart, Length);
- Status = BspAcceptMemoryResourceRange (PhysicalStart, PhysicalStart + Length);
- ASSERT (Status == EFI_SUCCESS);
-
- PhysicalStart += Length;
- Length = PhysicalEnd - PhysicalStart;
- }
-
- if (Length == 0) {
- return EFI_SUCCESS;
- }
-
- //
- // BSP will accept the memory by itself if the memory is not big enough compared with a chunk.
- //
- if (Length <= ACCEPT_CHUNK_SIZE) {
- return BspAcceptMemoryResourceRange (PhysicalStart, PhysicalEnd);
- }
-
- //
- // Now APs are asked to accept the memory together.
- //
- MpSerializeStart ();
-
- MpSendWakeupCommand (
- MpProtectedModeWakeupCommandAcceptPages,
- (UINT64)(UINTN)ApAcceptMemoryResourceRange,
- PhysicalStart,
- PhysicalEnd,
- APsStackAddress,
- AP_STACK_SIZE
- );
-
- //
- // Now BSP does its job.
- //
- BspApAcceptMemoryResourceRange (0, CpusNum, PhysicalStart, PhysicalEnd);
-
- MpSerializeEnd ();
-
- return EFI_SUCCESS;
-}
-
-/**
- BSP accept a small piece of memory which will be used as APs stack.
-
- @param[in] VmmHobList The Hoblist pass the firmware
- @param[in] APsStackSize APs stack size
- @param[out] PhysicalAddressEnd The physical end address of accepted memory in phase-1
-
- @retval EFI_SUCCESS Process the HobList successfully
- @retval Others Other errors as indicated
-**/
-EFI_STATUS
-EFIAPI
-AcceptMemoryForAPsStack (
- IN CONST VOID *VmmHobList,
- IN UINT32 APsStackSize,
- OUT EFI_PHYSICAL_ADDRESS *PhysicalAddressEnd
- )
-{
- EFI_STATUS Status;
- EFI_PEI_HOB_POINTERS Hob;
- EFI_PHYSICAL_ADDRESS PhysicalEnd;
- EFI_PHYSICAL_ADDRESS PhysicalStart;
- UINT64 ResourceLength;
- BOOLEAN MemoryRegionFound;
-
- ASSERT (VmmHobList != NULL);
-
- Status = EFI_SUCCESS;
- Hob.Raw = (UINT8 *)VmmHobList;
- MemoryRegionFound = FALSE;
-
- DEBUG ((DEBUG_INFO, "AcceptMemoryForAPsStack with APsStackSize=0x%x\n", APsStackSize));
-
- //
- // Parse the HOB list until end of list or matching type is found.
- //
- while (!END_OF_HOB_LIST (Hob) && !MemoryRegionFound) {
- if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
- DEBUG ((DEBUG_INFO, "\nResourceType: 0x%x\n", Hob.ResourceDescriptor->ResourceType));
-
- if (Hob.ResourceDescriptor->ResourceType == BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
- ResourceLength = Hob.ResourceDescriptor->ResourceLength;
- PhysicalStart = Hob.ResourceDescriptor->PhysicalStart;
- PhysicalEnd = PhysicalStart + ResourceLength;
-
- DEBUG ((DEBUG_INFO, "ResourceAttribute: 0x%x\n", Hob.ResourceDescriptor->ResourceAttribute));
- DEBUG ((DEBUG_INFO, "PhysicalStart: 0x%llx\n", PhysicalStart));
- DEBUG ((DEBUG_INFO, "ResourceLength: 0x%llx\n", ResourceLength));
- DEBUG ((DEBUG_INFO, "Owner: %g\n\n", &Hob.ResourceDescriptor->Owner));
-
- if (ResourceLength >= APsStackSize) {
- MemoryRegionFound = TRUE;
- if (ResourceLength > ACCEPT_CHUNK_SIZE) {
- PhysicalEnd = Hob.ResourceDescriptor->PhysicalStart + APsStackSize;
- }
- }
-
- Status = BspAcceptMemoryResourceRange (
- Hob.ResourceDescriptor->PhysicalStart,
- PhysicalEnd
- );
- if (EFI_ERROR (Status)) {
- break;
- }
- }
- }
-
- Hob.Raw = GET_NEXT_HOB (Hob);
- }
-
- ASSERT (MemoryRegionFound);
- *PhysicalAddressEnd = PhysicalEnd;
-
- return Status;
-}
-
-/**
- BSP and APs work togeter to accept memory which is under the address of 4G.
-
- @param[in] VmmHobList The Hoblist pass the firmware
- @param[in] CpusNum Number of vCPUs
- @param[in] APsStackStartAddres Start address of APs stack
- @param[in] PhysicalAddressStart Start physical address which to be accepted
-
- @retval EFI_SUCCESS Process the HobList successfully
- @retval Others Other errors as indicated
-**/
-EFI_STATUS
-EFIAPI
-AcceptMemory (
- IN CONST VOID *VmmHobList,
- IN UINT32 CpusNum,
- IN EFI_PHYSICAL_ADDRESS APsStackStartAddress,
- IN EFI_PHYSICAL_ADDRESS PhysicalAddressStart
- )
-{
- EFI_STATUS Status;
- EFI_PEI_HOB_POINTERS Hob;
- EFI_PHYSICAL_ADDRESS PhysicalStart;
- EFI_PHYSICAL_ADDRESS PhysicalEnd;
- EFI_PHYSICAL_ADDRESS AcceptMemoryEndAddress;
-
- Status = EFI_SUCCESS;
- AcceptMemoryEndAddress = BASE_4GB;
-
- ASSERT (VmmHobList != NULL);
- Hob.Raw = (UINT8 *)VmmHobList;
-
- DEBUG ((DEBUG_INFO, "AcceptMemory under address of 4G\n"));
-
- //
- // Parse the HOB list until end of list or matching type is found.
- //
- while (!END_OF_HOB_LIST (Hob)) {
- if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
- if (Hob.ResourceDescriptor->ResourceType == BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
- PhysicalStart = Hob.ResourceDescriptor->PhysicalStart;
- PhysicalEnd = PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
-
- if (PhysicalEnd <= PhysicalAddressStart) {
- // this memory region has been accepted. Skipped it.
- Hob.Raw = GET_NEXT_HOB (Hob);
- continue;
- }
-
- if (PhysicalStart >= AcceptMemoryEndAddress) {
- // this memory region is not to be accepted. And we're done.
- break;
- }
-
- if (PhysicalStart >= PhysicalAddressStart) {
- // this memory region has not been acceted.
- } else if ((PhysicalStart < PhysicalAddressStart) && (PhysicalEnd > PhysicalAddressStart)) {
- // part of the memory region has been accepted.
- PhysicalStart = PhysicalAddressStart;
- }
-
- // then compare the PhysicalEnd with AcceptMemoryEndAddress
- if (PhysicalEnd >= AcceptMemoryEndAddress) {
- PhysicalEnd = AcceptMemoryEndAddress;
- }
-
- DEBUG ((DEBUG_INFO, "ResourceAttribute: 0x%x\n", Hob.ResourceDescriptor->ResourceAttribute));
- DEBUG ((DEBUG_INFO, "PhysicalStart: 0x%llx\n", Hob.ResourceDescriptor->PhysicalStart));
- DEBUG ((DEBUG_INFO, "ResourceLength: 0x%llx\n", Hob.ResourceDescriptor->ResourceLength));
- DEBUG ((DEBUG_INFO, "Owner: %g\n\n", &Hob.ResourceDescriptor->Owner));
-
- // Now we're ready to accept memory [PhysicalStart, PhysicalEnd)
- if (CpusNum == 1) {
- Status = BspAcceptMemoryResourceRange (PhysicalStart, PhysicalEnd);
- } else {
- Status = MpAcceptMemoryResourceRange (
- PhysicalStart,
- PhysicalEnd,
- APsStackStartAddress,
- CpusNum
- );
- }
-
- if (EFI_ERROR (Status)) {
- ASSERT (FALSE);
- break;
- }
-
- if (PhysicalEnd == AcceptMemoryEndAddress) {
- break;
- }
- }
- }
-
- Hob.Raw = GET_NEXT_HOB (Hob);
- }
-
- return Status;
-}
-
-/**
- Check the value whether in the valid list.
-
- @param[in] Value A value
- @param[in] ValidList A pointer to valid list
- @param[in] ValidListLength Length of valid list
-
- @retval TRUE The value is in valid list.
- @retval FALSE The value is not in valid list.
-
-**/
-BOOLEAN
-EFIAPI
-IsInValidList (
- IN UINT32 Value,
- IN UINT32 *ValidList,
- IN UINT32 ValidListLength
- )
-{
- UINT32 index;
-
- if (ValidList == NULL) {
- return FALSE;
- }
-
- for (index = 0; index < ValidListLength; index++) {
- if (ValidList[index] == Value) {
- return TRUE;
- }
- }
-
- return FALSE;
-}
-
-/**
- Check the integrity of VMM Hob List.
-
- @param[in] VmmHobList A pointer to Hob List
-
- @retval TRUE The Hob List is valid.
- @retval FALSE The Hob List is invalid.
-
-**/
-BOOLEAN
-EFIAPI
-ValidateHobList (
- IN CONST VOID *VmmHobList
- )
-{
- EFI_PEI_HOB_POINTERS Hob;
- UINT32 EFI_BOOT_MODE_LIST[] = {
- BOOT_WITH_FULL_CONFIGURATION,
- BOOT_WITH_MINIMAL_CONFIGURATION,
- BOOT_ASSUMING_NO_CONFIGURATION_CHANGES,
- BOOT_WITH_FULL_CONFIGURATION_PLUS_DIAGNOSTICS,
- BOOT_WITH_DEFAULT_SETTINGS,
- BOOT_ON_S4_RESUME,
- BOOT_ON_S5_RESUME,
- BOOT_WITH_MFG_MODE_SETTINGS,
- BOOT_ON_S2_RESUME,
- BOOT_ON_S3_RESUME,
- BOOT_ON_FLASH_UPDATE,
- BOOT_IN_RECOVERY_MODE
- };
-
- UINT32 EFI_RESOURCE_TYPE_LIST[] = {
- EFI_RESOURCE_SYSTEM_MEMORY,
- EFI_RESOURCE_MEMORY_MAPPED_IO,
- EFI_RESOURCE_IO,
- EFI_RESOURCE_FIRMWARE_DEVICE,
- EFI_RESOURCE_MEMORY_MAPPED_IO_PORT,
- EFI_RESOURCE_MEMORY_RESERVED,
- EFI_RESOURCE_IO_RESERVED,
- BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED
- };
-
- if (VmmHobList == NULL) {
- DEBUG ((DEBUG_ERROR, "HOB: HOB data pointer is NULL\n"));
- return FALSE;
- }
-
- Hob.Raw = (UINT8 *)VmmHobList;
-
- //
- // Parse the HOB list until end of list or matching type is found.
- //
- while (!END_OF_HOB_LIST (Hob)) {
- if (Hob.Header->Reserved != (UINT32)0) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob header Reserved filed should be zero\n"));
- return FALSE;
- }
-
- if (Hob.Header->HobLength == 0) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob header LEANGTH should not be zero\n"));
- return FALSE;
- }
-
- switch (Hob.Header->HobType) {
- case EFI_HOB_TYPE_HANDOFF:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_HANDOFF_INFO_TABLE)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_HANDOFF));
- return FALSE;
- }
-
- if (IsInValidList (Hob.HandoffInformationTable->BootMode, EFI_BOOT_MODE_LIST, ARRAY_SIZE (EFI_BOOT_MODE_LIST)) == FALSE) {
- DEBUG ((DEBUG_ERROR, "HOB: Unknow HandoffInformationTable BootMode type. Type: 0x%08x\n", Hob.HandoffInformationTable->BootMode));
- return FALSE;
- }
-
- if ((Hob.HandoffInformationTable->EfiFreeMemoryTop % 4096) != 0) {
- DEBUG ((DEBUG_ERROR, "HOB: HandoffInformationTable EfiFreeMemoryTop address must be 4-KB aligned to meet page restrictions of UEFI.\
- Address: 0x%016lx\n", Hob.HandoffInformationTable->EfiFreeMemoryTop));
- return FALSE;
- }
-
- break;
-
- case EFI_HOB_TYPE_RESOURCE_DESCRIPTOR:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_RESOURCE_DESCRIPTOR)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_RESOURCE_DESCRIPTOR));
- return FALSE;
- }
-
- if (IsInValidList (Hob.ResourceDescriptor->ResourceType, EFI_RESOURCE_TYPE_LIST, ARRAY_SIZE (EFI_RESOURCE_TYPE_LIST)) == FALSE) {
- DEBUG ((DEBUG_ERROR, "HOB: Unknow ResourceDescriptor ResourceType type. Type: 0x%08x\n", Hob.ResourceDescriptor->ResourceType));
- return FALSE;
- }
-
- if ((Hob.ResourceDescriptor->ResourceAttribute & (~(EFI_RESOURCE_ATTRIBUTE_PRESENT |
- EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
- EFI_RESOURCE_ATTRIBUTE_TESTED |
- EFI_RESOURCE_ATTRIBUTE_READ_PROTECTED |
- EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTED |
- EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTED |
- EFI_RESOURCE_ATTRIBUTE_PERSISTENT |
- EFI_RESOURCE_ATTRIBUTE_SINGLE_BIT_ECC |
- EFI_RESOURCE_ATTRIBUTE_MULTIPLE_BIT_ECC |
- EFI_RESOURCE_ATTRIBUTE_ECC_RESERVED_1 |
- EFI_RESOURCE_ATTRIBUTE_ECC_RESERVED_2 |
- EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
- EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
- EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
- EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
- EFI_RESOURCE_ATTRIBUTE_16_BIT_IO |
- EFI_RESOURCE_ATTRIBUTE_32_BIT_IO |
- EFI_RESOURCE_ATTRIBUTE_64_BIT_IO |
- EFI_RESOURCE_ATTRIBUTE_UNCACHED_EXPORTED |
- EFI_RESOURCE_ATTRIBUTE_READ_PROTECTABLE |
- EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE |
- EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTABLE |
- EFI_RESOURCE_ATTRIBUTE_PERSISTABLE |
- EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED |
- EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE |
- EFI_RESOURCE_ATTRIBUTE_MORE_RELIABLE))) != 0)
- {
- DEBUG ((DEBUG_ERROR, "HOB: Unknow ResourceDescriptor ResourceAttribute type. Type: 0x%08x\n", Hob.ResourceDescriptor->ResourceAttribute));
- return FALSE;
- }
-
- break;
-
- // EFI_HOB_GUID_TYPE is variable length data, so skip check
- case EFI_HOB_TYPE_GUID_EXTENSION:
- break;
-
- case EFI_HOB_TYPE_FV:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV));
- return FALSE;
- }
-
- break;
-
- case EFI_HOB_TYPE_FV2:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME2)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV2));
- return FALSE;
- }
-
- break;
-
- case EFI_HOB_TYPE_FV3:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_FIRMWARE_VOLUME3)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_FV3));
- return FALSE;
- }
-
- break;
-
- case EFI_HOB_TYPE_CPU:
- if (Hob.Header->HobLength != sizeof (EFI_HOB_CPU)) {
- DEBUG ((DEBUG_ERROR, "HOB: Hob length is not equal corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_CPU));
- return FALSE;
- }
-
- for (UINT32 index = 0; index < 6; index++) {
- if (Hob.Cpu->Reserved[index] != 0) {
- DEBUG ((DEBUG_ERROR, "HOB: Cpu Reserved field will always be set to zero.\n"));
- return FALSE;
- }
- }
-
- break;
-
- default:
- DEBUG ((DEBUG_ERROR, "HOB: Hob type is not know. Type: 0x%04x\n", Hob.Header->HobType));
- return FALSE;
- }
-
- // Get next HOB
- Hob.Raw = (UINT8 *)(Hob.Raw + Hob.Header->HobLength);
- }
-
- return TRUE;
-}
-
-/**
- Processing the incoming HobList for the TDX
-
- Firmware must parse list, and accept the pages of memory before their can be
- use by the guest.
-
- @param[in] VmmHobList The Hoblist pass the firmware
-
- @retval EFI_SUCCESS Process the HobList successfully
- @retval Others Other errors as indicated
-
-**/
-EFI_STATUS
-EFIAPI
-ProcessHobList (
- IN CONST VOID *VmmHobList
- )
-{
- EFI_STATUS Status;
- UINT32 CpusNum;
- EFI_PHYSICAL_ADDRESS PhysicalEnd;
- EFI_PHYSICAL_ADDRESS APsStackStartAddress;
-
- CpusNum = GetCpusNum ();
-
- //
- // If there are mutli-vCPU in a TDX guest, accept memory is split into 2 phases.
- // Phase-1 accepts a small piece of memory by BSP. This piece of memory
- // is used to setup AP's stack.
- // After that phase-2 accepts a big piece of memory by BSP/APs.
- //
- // TDVF supports 4K and 2M accept-page-size. The memory which can be accpeted
- // in 2M accept-page-size must be 2M aligned and multiple 2M. So we align
- // APsStackSize to 2M size aligned.
- //
- if (CpusNum > 1) {
- Status = AcceptMemoryForAPsStack (VmmHobList, APS_STACK_SIZE (CpusNum), &PhysicalEnd);
- ASSERT (Status == EFI_SUCCESS);
- APsStackStartAddress = PhysicalEnd - APS_STACK_SIZE (CpusNum);
- } else {
- PhysicalEnd = 0;
- APsStackStartAddress = 0;
- }
-
- Status = AcceptMemory (VmmHobList, CpusNum, APsStackStartAddress, PhysicalEnd);
- ASSERT (Status == EFI_SUCCESS);
-
- return Status;
-}
-
-/**
- In Tdx guest, some information need to be passed from host VMM to guest
- firmware. For example, the memory resource, etc. These information are
- prepared by host VMM and put in HobList which is described in TdxMetadata.
-
- Information in HobList is treated as external input. From the security
- perspective before it is consumed, it should be validated.
-
- @retval EFI_SUCCESS Successfully process the hoblist
- @retval Others Other error as indicated
-**/
-EFI_STATUS
-EFIAPI
-ProcessTdxHobList (
- VOID
- )
-{
- EFI_STATUS Status;
- VOID *TdHob;
- TD_RETURN_DATA TdReturnData;
-
- TdHob = (VOID *)(UINTN)FixedPcdGet32 (PcdOvmfSecGhcbBase);
- Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "Intel Tdx Started with (GPAW: %d, Cpus: %d)\n",
- TdReturnData.TdInfo.Gpaw,
- TdReturnData.TdInfo.NumVcpus
- ));
-
- //
- // Validate HobList
- //
- if (ValidateHobList (TdHob) == FALSE) {
- return EFI_INVALID_PARAMETER;
- }
-
- //
- // Process Hoblist to accept memory
- //
- Status = ProcessHobList (TdHob);
-
- return Status;
-}
-
/**
* Build ResourceDescriptorHob for the unaccepted memory region.
* This memory region may be splitted into 2 parts because of lazy accept.
diff --git a/OvmfPkg/Library/PlatformInitLib/IntelTdxNull.c b/OvmfPkg/Library/PlatformInitLib/IntelTdxNull.c
index 3ebe582af8de..7a7c2fb1f6f5 100644
--- a/OvmfPkg/Library/PlatformInitLib/IntelTdxNull.c
+++ b/OvmfPkg/Library/PlatformInitLib/IntelTdxNull.c
@@ -9,26 +9,6 @@

#include <PiPei.h>

-/**
- In Tdx guest, some information need to be passed from host VMM to guest
- firmware. For example, the memory resource, etc. These information are
- prepared by host VMM and put in HobList which is described in TdxMetadata.
-
- Information in HobList is treated as external input. From the security
- perspective before it is consumed, it should be validated.
-
- @retval EFI_SUCCESS Successfully process the hoblist
- @retval Others Other error as indicated
-**/
-EFI_STATUS
-EFIAPI
-ProcessTdxHobList (
- VOID
- )
-{
- return EFI_UNSUPPORTED;
-}
-
/**
In Tdx guest, the system memory is passed in TdHob by host VMM. So
the major task of PlatformTdxPublishRamRegions is to walk thru the
diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
index 140216979a54..86a82ad3e084 100644
--- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
@@ -52,7 +52,6 @@
PcdLib
PciLib
PeiHardwareInfoLib
- TdxMailboxLib

[LibraryClasses.X64]
TdxLib
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3f970a79a08a..d87013a4422c 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -724,7 +724,8 @@
OvmfPkg/Sec/SecMain.inf {
<LibraryClasses>
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
- NULL|OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
+ BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SecCryptLib.inf
}

#
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 1167d22a68cc..a27dc9406b70 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -29,7 +29,7 @@
#include <Library/CpuExceptionHandlerLib.h>
#include <Ppi/TemporaryRamSupport.h>
#include <Ppi/MpInitLibDep.h>
-#include <Library/PlatformInitLib.h>
+#include <Library/TdxHelperLib.h>
#include <Library/CcProbeLib.h>
#include "AmdSev.h"

@@ -765,7 +765,7 @@ SecCoreStartupWithStack (
// first so that the memory is accepted. Otherwise access to the unaccepted
// memory will trigger tripple fault.
//
- if (ProcessTdxHobList () != EFI_SUCCESS) {
+ if (TdxHelperProcessTdHob () != EFI_SUCCESS) {
CpuDeadLoop ();
}
}
--
2.29.2.windows.2


[PATCH V5 06/13] OvmfPkg: Refactor MeaureFvImage

Min Xu
 

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

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@...>
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/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


[PATCH V5 05/13] OvmfPkg: Refactor MeasureHobList

Min Xu
 

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

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

MeasureHobList once was implemented in PeilessStartupLib and it does
measurement and logging for TdHob in one go, using TpmMeasureAndLogData().
But it doesn't work in SEC.

This patch splits MeasureHobList into 2 functions and implement them in
SecTdxHelperLib.
- TdxHelperMeasureTdHob
- TdxHelperBuildGuidHobForTdxMeasurement

TdxHelperMeasureTdHob measures the TdHob 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 MeasureHobList
- Delete the duplicated codes in PeilessStartupLib

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/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c | 123 ++++++++++++-
.../IntelTdx/TdxHelperLib/SecTdxHelperLib.inf | 1 +
.../IntelTdx/TdxHelperLib/TdxMeasurementHob.c | 173 ++++++++++++++++++
OvmfPkg/Library/PeilessStartupLib/IntelTdx.c | 71 -------
.../PeilessStartupLib/PeilessStartup.c | 12 +-
.../PeilessStartupInternal.h | 15 --
7 files changed, 308 insertions(+), 88 deletions(-)
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 0f1e970fbbb3..920f1c6080d4 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -549,6 +549,7 @@
<LibraryClasses>
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
TpmMeasurementLib|SecurityPkg/Library/SecTpmMeasurementLib/SecTpmMeasurementLibTdx.inf
+ NULL|OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SecCryptLib.inf
HashLib|SecurityPkg/Library/HashLibTdx/HashLibTdx.inf
NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
index ef958b335940..6ca6f01aff57 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
@@ -8,6 +8,31 @@
**/

#include <PiPei.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/BaseCryptLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <IndustryStandard/Tdx.h>
+#include <IndustryStandard/IntelTdx.h>
+#include <IndustryStandard/Tpm20.h>
+#include <Library/TdxLib.h>
+#include <Pi/PrePiHob.h>
+#include <WorkArea.h>
+#include <ConfidentialComputingGuestAttr.h>
+#include <Library/TdxHelperLib.h>
+
+/**
+ 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
+ );

/**
In Tdx guest, some information need to be passed from host VMM to guest
@@ -27,6 +52,58 @@ TdxHelperProcessTdHob (
return EFI_UNSUPPORTED;
}

+/**
+ * Calculate the sha384 of input Data and extend it to RTMR register.
+ *
+ * @param RtmrIndex Index of the RTMR register
+ * @param DataToHash Data to be hashed
+ * @param DataToHashLen Length of the data
+ * @param Digest Hash value of the input data
+ * @param DigestLen Length of the hash value
+ *
+ * @retval EFI_SUCCESS Successfully hash and extend to RTMR
+ * @retval Others Other errors as indicated
+ */
+STATIC
+EFI_STATUS
+HashAndExtendToRtmr (
+ IN UINT32 RtmrIndex,
+ IN VOID *DataToHash,
+ IN UINTN DataToHashLen,
+ OUT UINT8 *Digest,
+ IN UINTN DigestLen
+ )
+{
+ EFI_STATUS Status;
+
+ if ((DataToHash == NULL) || (DataToHashLen == 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((Digest == NULL) || (DigestLen != SHA384_DIGEST_SIZE)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Calculate the sha384 of the data
+ //
+ if (!Sha384HashAll (DataToHash, DataToHashLen, Digest)) {
+ return EFI_ABORTED;
+ }
+
+ //
+ // Extend to RTMR
+ //
+ Status = TdExtendRtmr (
+ (UINT32 *)Digest,
+ SHA384_DIGEST_SIZE,
+ (UINT8)RtmrIndex
+ );
+
+ ASSERT (!EFI_ERROR (Status));
+ return Status;
+}
+
/**
In Tdx guest, TdHob is passed from host VMM to guest firmware and it contains
the information of the memory resource. From the security perspective before
@@ -41,7 +118,47 @@ TdxHelperMeasureTdHob (
VOID
)
{
- return EFI_UNSUPPORTED;
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_STATUS Status;
+ UINT8 Digest[SHA384_DIGEST_SIZE];
+ OVMF_WORK_AREA *WorkArea;
+ VOID *TdHob;
+
+ TdHob = (VOID *)(UINTN)FixedPcdGet32 (PcdOvmfSecGhcbBase);
+ Hob.Raw = (UINT8 *)TdHob;
+
+ //
+ // Walk thru the TdHob list until end of list.
+ //
+ while (!END_OF_HOB_LIST (Hob)) {
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ }
+
+ Status = HashAndExtendToRtmr (
+ 0,
+ (UINT8 *)TdHob,
+ (UINTN)((UINT8 *)Hob.Raw - (UINT8 *)TdHob),
+ 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 the TdHob 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_TDHOB_BITMASK;
+ CopyMem (WorkArea->TdxWorkArea.SecTdxWorkArea.TdxMeasurementsData.TdHobHashValue, Digest, SHA384_DIGEST_SIZE);
+
+ return EFI_SUCCESS;
}

/**
@@ -74,5 +191,9 @@ TdxHelperBuildGuidHobForTdxMeasurement (
VOID
)
{
+ #ifdef TDX_PEI_LESS_BOOT
+ return InternalBuildGuidHobForTdxMeasurement ();
+ #else
return EFI_UNSUPPORTED;
+ #endif
}
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
index 3c6b96f7759a..d17b84c01f20 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
@@ -24,6 +24,7 @@

[Sources]
SecTdxHelper.c
+ TdxMeasurementHob.c

[Packages]
CryptoPkg/CryptoPkg.dec
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c b/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
new file mode 100644
index 000000000000..6cbc7600adb6
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
@@ -0,0 +1,173 @@
+/** @file
+ Build GuidHob for tdx measurement.
+
+ Copyright (c) 2022 - 2023, 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 <WorkArea.h>
+
+#pragma pack(1)
+
+#define HANDOFF_TABLE_DESC "TdxTable"
+typedef struct {
+ UINT8 TableDescriptionSize;
+ UINT8 TableDescription[sizeof (HANDOFF_TABLE_DESC)];
+ UINT64 NumberOfTables;
+ EFI_CONFIGURATION_TABLE TableEntry[1];
+} TDX_HANDOFF_TABLE_POINTERS2;
+
+#pragma pack()
+
+/**
+ * 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;
+}
+
+/**
+ 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;
+ 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;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
index 4e8dca3d7712..ae0ffcc95da5 100644
--- a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
+++ b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
@@ -16,80 +16,9 @@

#include "PeilessStartupInternal.h"

-#pragma pack(1)
-
-#define HANDOFF_TABLE_DESC "TdxTable"
-typedef struct {
- UINT8 TableDescriptionSize;
- UINT8 TableDescription[sizeof (HANDOFF_TABLE_DESC)];
- UINT64 NumberOfTables;
- EFI_CONFIGURATION_TABLE TableEntry[1];
-} TDX_HANDOFF_TABLE_POINTERS2;
-
-#pragma pack()
-
#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
typedef PLATFORM_FIRMWARE_BLOB2_STRUCT CFV_HANDOFF_TABLE_POINTERS2;

-/**
- Measure the Hoblist passed from the VMM.
-
- @param[in] VmmHobList The Hoblist pass the firmware
-
- @retval EFI_SUCCESS Fv image is measured successfully
- or it has been already measured.
- @retval Others Other errors as indicated
-**/
-EFI_STATUS
-EFIAPI
-MeasureHobList (
- IN CONST VOID *VmmHobList
- )
-{
- EFI_PEI_HOB_POINTERS Hob;
- TDX_HANDOFF_TABLE_POINTERS2 HandoffTables;
- EFI_STATUS Status;
-
- if (!TdIsEnabled ()) {
- ASSERT (FALSE);
- return EFI_UNSUPPORTED;
- }
-
- Hob.Raw = (UINT8 *)VmmHobList;
-
- //
- // Parse the HOB list until end of list.
- //
- while (!END_OF_HOB_LIST (Hob)) {
- Hob.Raw = GET_NEXT_HOB (Hob);
- }
-
- //
- // Init the log event for HOB measurement
- //
-
- 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 = (VOID *)VmmHobList;
-
- Status = TpmMeasureAndLogData (
- 1, // PCRIndex
- EV_EFI_HANDOFF_TABLES2, // EventType
- (VOID *)&HandoffTables, // EventData
- sizeof (HandoffTables), // EventSize
- (UINT8 *)(UINTN)VmmHobList, // HashData
- (UINTN)((UINT8 *)Hob.Raw - (UINT8 *)VmmHobList) // HashDataLen
- );
-
- if (EFI_ERROR (Status)) {
- ASSERT (FALSE);
- }
-
- return Status;
-}
-
/**
Get the FvName from the FV header.

diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
index 928120d183ba..4efbc14d5921 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
@@ -17,6 +17,7 @@
#include <Library/PrePiLib.h>
#include <Library/PeilessStartupLib.h>
#include <Library/PlatformInitLib.h>
+#include <Library/TdxHelperLib.h>
#include <ConfidentialComputingGuestAttr.h>
#include <Guid/MemoryTypeInformation.h>
#include <OvmfPlatforms.h>
@@ -179,7 +180,16 @@ PeilessStartup (
//
// Measure HobList
//
- Status = MeasureHobList (VmmHobList);
+ Status = TdxHelperMeasureTdHob ();
+ if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+
+ //
+ // Build GuidHob for tdx measurement
+ //
+ Status = TdxHelperBuildGuidHobForTdxMeasurement ();
if (EFI_ERROR (Status)) {
ASSERT (FALSE);
CpuDeadLoop ();
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
index f56bc3578e5e..a2d2c1c9145b 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
@@ -58,21 +58,6 @@ EFIAPI
ConstructSecHobList (
);

-/**
- Measure the Hoblist passed from the VMM.
-
- @param[in] VmmHobList The Hoblist pass the firmware
-
- @retval EFI_SUCCESS Fv image is measured successfully
- or it has been already measured.
- @retval Others Other errors as indicated
-**/
-EFI_STATUS
-EFIAPI
-MeasureHobList (
- IN CONST VOID *VmmHobList
- );
-
/**
Measure FV image.

--
2.29.2.windows.2


[PATCH V5 04/13] OvmfPkg/PeilessStartupLib: Update the define of FV_HANDOFF_TABLE_POINTERS2

Min Xu
 

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

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

FV_HANDOFF_TABLE_POINTERS2 once was defined in IntelTdx.c. Its structure
is same as PLATFORM_FIRMWARE_BLOB2_STRUCT which is defined in
Library/TcgEventLogRecordLib.h. So this patch reuse the define of
PLATFORM_FIRMWARE_BLOB2_STRUCT as FV_HANDOFF_TABLE_POINTERS2. Furthermore
FV_HANDOFF_TABLE_POINTERS2 is renamed as CFV_HANDOFF_TABLE_POINTERS2
so that the name is more meaningful.

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 | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
index 216c413caad5..4e8dca3d7712 100644
--- a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
+++ b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
@@ -11,6 +11,7 @@
#include <IndustryStandard/UefiTcgPlatform.h>
#include <Library/HobLib.h>
#include <Library/PrintLib.h>
+#include <Library/TcgEventLogRecordLib.h>
#include <Library/TpmMeasurementLib.h>

#include "PeilessStartupInternal.h"
@@ -25,16 +26,11 @@ typedef struct {
EFI_CONFIGURATION_TABLE TableEntry[1];
} TDX_HANDOFF_TABLE_POINTERS2;

-#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
-typedef struct {
- UINT8 BlobDescriptionSize;
- UINT8 BlobDescription[sizeof (FV_HANDOFF_TABLE_DESC)];
- EFI_PHYSICAL_ADDRESS BlobBase;
- UINT64 BlobLength;
-} FV_HANDOFF_TABLE_POINTERS2;
-
#pragma pack()

+#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef PLATFORM_FIRMWARE_BLOB2_STRUCT CFV_HANDOFF_TABLE_POINTERS2;
+
/**
Measure the Hoblist passed from the VMM.

@@ -161,9 +157,9 @@ MeasureFvImage (
IN UINT8 PcrIndex
)
{
- EFI_STATUS Status;
- FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
- VOID *FvName;
+ EFI_STATUS Status;
+ CFV_HANDOFF_TABLE_POINTERS2 FvBlob2;
+ VOID *FvName;

//
// Init the log event for FV measurement
--
2.29.2.windows.2


[PATCH V5 03/13] OvmfPkg/IntelTdx: Add SecTdxHelperLib

Min Xu
 

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

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

TdxHelperLib provides below helper functions for a td-guest.
- TdxHelperProcessTdHob
- TdxHelperMeasureTdHob
- TdxHelperMeasureCfvImage
- TdxHelperBuildGuidHobForTdxMeasurement

SecTdxHelperLib is the SEC instance of TdxHelperLib. It implements 4
functions for tdx in SEC phase:
- TdxHelperProcessTdHob consumes TdHob to accept un-accepted memories.
Before the TdHob is consumed, it is first validated.

- TdxHelperMeasureTdHob measure/extend TdHob and store the measurement
value in workarea.

- TdxHelperMeasureCfvImage measure/extend the Configuration FV image and
store the measurement value in workarea.

- TdxHelperBuildGuidHobForTdxMeasurement builds GuidHob for tdx
measurement.

This patch implements the stubs of the functions. The actual
implementations are in the following patches. Because they are moved from
other files.

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/IntelTdx/TdxHelperLib/SecTdxHelper.c | 78 +++++++++++++++++++
.../IntelTdx/TdxHelperLib/SecTdxHelperLib.inf | 52 +++++++++++++
2 files changed, 130 insertions(+)
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf

diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
new file mode 100644
index 000000000000..ef958b335940
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
@@ -0,0 +1,78 @@
+/** @file
+ TdxHelper Functions which are used in SEC phase
+
+ Copyright (c) 2022 - 2023, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+
+/**
+ In Tdx guest, some information need to be passed from host VMM to guest
+ firmware. For example, the memory resource, etc. These information are
+ prepared by host VMM and put in TdHob which is described in TdxMetadata.
+ TDVF processes the TdHob to accept memories.
+
+ @retval EFI_SUCCESS Successfully process the TdHob
+ @retval Others Other error as indicated
+**/
+EFI_STATUS
+EFIAPI
+TdxHelperProcessTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ In Tdx guest, TdHob is passed from host VMM to guest firmware and it contains
+ the information of the memory resource. From the security perspective before
+ it is consumed, it should be measured and extended.
+ *
+ * @retval EFI_SUCCESS Successfully measure the TdHob
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * In Tdx guest, Configuration FV (CFV) is treated as external input because it
+ * may contain the data provided by VMM. From the sucurity perspective Cfv image
+ * should be measured before it is consumed.
+ *
+ * @retval EFI_SUCCESS Successfully measure the CFV image
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureCfvImage (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ 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
+EFIAPI
+TdxHelperBuildGuidHobForTdxMeasurement (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
new file mode 100644
index 000000000000..3c6b96f7759a
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
@@ -0,0 +1,52 @@
+## @file
+# TdxHelperLib SEC instance
+#
+# This module provides Tdx helper functions in SEC phase.
+# Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = SecTdxHelperLib
+ FILE_GUID = ba69ac6b-0c59-4472-899d-b684590ec1e9
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxHelperLib|SEC
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = X64
+#
+
+[Sources]
+ SecTdxHelper.c
+
+[Packages]
+ CryptoPkg/CryptoPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+ SecurityPkg/SecurityPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseCryptLib
+ DebugLib
+ HobLib
+ PcdLib
+ TdxMailboxLib
+ TdxLib
+
+[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize
+
+[Guids]
+ gCcEventEntryHobGuid
--
2.29.2.windows.2


[PATCH V5 02/13] OvmfPkg/IntelTdx: Add TdxHelperLibNull

Min Xu
 

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

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

TdxHelperLib provides below helper functions for a td-guest.
- TdxHelperProcessTdHob
- TdxHelperMeasureTdHob
- TdxHelperMeasureCfvImage
- TdxHelperBuildGuidHobForTdxMeasurement

TdxHelperLibNull is the NULL instance of TdxHelperLib.

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@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
OvmfPkg/Include/Library/TdxHelperLib.h | 70 ++++++++++++++++
.../TdxHelperLib/TdxHelperLibNull.inf | 32 ++++++++
OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c | 79 +++++++++++++++++++
OvmfPkg/OvmfPkg.dec | 4 +
4 files changed, 185 insertions(+)
create mode 100644 OvmfPkg/Include/Library/TdxHelperLib.h
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c

diff --git a/OvmfPkg/Include/Library/TdxHelperLib.h b/OvmfPkg/Include/Library/TdxHelperLib.h
new file mode 100644
index 000000000000..199aade42f8e
--- /dev/null
+++ b/OvmfPkg/Include/Library/TdxHelperLib.h
@@ -0,0 +1,70 @@
+/** @file
+ TdxHelperLib header file
+
+ Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef TDX_HELPER_LIB_H
+#define TDX_HELPER_LIB_H
+
+#include <PiPei.h>
+
+/**
+ In Tdx guest, some information need to be passed from host VMM to guest
+ firmware. For example, the memory resource, etc. These information are
+ prepared by host VMM and put in TdHob which is described in TdxMetadata.
+ TDVF processes the TdHob to accept memories.
+
+ @retval EFI_SUCCESS Successfully process the TdHob
+ @retval Others Other error as indicated
+**/
+EFI_STATUS
+EFIAPI
+TdxHelperProcessTdHob (
+ VOID
+ );
+
+/**
+ In Tdx guest, TdHob is passed from host VMM to guest firmware and it contains
+ the information of the memory resource. From the security perspective before
+ it is consumed, it should be measured and extended.
+ *
+ * @retval EFI_SUCCESS Successfully measure the TdHob
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureTdHob (
+ VOID
+ );
+
+/**
+ * In Tdx guest, Configuration FV (CFV) is treated as external input because it
+ * may contain the data provided by VMM. From the sucurity perspective Cfv image
+ * should be measured before it is consumed.
+ *
+ * @retval EFI_SUCCESS Successfully measure the CFV image
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureCfvImage (
+ VOID
+ );
+
+/**
+ 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
+EFIAPI
+TdxHelperBuildGuidHobForTdxMeasurement (
+ VOID
+ );
+
+#endif
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf b/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
new file mode 100644
index 000000000000..27d07b3886cf
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
@@ -0,0 +1,32 @@
+## @file
+# TdxHelperLib NULL instance
+#
+# Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxHelperLibNull
+ FILE_GUID = 853603b2-53ea-463d-93e6-35d09a79e358
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxHelperLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = X64
+#
+
+[Sources]
+ TdxHelperNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseLib
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c b/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c
new file mode 100644
index 000000000000..a2125190d6aa
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c
@@ -0,0 +1,79 @@
+/** @file
+ NULL instance of TdxHelperLib
+
+ Copyright (c) 2022 - 2023, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <PiPei.h>
+
+/**
+ In Tdx guest, some information need to be passed from host VMM to guest
+ firmware. For example, the memory resource, etc. These information are
+ prepared by host VMM and put in TdHob which is described in TdxMetadata.
+ TDVF processes the TdHob to accept memories.
+
+ @retval EFI_SUCCESS Successfully process the TdHob
+ @retval Others Other error as indicated
+**/
+EFI_STATUS
+EFIAPI
+TdxHelperProcessTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ In Tdx guest, TdHob is passed from host VMM to guest firmware and it contains
+ the information of the memory resource. From the security perspective before
+ it is consumed, it should be measured and extended.
+ *
+ * @retval EFI_SUCCESS Successfully measure the TdHob
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureTdHob (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * In Tdx guest, Configuration FV (CFV) is treated as external input because it
+ * may contain the data provided by VMM. From the sucurity perspective Cfv image
+ * should be measured before it is consumed.
+ *
+ * @retval EFI_SUCCESS Successfully measure the CFV image
+ * @retval Others Other error as indicated
+ */
+EFI_STATUS
+EFIAPI
+TdxHelperMeasureCfvImage (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ 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
+EFIAPI
+TdxHelperBuildGuidHobForTdxMeasurement (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index a22eb246c625..be30547474ae 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -98,6 +98,10 @@
#
SerializeVariablesLib|Include/Library/SerializeVariablesLib.h

+ ## @libraryclass TdxHelper
+ #
+ TdxHelperLib|Include/Library/TdxHelperLib.h
+
## @libraryclass Declares utility functions for virtio device drivers.
VirtioLib|Include/Library/VirtioLib.h

--
2.29.2.windows.2


[PATCH V5 01/13] OvmfPkg: Add Tdx measurement data structure in WorkArea

Min Xu
 

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

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

From the perspective of security any external input should be measured
and extended to some registers (TPM PCRs or TDX RTMR registers).

There are below 2 external input in a Td guest:
- TdHob
- Configuration FV (CFV)

TdHob contains the resource information passed from VMM, such as
unaccepted memory region. CFV contains the configurations, such as
secure boot variables.

TdHob and CFV should be measured and extended to RTMRs before they're
consumed. TdHob is consumed in the very early stage of boot process.
At that moment the memory service is not ready. Cfv is consumed in
PlatformPei to initialize the EmuVariableNvStore. To make the
implementation simple and clean, these 2 external input are measured
and extended to RTMRs in SEC phase. That is to say the tdx measurement
is only supported in SEC phase.

After the measurement the hash values are stored in WorkArea. Then after
the Hob service is available, these 2 measurement values are retrieved
and GuidHobs for these 2 tdx measurements are generated.

This patch defines the structure of TDX_MEASUREMENTS_DATA in
SEC_TDX_WORK_AREA to store above 2 tdx measurements. It can be extended
to store more tdx measurements if needed in the future.

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@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
OvmfPkg/Include/WorkArea.h | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index 6c3702b716f0..b1c7045ce18c 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -11,6 +11,7 @@
#define __OVMF_WORK_AREA_H__

#include <ConfidentialComputingGuestAttr.h>
+#include <IndustryStandard/Tpm20.h>

//
// Confidential computing work area header definition. Any change
@@ -64,13 +65,27 @@ typedef struct _SEV_WORK_AREA {
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
} SEV_WORK_AREA;

+//
+// Start of TDX Specific WorkArea definition
+//
+
+#define TDX_MEASUREMENT_TDHOB_BITMASK 0x1
+#define TDX_MEASUREMENT_CFVIMG_BITMASK 0x2
+
+typedef struct _TDX_MEASUREMENTS_DATA {
+ UINT32 MeasurementsBitmap;
+ UINT8 TdHobHashValue[SHA384_DIGEST_SIZE];
+ UINT8 CfvImgHashValue[SHA384_DIGEST_SIZE];
+} TDX_MEASUREMENTS_DATA;
+
//
// The TDX work area definition
//
typedef struct _SEC_TDX_WORK_AREA {
- UINT32 PageTableReady;
- UINT32 Gpaw;
- UINT64 HobList;
+ UINT32 PageTableReady;
+ UINT32 Gpaw;
+ UINT64 HobList;
+ TDX_MEASUREMENTS_DATA TdxMeasurementsData;
} SEC_TDX_WORK_AREA;

typedef struct _TDX_WORK_AREA {
@@ -78,6 +93,10 @@ typedef struct _TDX_WORK_AREA {
SEC_TDX_WORK_AREA SecTdxWorkArea;
} TDX_WORK_AREA;

+//
+// End of TDX Specific WorkArea definition
+//
+
typedef union {
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER Header;
SEV_WORK_AREA SevWorkArea;
--
2.29.2.windows.2


[PATCH V5 00/13] Enable Tdx measurement in OvmfPkgX64

Min Xu
 

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

Tdx measurement (RTMR based measurement) is enabled in OvmfPkg/IntelTdx.
This patch-set enables the feature in OvmfPkgX64 as well.

Patch #1:
Introduce TDX_MEASUREMETNS_DATA in SEC_TDX_WORK_AREA. That is because
the RTMR measurement of TdHob and Configuration FV (CFV) are executed
in very early stage of boot process. At that time the memory service is
not ready and the measurement values have to be stored in OvmfWorkArea.

Patch #2:
Introduce TdxHelperLibNull which is the NULL instance of TdxHelperLib.

Patch #3:
Introduce SecTdxHelperLib which is the instance of TdxHelperLib for SEC
Phase. This patch adds the stubs of TdxHelperLib functions. The actual
implementation are in the following patches.

Patch #4:
Re-use the data struct of PLATFORM_FIRMWARE_BLOB2_STRUCT for
FV_HANDOFF_TABLE_POINTERS2.

Patch #5-7:
These 3 patches move the functions ( which were implemented in
PeilessStartupLib and PlatformInitLib ) to TdxHelperLib. So that they
can be called in both OvmfPkgX64 and IntelTdxX64.

Patch #8/9:
These 2 patches are the changes for tdx measurement in IntelTdxX64.

Patch #10-13:
These 4 patches are the changes for OvmfPkgX64 to enable Tdx
measurement.

Code: https://github.com/mxu9/edk2/tree/TdxMeasurementInOvmfX64.v5

v5 changes:
- Re-organize the patches. Its purpose is not only to simplify review, but also
to simplify testing. https://edk2.groups.io/g/devel/message/99209

v4 changes:
- To make the code reviewable, the implementation of
TdxHelperBuildGuidHobForTdxMeasurement is split into 4 patches (5-8).
- Call Sha384HashAll instead of the 3 Sha384XXX functions so that we
need to allocate memory in SEC phase.

v3 changes:
- Use the definition of PLATFORM_FIRMWARE_BLOB2_STRUCT in
Library/TcgEventLogRecordLib.h.
- Rename TDX_ENABLE as TDX_MEASUREMENT_ENABLE because this flag is
introduced for Tdx-measurement.
- Split the patch of SecTdxHelperLib into 2 separate patches (#3/#9).
Patch#3 implements TdxHelperMeasureTdHob and TdxHelperMeasureCfvImage.
Patch#9 implements TdxHelperProcessTdHob. This is to make the patches
more reviewable. The duplicated codes of TdxHelperProcessTdHob are
deleted in Patch#9 as well.
- The implementation of TdxHelperBuildGuidHobForTdxMeasurement and update
of PeilessStartupLib are in one patch (#5). Because the implmentation
of TdxHelperBuildGuidHobForTdxMeasurement was once in PeilessStartupLib.

v2 changes:
- Split the patch of TdxHelperLib into 4 separate patches. So that it is
more reviewable.
- Add commit message in Patch#1 to emphasize that the tdx-measurement in
OvmfPkgX64 is supported in SEC phase.

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@...>

Min M Xu (13):
OvmfPkg: Add Tdx measurement data structure in WorkArea
OvmfPkg/IntelTdx: Add TdxHelperLibNull
OvmfPkg/IntelTdx: Add SecTdxHelperLib
OvmfPkg/PeilessStartupLib: Update the define of
FV_HANDOFF_TABLE_POINTERS2
OvmfPkg: Refactor MeasureHobList
OvmfPkg: Refactor MeaureFvImage
OvmfPkg: Refactor ProcessHobList
OvmfPkg/IntelTdx: Measure TdHob and Configuration FV in SecMain
OvmfPkg/PeilessStartupLib: Delete the duplicated tdx measurement
OvmfPkg/IntelTdx: Add PeiTdxHelperLib
OvmfPkg/OvmfPkgX64: Measure TdHob and Configuration FV in SecMain
OvmfPkg/PlatformPei: Build GuidHob for Tdx measurement
OvmfPkg: Support Tdx measurement in OvmfPkgX64

OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/CloudHv/CloudHvX64.dsc | 5 +-
OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc | 10 +-
.../Include/Dsc/OvmfTpmSecurityStub.dsc.inc | 8 +
OvmfPkg/Include/Library/PlatformInitLib.h | 17 -
OvmfPkg/Include/Library/TdxHelperLib.h | 70 ++
OvmfPkg/Include/WorkArea.h | 25 +-
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 4 +-
OvmfPkg/IntelTdx/Sec/SecMain.c | 17 +-
OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c | 91 +++
.../IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf | 48 ++
.../TdxHelperLib/SecTdxHelper.c} | 304 +++----
.../IntelTdx/TdxHelperLib/SecTdxHelperLib.inf | 53 ++
.../TdxHelperLib/TdxHelperLibNull.inf | 32 +
OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c | 79 ++
.../IntelTdx/TdxHelperLib/TdxMeasurementHob.c | 259 ++++++
OvmfPkg/Library/PeilessStartupLib/IntelTdx.c | 196 -----
.../PeilessStartupLib/PeilessStartup.c | 16 +-
.../PeilessStartupInternal.h | 36 -
.../PeilessStartupLib/PeilessStartupLib.inf | 6 -
OvmfPkg/Library/PlatformInitLib/IntelTdx.c | 768 ------------------
.../Library/PlatformInitLib/IntelTdxNull.c | 20 -
.../PlatformInitLib/PlatformInitLib.inf | 1 -
OvmfPkg/Microvm/MicrovmX64.dsc | 5 +-
OvmfPkg/OvmfPkg.dec | 4 +
OvmfPkg/OvmfPkgX64.dsc | 20 +-
OvmfPkg/OvmfPkgX64.fdf | 7 +
OvmfPkg/PlatformPei/IntelTdx.c | 3 +
OvmfPkg/Sec/SecMain.c | 17 +-
29 files changed, 915 insertions(+), 1211 deletions(-)
create mode 100644 OvmfPkg/Include/Library/TdxHelperLib.h
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelper.c
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/PeiTdxHelperLib.inf
copy OvmfPkg/{Library/PlatformInitLib/IntelTdx.c => IntelTdx/TdxHelperLib/SecTdxHelper.c} (80%)
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperLibNull.inf
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxHelperNull.c
create mode 100644 OvmfPkg/IntelTdx/TdxHelperLib/TdxMeasurementHob.c
delete mode 100644 OvmfPkg/Library/PeilessStartupLib/IntelTdx.c

--
2.29.2.windows.2


Re: [PATCH V4 06/12] OvmfPkg/PeilessStartupLib: Build GuidHob for Tdx measurement

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:
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.
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


Re: [PATCH] ShellPkg/AcpiView: ERST Parser

Gao, Zhichao
 

Can you share then code change link of your personal repo? The patch I download from this email cannot apply to the open source branch.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeshua
Smith via groups.io
Sent: Wednesday, January 25, 2023 4:55 AM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>
Subject: Re: [edk2-devel] [PATCH] ShellPkg/AcpiView: ERST Parser

Thanks for the review. What is the next step to get this merged?

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@...>
Sent: Sunday, January 15, 2023 5:59 PM
To: Jeshua Smith <jeshuas@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>
Subject: RE: [PATCH] ShellPkg/AcpiView: ERST Parser

External email: Use caution opening links or attachments


Reviewed-by: Zhichao Gao <zhichao.gao@...>

Thanks,
Zhichao

-----Original Message-----
From: Jeshua Smith <jeshuas@...>
Sent: Thursday, December 1, 2022 2:31 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Gao, Zhichao <zhichao.gao@...>;
Jeshua Smith <jeshuas@...>
Subject: [PATCH] ShellPkg/AcpiView: ERST Parser

Add a new parser for the Error Record Serialization Table.

The ERST table describes how an OS can save and retrieve

hardware error information to and from a persistent store.



Signed-off-by: Jeshua Smith <jeshuas@...>

---

.../UefiShellAcpiViewCommandLib/AcpiParser.h | 22 ++

.../Parsers/Erst/ErstParser.c | 278 ++++++++++++++++++

.../UefiShellAcpiViewCommandLib.c | 2 +

.../UefiShellAcpiViewCommandLib.inf | 2 +

4 files changed, 304 insertions(+)

create mode 100644
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c



diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h

index db8c88f6df..66c992c55c 100644

--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h

+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h

@@ -1,6 +1,7 @@

/** @file

Header file for ACPI parser



+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.

Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.

Copyright (c) 2022, AMD Incorporated. All rights reserved.

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -594,6 +595,27 @@ ParseAcpiDsdt (

IN UINT8 AcpiTableRevision

);



+/**

+ This function parses the ACPI ERST table.

+ When trace is enabled this function parses the ERST table and

+ traces the ACPI table fields.

+

+ This function also performs validation of the ACPI table fields.

+

+ @param [in] Trace If TRUE, trace the ACPI fields.

+ @param [in] Ptr Pointer to the start of the buffer.

+ @param [in] AcpiTableLength Length of the ACPI table.

+ @param [in] AcpiTableRevision Revision of the ACPI table.

+**/

+VOID

+EFIAPI

+ParseAcpiErst (

+ IN BOOLEAN Trace,

+ IN UINT8 *Ptr,

+ IN UINT32 AcpiTableLength,

+ IN UINT8 AcpiTableRevision

+ );

+

/**

This function parses the ACPI FACS table.

When trace is enabled this function parses the FACS table and

diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser
.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser
.c

new file mode 100644

index 0000000000..e2af0c44b4

--- /dev/null

+++
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser
.c

@@ -0,0 +1,278 @@

+/** @file

+ ERST table parser

+

+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.

+ Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.

+ SPDX-License-Identifier: BSD-2-Clause-Patent

+

+ @par Reference(s):

+ - ACPI 6.5 Specification - August 2022

+**/

+

+#include <IndustryStandard/Acpi.h>

+#include <Library/UefiLib.h>

+#include "AcpiParser.h"

+#include "AcpiTableParser.h"

+

+// Local variables

+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;

+STATIC UINT32 *InstructionEntryCount;

+

+/**

+ An array of strings describing the Erst actions

+**/

+STATIC CONST CHAR16 *ErstActionTable[] = {

+ L"BEGIN_WRITE_OPERATION",

+ L"BEGIN_READ_OPERATION",

+ L"BEGIN_CLEAR_OPERATION",

+ L"END_OPERATION",

+ L"SET_RECORD_OFFSET",

+ L"EXECUTE_OPERATION",

+ L"CHECK_BUSY_STATUS",

+ L"GET_COMMAND_STATUS",

+ L"GET_RECORD_IDENTIFIER",

+ L"SET_RECORD_IDENTIFIER",

+ L"GET_RECORD_COUNT",

+ L"BEGIN_DUMMY_WRITE_OPERATION",

+ L"RESERVED",

+ L"GET_ERROR_LOG_ADDRESS_RANGE",

+ L"GET_ERROR_LOG_ADDRESS_RANGE_LENGTH",

+ L"GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES",

+ L"GET_EXECUTE_OPERATION_TIMINGS"

+};

+

+/**

+ An array of strings describing the Erst instructions

+**/

+STATIC CONST CHAR16 *ErstInstructionTable[] = {

+ L"READ_REGISTER",

+ L"READ_REGISTER_VALUE",

+ L"WRITE_REGISTER",

+ L"WRITE_REGISTER_VALUE",

+ L"NOOP",

+ L"LOAD_VAR1",

+ L"LOAD_VAR2",

+ L"STORE_VAR1",

+ L"ADD",

+ L"SUBTRACT",

+ L"ADD_VALUE",

+ L"SUBTRACT_VALUE",

+ L"STALL",

+ L"STALL_WHILE_TRUE",

+ L"SKIP_NEXT_INSTRUCTION_IF_TRUE",

+ L"GOTO",

+ L"SET_SRC_ADDRESS_BASE",

+ L"SET_DST_ADDRESS_BASE",

+ L"MOVE_DATA"

+};

+

+/**

+ Validate Erst action.

+

+ @param [in] Ptr Pointer to the start of the field data.

+ @param [in] Context Pointer to context specific information e.g. this

+ could be a pointer to the ACPI table header.

+**/

+STATIC

+VOID

+EFIAPI

+ValidateErstAction (

+ IN UINT8 *Ptr,

+ IN VOID *Context

+ )

+{

+ if (*Ptr > EFI_ACPI_6_4_ERST_GET_EXECUTE_OPERATION_TIMINGS) {

+ IncrementErrorCount ();

+ Print (L"\nError: 0x%02x is not a valid action encoding", *Ptr);

+ }

+}

+

+/**

+ Validate Erst instruction.

+

+ @param [in] Ptr Pointer to the start of the field data.

+ @param [in] Context Pointer to context specific information e.g. this

+ could be a pointer to the ACPI table header.

+**/

+STATIC

+VOID

+EFIAPI

+ValidateErstInstruction (

+ IN UINT8 *Ptr,

+ IN VOID *Context

+ )

+{

+ if (*Ptr > EFI_ACPI_6_4_ERST_MOVE_DATA) {

+ IncrementErrorCount ();

+ Print (L"\nError: 0x%02x is not a valid instruction encoding",
+ *Ptr);

+ }

+}

+

+/**

+ Validate Erst flags.

+

+ @param [in] Ptr Pointer to the start of the field data.

+ @param [in] Context Pointer to context specific information e.g. this

+ could be a pointer to the ACPI table header.

+**/

+STATIC

+VOID

+EFIAPI

+ValidateErstFlags (

+ IN UINT8 *Ptr,

+ IN VOID *Context

+ )

+{

+ if ((*Ptr & 0xfe) != 0) {

+ IncrementErrorCount ();

+ Print (L"\nError: Reserved Flag bits not set to 0");

+ }

+}

+

+/**

+ Looks up and prints the string corresponding to the index

+

+ @param [in] Table Lookup table

+ @param [in] Index Entry to print

+ @param [in] NumEntries Number of valid entries in the table

+**/

+STATIC

+VOID

+EFIAPI

+FormatByte (

+ IN CONST CHAR16 *Table[],

+ IN UINT8 Index,

+ IN UINT8 NumEntries

+ )

+{

+ CONST CHAR16 *String;

+

+ if (Index < NumEntries) {

+ String = Table[Index];

+ } else {

+ String = L"**INVALID**";

+ }

+

+ Print (

+ L"0x%02x (%s)",

+ Index,

+ String

+ );

+}

+

+/**

+ Prints the Erst Action

+

+ @param [in] Format Optional format string for tracing the data.

+ @param [in] Ptr Pointer to the Action byte

+**/

+STATIC

+VOID

+EFIAPI

+DumpErstAction (

+ IN CONST CHAR16 *Format OPTIONAL,

+ IN UINT8 *Ptr

+ )

+{

+ FormatByte (ErstActionTable, *Ptr, ARRAY_SIZE (ErstActionTable));

+}

+

+/**

+ Prints the Erst Instruction

+

+ @param [in] Format Optional format string for tracing the data.

+ @param [in] Ptr Pointer to the Instruction byte

+**/

+STATIC

+VOID

+EFIAPI

+DumpErstInstruction (

+ IN CONST CHAR16 *Format OPTIONAL,

+ IN UINT8 *Ptr

+ )

+{

+ FormatByte (ErstInstructionTable, *Ptr, ARRAY_SIZE
+ (ErstInstructionTable));

+}

+

+/**

+ An ACPI_PARSER array describing the ACPI ERST Table.

+**/

+STATIC CONST ACPI_PARSER ErstParser[] = {

+ PARSE_ACPI_HEADER (&AcpiHdrInfo),

+ { L"Serialization Header Size", 4, 36, L"0x%x", NULL, NULL,
NULL, NULL },

+ { L"Reserved", 4, 40, L"0x%x", NULL, NULL, NULL,
NULL },

+ { L"Instruction Entry Count", 4, 44, L"0x%x", NULL, (VOID
**)&InstructionEntryCount, NULL, NULL }

+};

+

+/**

+ An ACPI_PARSER array describing the Serialization Instruction Entry
structure.

+**/

+STATIC CONST ACPI_PARSER SerializationInstructionEntryParser[] = {

+ { L"Serialization Action", 1, 0, L"0x%x", DumpErstAction, NULL,
ValidateErstAction, NULL },

+ { L"Instruction", 1, 1, L"0x%x", DumpErstInstruction, NULL,
ValidateErstInstruction, NULL },

+ { L"Flags", 1, 2, L"0x%x", NULL, NULL, ValidateErstFlags,
NULL },

+ { L"Reserved", 1, 3, L"0x%x", NULL, NULL, NULL,
NULL },

+ { L"Register Region", 12, 4, NULL, DumpGas, NULL, NULL,
NULL },

+ { L"Value", 8, 16, L"0x%llx", NULL, NULL, NULL,
NULL },

+ { L"Mask", 8, 24, L"0x%llx", NULL, NULL, NULL,
NULL }

+};

+

+/**

+ This function parses the ACPI ERST table.

+ When trace is enabled this function parses the ERST table and

+ traces the ACPI table fields.

+

+ This function also performs validation of the ACPI table fields.

+

+ @param [in] Trace If TRUE, trace the ACPI fields.

+ @param [in] Ptr Pointer to the start of the buffer.

+ @param [in] AcpiTableLength Length of the ACPI table.

+ @param [in] AcpiTableRevision Revision of the ACPI table.

+**/

+VOID

+EFIAPI

+ParseAcpiErst (

+ IN BOOLEAN Trace,

+ IN UINT8 *Ptr,

+ IN UINT32 AcpiTableLength,

+ IN UINT8 AcpiTableRevision

+ )

+{

+ UINT32 Offset;

+

+ if (!Trace) {

+ return;

+ }

+

+ Offset = ParseAcpi (

+ Trace,

+ 0,

+ "ERST",

+ Ptr,

+ AcpiTableLength,

+ PARSER_PARAMS (ErstParser)

+ );

+

+ if (sizeof
(EFI_ACPI_6_4_ERST_SERIALIZATION_INSTRUCTION_ENTRY)*(*InstructionE
nt
ryCount) != (AcpiTableLength - Offset)) {

+ IncrementErrorCount ();

+ Print (

+ L"ERROR: Invalid InstructionEntryCount. " \

+ L"Count = %d. Offset = %d. AcpiTableLength = %d.\n",

+ *InstructionEntryCount,

+ Offset,

+ AcpiTableLength

+ );

+ return;

+ }

+

+ while (Offset < AcpiTableLength) {

+ Offset += ParseAcpi (

+ Trace,

+ 2,

+ "Serialization Action",

+ Ptr + Offset,

+ (AcpiTableLength - Offset),

+ PARSER_PARAMS (SerializationInstructionEntryParser)

+ );

+ }

+}

diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.c

index 09bdddb56e..d37ad7cacc 100644

---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.c

+++
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.c

@@ -1,6 +1,7 @@

/** @file

Main file for 'acpiview' Shell command function.



+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.

Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent

**/

@@ -52,6 +53,7 @@ ACPI_TABLE_PARSER ParserList[] = {

{ EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
ParseAcpiDbg2 },


{ EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATUR
E,

ParseAcpiDsdt },

+ { EFI_ACPI_6_4_ERROR_RECORD_SERIALIZATION_TABLE_SIGNATURE,
ParseAcpiErst },

{ EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
ParseAcpiFacs },

{ EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
ParseAcpiFadt },

{ EFI_ACPI_6_4_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
ParseAcpiGtdt },

diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.inf
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.inf

index 63fc5a1281..904fea83de 100644

---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.inf

+++
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
a
ndLib.inf

@@ -1,6 +1,7 @@

## @file

# Provides Shell 'acpiview' command functions

#

+# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.

# Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -31,6 +32,7 @@

Parsers/Bgrt/BgrtParser.c

Parsers/Dbg2/Dbg2Parser.c

Parsers/Dsdt/DsdtParser.c

+ Parsers/Erst/ErstParser.c

Parsers/Facs/FacsParser.c

Parsers/Fadt/FadtParser.c

Parsers/Gtdt/GtdtParser.c

--

2.25.1





Re: [PATCH v1 0/4] Don't require self-signed PK in setup mode

Sean
 

Did i mention how much i hate reviewing code over email?  :)

Even after looking at it a few times I missed that change.

I think this change looks fine.  Personally, i think this code has terrible readability and high complexity and with high impact.  It would be great to at least get unit tests if not a full refactor of the library.  I also think the edk2 idea of "custom mode" should be removed.  But that feedback isn't for this patch and I don't think this patch should be held up just because the surrounding code is less than ideal.

For patch 1 and 4.

Reviewed-by: Sean Brogan <sean.brogan@...>

Thanks

Sean

On 1/27/2023 2:05 PM, Jan Bobek wrote:
I read your replacement a little different.

- if ((InCustomMode () && UserPhysicalPresent ()) || ((mPlatformMode == SETUP_MODE) && !IsPk)) {

with

+ if ( (InCustomMode () && UserPhysicalPresent ())
+ || ( (mPlatformMode == SETUP_MODE)
+ && !(FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk)))
+ {


In the upper part you replaced it says !IsPk. What am i missing?


If a payload was in this function targeting a KEK change with no PK
installed it would go in the original IF condition. In the new code
it would because device is not in custom mode and the payload is not
targeted PK.
Is it possible that you're missing the negation (i.e. exclamation mark)
in front of the parethesis? The old code was

!IsPk

The new code is

!(FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk)

If IsPk is FALSE, both of these evaluate to TRUE no matter what the PCD
is.

-Jan

On 1/25/2023 1:38 PM, Jan Bobek wrote:
Hi Sean,

From looking over the patch 1/4 email i have a concern.

In AuthService.c on the conditional change you made. Aren't we losing
a case where we are evaluating a nonPK payload signed by the PK?
Given the system is in SetupMode that means there is no PK but is this
the conditional path that is used when installing Secure boot keys in
reverse (DBX,DX,KEK,PK) order?
Thanks for sharing your concern! They way I think about the change is
that I've replaced the expression

IsPk

with

FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk

and nothing else. When you look at it this way, it's fairly easy to
break down how it affects the logic:

1. Assume PcdRequireSelfSignedPk is TRUE. In this case, the two
expressions are exactly the same and no change in behavior occurs,
just as we want.

2. Assume IsPk is FALSE. In this case, both expressions evaluate to
FALSE, no matter what the PCD is configured to. That's also good,
because we don't want to change how non-PK payloads are handled.

3. In fact, the only change in behavior that occurs is when
PcdRequireSelfSignedPk is FALSE and IsPk is TRUE; here the former
expression would be TRUE, whereas the latter is FALSE. That's exactly
what we want: we wish to enter the first block of the if-else branch
(which changes the variable similarly to when we're in custom mode)
rather than falling through to the third block (which checks the
self-signature).

To directly answer your question, I don't think the behavior changes at
all when processing non-PK payloads, by virtue of IsPk being FALSE and
what I said in point (2.) above.

Additionally, yes, the first block of the if-else branch is exactly the
path that's taken when enrolling KEK/DB/DBX in Setup mode, and one that
has always been available even without Custom mode. It used to be that
you couldn't use this path to enroll PK without Custom mode (precisely
because of !IsPk in the condition), but I'm hoping to enable this path
with my patch.

Let me know if I haven't answered or misunderstood your question.

Is there testing you have done? This code should be pretty easy to do
host based unit testing on. Any chance you have authored that to
confirm use cases are not unexpectedly impacted? Example of host
based unit test of library is here:
edk2/SecurityPkg/Library/SecureBootVariableLib/UnitTest at master ·
tianocore/edk2
(github.com)<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Ftree%2Fmaster%2FSecurityPkg%2FLibrary%2FSecureBootVariableLib%2FUnitTest&data=05%7C01%7Cjbobek%40nvidia.com%7Cf2eff15ce44945cc0b4e08db00ad6625%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638104516992386171%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=djptmpLUwESbPeqFwFUz5sVbS1MBnD%2FPua9H2y1nxFE%3D&reserved=0>
I've done an ad-hoc test by commenting out the switch to/from Custom
mode in EnrollFromDefaultKeysApp from SecurityPkg, booting in Setup mode
and using the modified app to enroll the keys. You could do a similar
test from the OS, but in my case this was more straightforward.

I am aware of the host-based unit testing library in edk2, and I agree
that it would be great to have the code in AuthVariableLib tested for
all the different cases. However, I don't have any such tests right now,
and while I'm willing to potentially look into writing some, I'd have to
do it more or less on the side, meaning it could take a while.

Best,
-Jan

On 1/22/2023 10:13 PM, Yao, Jiewen wrote:

Hi Sean
I would like to hear your feedback, since it is a little different from the original MSFT patch.

Would you please take a look?

Thank you
Yao, Jiewen



-----Original Message-----
From: Jan Bobek <jbobek@...><mailto:jbobek@...>
Sent: Saturday, January 21, 2023 6:59 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Jan Bobek <jbobek@...><mailto:jbobek@...>; Laszlo Ersek <lersek@...><mailto:lersek@...>; Yao,
Jiewen <jiewen.yao@...><mailto:jiewen.yao@...>
Subject: [PATCH v1 0/4] Don't require self-signed PK in setup mode

Hi all,

I'm sending out v1 of my patch series that addresses a UEFI spec
non-compliance when enrolling PK in setup mode. Additional info can be
found in bugzilla [1]; the changes are split into 4 patches as
suggested by Laszlo Ersek in comment #4.

I've based my work on the patch by Matthew Carlson; I've credited him
with co-authorship of the first patch even though in the end I decided
to do the implementation a bit differently.

Comments & reviews welcome!

Cheers,
-Jan

References:
1. https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2506&data=05%7C01%7Cjbobek%40nvidia.com%7Cf2eff15ce44945cc0b4e08db00ad6625%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638104516992386171%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PXam5BVxMmUwgRdGsq1RcnwC8yb8nmOzcLz3oKica7s%3D&reserved=0

Jan Bobek (4):
SecurityPkg: limit verification of enrolled PK in setup mode
OvmfPkg: require self-signed PK when secure boot is enabled
ArmVirtPkg: require self-signed PK when secure boot is enabled
SecurityPkg: don't require PK to be self-signed by default

SecurityPkg/SecurityPkg.dec | 7 +++++++
ArmVirtPkg/ArmVirtCloudHv.dsc | 4 ++++
ArmVirtPkg/ArmVirtQemu.dsc | 4 ++++
ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ++++
OvmfPkg/Bhyve/BhyveX64.dsc | 3 +++
OvmfPkg/CloudHv/CloudHvX64.dsc | 3 +++
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 +++
OvmfPkg/Microvm/MicrovmX64.dsc | 3 +++
OvmfPkg/OvmfPkgIa32.dsc | 3 +++
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
OvmfPkg/OvmfPkgX64.dsc | 3 +++
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 3 +++
SecurityPkg/Library/AuthVariableLib/AuthService.c | 9 +++++++--
13 files changed, 50 insertions(+), 2 deletions(-)


[PATCH v1 1/1] StandaloneMmPkg: StandaloneMmMemLib: Change max address computation

Girish Mahadevan
 

Currently the standalonemmlibinternal assumes the max physical bits
to be 36 which is causing issues on v8 architectures.
Instead use the MAX_ADDRESS macro to determine the maximum allowed address
rather than recomputing it locally.

Signed-off-by: Girish Mahadevan <gmahadevan@...>
---
.../ArmStandaloneMmMemLibInternal.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c b/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
index 297cfae916..4dc392b4e3 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
@@ -20,13 +20,6 @@
//
extern EFI_PHYSICAL_ADDRESS mMmMemLibInternalMaximumSupportAddress;

-#ifdef MDE_CPU_AARCH64
-#define ARM_PHYSICAL_ADDRESS_BITS 36
-#endif
-#ifdef MDE_CPU_ARM
-#define ARM_PHYSICAL_ADDRESS_BITS 32
-#endif
-
/**
Calculate and save the maximum support address.

@@ -36,14 +29,8 @@ MmMemLibInternalCalculateMaximumSupportAddress (
VOID
)
{
- UINT8 PhysicalAddressBits;
-
- PhysicalAddressBits = ARM_PHYSICAL_ADDRESS_BITS;
+ mMmMemLibInternalMaximumSupportAddress = MAX_ADDRESS;

- //
- // Save the maximum support address in one global variable
- //
- mMmMemLibInternalMaximumSupportAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, PhysicalAddressBits) - 1);
DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress = 0x%lx\n", mMmMemLibInternalMaximumSupportAddress));
}

--
2.17.1


Re: [PATCH 2/2] DynamicTablesPkg: Add Smbios Type17 Table generator

Girish Mahadevan
 

Hi Sami

Sorry for the long silence. I've sent you v2 of the patch (only framework not the Type17) which includes a link to a github branch.

Few more comments inline , prefixed by [GM].

Best Regards
Girish

On 10/18/2022 9:36 AM, Sami Mujawar via groups.io wrote:
*External email: Use caution opening links or attachments*
Hi Girish,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 04/10/2022 11:43 pm, Girish Mahadevan via groups.io wrote:
Hello Sami

Thank you so much for your review, I apologize for the late response.

My comment in line about the handle manager [GM].

Best Regards
Girish

On 9/12/2022 8:57 AM, Sami Mujawar wrote:
External email: Use caution opening links or attachments


Hi Girish,

Thank you for this patch and for the effort for bringing forward dynamic
SMBIOS generation.

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar

On 26/08/2022 06:37 pm, Girish Mahadevan wrote:
Add a new CM object to describe memory devices and setup a new
Generator Library for SMBIOS Type17 table.

Signed-off-by: Girish Mahadevan<gmahadevan@...>
---
  .../Include/ArmNameSpaceObjects.h             |  59 +++
  .../SmbiosType17Lib/SmbiosType17Generator.c   | 338 ++++++++++++++++++
  .../SmbiosType17Lib/SmbiosType17LibArm.inf    |  32 ++
  3 files changed, 429 insertions(+)
  create mode 100644 DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17Generator.c
  create mode 100644 DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 102e0f96be..199a19e997 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -63,6 +63,7 @@ typedef enum ArmObjectID {
    EArmObjPciInterruptMapInfo,          ///< 39 - Pci Interrupt Map Info
    EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
    EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
+  EArmObjMemoryDeviceInfo,             ///< 42 - Memory Device Information
    EArmObjMax
  } EARM_OBJECT_ID;

@@ -1070,6 +1071,64 @@ typedef struct CmArmRmrDescriptor {
    UINT64    Length;
  } CM_ARM_MEMORY_RANGE_DESCRIPTOR;

+/** A structure that describes the physical memory device.
+
+  The physical memory devices on the system are described by this object.
+
+  SMBIOS Specification v3.5.0 Type17
+
+  ID: EArmObjMemoryDeviceInfo,
+*/
+typedef struct CmArmMemoryDeviceInfo {
[SAMI] I think we may need a Token pointing to the Type 16 object so
that the Physical Memory Array Handle can be setup, see my comment below
about the HandleManager.
+  /** Size of the device.
+    Size of the device in bytes.
+  */
+  UINT64  Size;
+
+  /** Device Set */
+  UINT8   DeviceSet;
+
+  /** Speed of the device
+    Speed of the device in MegaTransfers/second.
+  */
+  UINT32  Speed;
+
+  /** Serial Number of device  */
+  CHAR8   *SerialNum;
+
+  /** AssetTag identifying the device */
+  CHAR8   *AssetTag;
+
+  /** Device Locator String for the device.
+   String that describes the slot or position of the device on the board.
+   */
+  CHAR8   *DeviceLocator;
+
+  /** Bank locator string for the device.
+   String that describes the bank where the device is located.
+   */
+  CHAR8   *BankLocator;
+
+  /** Firmware version of the memory device */
+  CHAR8   *FirmwareVersion;
+
+  /** Manufacturer Id.
+   2 byte Manufacturer Id as per JEDEC Standard JEP106AV
+  */
+  UINT16  ModuleManufacturerId;
+
+  /** Manufacturer Product Id
+   2 byte Manufacturer Id as designated by Manufacturer.
+  */
+  UINT16  ModuleProductId;
+
+  /** Device Attributes */
+  UINT8   Attributes;
+
+  /** Device Configured Voltage in millivolts */
+  UINT16  ConfiguredVoltage;
[SAMI] This field does not appear to be used in the generator. If the
intention is to use this in the future, then it may be better to add
this at a later stage.
+} CM_ARM_MEMORY_DEVICE_INFO;
+
  #pragma pack()

  #endif // ARM_NAMESPACE_OBJECTS_H_
diff --git a/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17Generator.c b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17Generator.c
new file mode 100644
index 0000000000..5683ca570f
--- /dev/null
+++ b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17Generator.c
@@ -0,0 +1,338 @@
+/** @file
+  SMBIOS Type17 Table Generator.
+
+  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PrintLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/SmbiosType17FixupLib.h>
[SAMI] I could not find SmbiosType17FixupLib.h in this patch series. Can
you check, please?
+
+// Module specific include files.
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+#include <Protocol/Smbios.h>
[SAMI] I think Protocol/Smbios.h may not be required in this file. Can
you check, please?
+#include <IndustryStandard/SmBios.h>
+
+/** This macro expands to a function that retrieves the Memory Device
+    information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjMemoryDeviceInfo,
+  CM_ARM_MEMORY_DEVICE_INFO
+  )
+
+// Default Values for Memory Device
+STATIC SMBIOS_TABLE_TYPE17 MemDeviceInfoTemplate = {
+  {                                      // Hdr
+    EFI_SMBIOS_TYPE_MEMORY_DEVICE,       // Type
+    sizeof (SMBIOS_TABLE_TYPE17),        // Length
+    0                                    // Handle
+  },
+  0,                                     // MemoryArrayHandle
[SAMI] Do you have any thoughts on how the MemoryArrayHandle can be setup?

The same applies for the following MemoryErrorInformationHandle field.

I think we need some sort of a HandleManager in DynamicTablesFramework
that can keep track of the mappings between SMBIOS Objects and Table
Handles.

e.g. Smbios - HandleManager

+-------------------------------+-------------------------------+

       |    Object Token               | Table Handle                  |

+-------------------------------+-------------------------------+

       | Type16Obj_token         | Type 16 Table handle    |

+-------------------------------+-------------------------------+

       ...

- The Type17Object i.e. CM_ARM_MEMORY_DEVICE_INFO can then hold a token
for the Type16Object.

  - If Type 17 table is to be installed, DynamicTablemanager shall
search the SMBIOS table list to see if a Type16 table is requested to be
installed.

- If a Type16 table is present in the list of SMBIOS table to install,
the Type16 table shall be installed first and an entry is made in the
Smbios HandleManager to create a mapping of Type16Obj_token  <==> Type16
Table Handle.

- The Type17 table can now be built and if a the Type16Object token is
provided in CM_ARM_MEMORY_DEVICE_INFO, the Smbios HandleManager shall be
searched (using Type16Obj_token) to retrieve the Type16 Table handle and
populate the Type 17 Physical Memory Array Handle field.

I think we may have to experiment a bit before we arrive at the correct
design. However, please do let me know your thoughts on the above.
[GM] I agree with the idea of having a map of CM_OBJECT_TOKENs to SMBIOS handles. I've added this to the DynamicTableFactoryProtocol.

However when it comes to actually figuring out and adding the reference SMBIOS handle We've come up with a slightly different approach.

If I understood what you were saying above is:
 If we happen to parse a Type17 table
   if that Type17 table has a token to a Physical memory array CM_OBJ
     if there is an existing Smbios Handle (look up this handle using
                                             the CM_OBJECT_TOKEN)
         then use this handle.
     else if there is a generator for Type16 registered
         call the Type16 generator code first

IMO this gets a bit difficult to manage. Right now the DynamicTableManagerDxe walks the array of EStdObjSmbiosTableList CM objects, finds the generator for each Table, invokes it, gets an SMBIOS record that it then installs via the SmbiosDxe driver.
It seemed a bit convoluted to call the generator and install an SMBIOS table while within the generator of another Smbios table.
And if there are layers of dependencies (or multiple dependencies) it could add to the complexities.

Instead what we came up with is:

If we happen to parse a Type17 table
  if that Type17 table has a token to a Physical memory array CM_OBJ
     if there is an existing Smbios Handle (look up this handle using
                                             the CM_OBJECT_TOKEN )
         then use this handle.
     else if there is a generator for Type16 Registered
           Generate an SMBIOS handle [since SmbiosDxe maintains the
                                      handle DB privately I had to
                                      pick a handle and check if it
                                      clashes with existing records]
           Add this SMBIOS handle to the map.
     else // No generator present
          Proceed without adding any reference


Before Adding any SMBIOS table, we check if there is an existing SMBIOS handle for the CM_OBJECT_TOKEN (the generator will return a CM_OBJECT_TOKEN for each SMBIOS record created).
If there is an existing SMBIOS handle, pass that in, else pass in SMBIOS_HANDLE_PI_RESERVED and let SmbiosDxe assign the handle.

Here is a sample implementation of this approach (it is a WIP, but I wanted to get your thoughts on it)

https://github.com/tianocore/edk2/compare/master...gmahadevan:edk2-upstream:RFC/smbios-dyntables
[SAMI] I had a look at the above branch and have the following suggestions:
a. To resolve the dependency order, please see my patches for SMBIOS table dispatcher at: https://edk2.groups.io/g/devel/message/95340
[GM]
Thanks !
I've setup a new patch/github branch that includes the dispatcher code and the SMBIOS string helper patch.

The SMBIOS string helper and dispatcher code look good to me.

You should be able to apply these patches on your 'edk2-upstream:RFC/smbios-dyntables' branch and enable the dispatcher in ProcessSmbiosTables().
e.g. In file DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c, the SMBIOS table dispatcher can be invoked once it is initialised as shown below.
@@ -1007,19 +1007,24 @@ ProcessSmbiosTables (
     SmbiosTableCount
     ));
+  // Initialise the SMBIOS Table Dispatcher state machine.
+  InitSmbiosTableDispatcher (SmbiosTableInfo, SmbiosTableCount);
+
   for (Idx = 0; Idx < SmbiosTableCount; Idx++) {
-    Status = BuildAndInstallSmbiosTable (
+    Status = DispatchSmbiosTable (
+               SmbiosTableInfo[Idx].TableType,
                TableFactoryProtocol,
                CfgMgrProtocol,
                SmbiosProtocol,
-               &SmbiosTableInfo[Idx]
+               SmbiosTableInfo,
+               SmbiosTableCount
                );
     if (EFI_ERROR (Status)) {
       DEBUG ((
         DEBUG_ERROR,
-        "ERROR: Failed to install SMBIOS Table." \
-        " Id = %u Status = %r\n",
-        SmbiosTableInfo[Idx].TableGeneratorId,
+        "ERROR: Failed to dispatch SMBIOS Table for installation." \
+        " Table Type = %u Status = %r\n",
+        SmbiosTableInfo[Idx].TableType,
         Status
         ));
     }
b. With the SMBIOS dispatcher patch and the handle manager, it should be possible to update the handles for dependent tables. This should remove the need for the handle generation and management.
c. With regards to the SMBIOS handle manager, I think it would be better to add CM_OBJECT_ID for the SmbiosCmTokenin SMBIOS_HANDLE_MAP.
d. A new SMBIOS object namespace should be defined.
    e.g Namespace ID 1010b or 1100b - SMBIOS Objects, see https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/ConfigurationManagerObject.h#L30-L34
   With this change, CM_ARM_MEMORY_DEVICE_INFO becomes CM_SMBIOS_MEMORY_DEVICE_INFO
    Similarly, EArmObjMemoryDeviceInfo becomes ESmbiosObjMemoryDeviceInfo
e. With regards to DynamicTableManager.c, I think we should split the ACPI & SMBIOS processing in individual files (e.g. AcpiBuilder.c & SmbiosBuilder.c) under DynamicTableManagerDxe.
f. Status appears to be returned uninitialised in DynamicTableManagerDxeInitialize().
g. DynamicTablesPkg\Library\Smbios\Arm\SmbiosType17Lib can be moved to DynamicTablesPkg\Library\Smbios\SmbiosType17Lib.
I prefer a github branch with the patch as it is very helpful to view the code being reviewed. However, it is difficult to provide feedback. Is it possible to submit a patch for the changes with the link for the guthub branch in the future, please?
[GM]
I've sent a new version of the patch minus the Type17 code as I wanted to just focus on the framework code first.
I've incorporated the comments from above except for the suggestion on using CM_OBJECT_ID instead of the CM_TOKEN for the SmbiosHandleMap.
IMO the CM_TOKEN is better since we can have unique tokens for cases where a single SMBIOS CM_OBJECT_ID can have multiple entries (type17/type9 etc), let me know your thoughts.

Best Regards
Girish

I am not sure if we need an edk2-staging branch for this feature. But if you think it would be helpful then please let me know and I will send out a request to create a new feature branch.
[/SAMI]

Sorry for the long message, please let me know your thoughts.

[/GM]

[SAMI]
+  0,                                     // MemoryErrorInformationHandle
+  0xFFFF,                                // TotalWidth
+  0xFFFF,                                // DataWIdth
[SAMI] I need to find out how these fields should be populated, but the
Annex A, SMBIOS specification version 3.6.0 says the following:

4.8.4 Total Width is not 0FFFFh (Unknown) if the memory device is
installed. (Size is not 0.)

4.8.5 Data Width is not 0FFFFh (Unknown).

Can you explain how this field is used, please?

[/SAMI]

+  0x7FFF,                                // Size (always use Extended Size)
[SAMI] I think this field should be set based on the Size.

The spec says "If the size is 32 GB-1 MB or greater, the field value is
7FFFh and the actual size is stored in the Extended Size field."

I think it would be good to have a static function  that encodes the
size in wither the Size field or the Extended Size field.

e.g. UpdateSmbiosTable17Size (SMBIOS_TABLE_TYPE17 *SmbiosRecord, UINTN
Size) {

          if (Size > 32GB-1MB) {

             SmbiosRecord->Size = EncodeSize (xxx);

           } else {

              SmbiosRecord->ExtendedSize = EncodeExtendedSize (xxx);

          }

     }

[/SAMI]

+  MemoryTypeUnknown,                     // FormFactor
+  0xFF,                                  // DeviceSet
+  1,                                     // Device Locator
+  2,                                     // Bank Locator
+  MemoryTypeSdram,                       // MemoryType
+  {                                      // TypeDetail
+    0
+  },
+  0xFFFF,                                // Speed (Use Extended Speed)
[SAMI] Maybe we need a helper function (similar to
UpdateSmbiosTable17Size()) for this field as well.
+  0,                                     // Manufacturer
+                                         // (Unused Use ModuleManufactuerId)
+  3,                                     // SerialNumber
+  4,                                     // AssetTag
+  0,                                     // PartNumber
+                                         // (Unused Use ModuleProductId)
+  0,                                     // Attributes
+  0,                                     // ExtendedSize
+  2000,                                  // ConfiguredMemoryClockSpeed
[SAMI] Should this be provided through CM_ARM_MEMORY_DEVICE_INFO?
+  0,                                     // MinimumVoltage
+  0,                                     // MaximumVoltage
+  0,                                     // ConfiguredVoltage
+  MemoryTechnologyDram,                  // MemoryTechnology
[SAMI] Should this be provided through CM_ARM_MEMORY_DEVICE_INFO?
+  {                                      // MemoryOperatingModeCapability
+    .Uint16 = 0x000
+  },
[SAMI] I think the above initialisation may not be portable.
+  5,                                    // FirmwareVersion
+  0,                                    // ModuleManufacturerId
+  0,                                    // ModuleProductId
+  0,                                    // MemorySubsystemContollerManufacturerId
+  0,                                    // MemorySybsystemControllerProductId
+  0,                                    // NonVolatileSize
+  0,                                    // VolatileSize
+  0,                                    // CacheSize
+  0,                                    // LogicalSize
+  0,                                    // ExtendedSpeed
+  0,                                    // ExtendedConfiguredMemorySpeed
+};
+
+STATIC CHAR8  UnknownStr[] = "Unknown\0";
[SAMI] Would it be possible to add the CONST qualifier, please?
+
+STATIC
+EFI_STATUS
+EFIAPI
+FreeSmbiosType17TableEx (
+  IN      CONST SMBIOS_TABLE_GENERATOR                   *CONST This,
+  IN      CONST CM_STD_OBJ_SMBIOS_TABLE_INFO             *CONST SmbiosTableInfo,
+  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL     *CONST CfgMgrProtocol,
+  IN OUT        SMBIOS_STRUCTURE                         ***CONST Table,
+  IN      CONST UINTN TableCount
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/** Construct SMBIOS Type17 Table describing memory devices.
+
+  If this function allocates any resources then they must be freed
+  in the FreeXXXXTableResources function.
+
+  @param [in]  This            Pointer to the SMBIOS table generator.
+  @param [in]  SmbiosTableInfo Pointer to the SMBIOS table information.
+  @param [in]  CfgMgrProtocol  Pointer to the Configuration Manager
+                               Protocol interface.
+  @param [out] Table           Pointer to the SMBIOS table.
+
+  @retval EFI_SUCCESS            Table generated successfully.
+  @retval EFI_BAD_BUFFER_SIZE    The size returned by the Configuration
+                                 Manager is less than the Object size for
+                                 the requested object.
+  @retval EFI_INVALID_PARAMETER  A parameter is invalid.
+  @retval EFI_NOT_FOUND          Could not find information.
+  @retval EFI_OUT_OF_RESOURCES   Could not allocate memory.
+  @retval EFI_UNSUPPORTED        Unsupported configuration.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildSmbiosType17TableEx (
+  IN  CONST SMBIOS_TABLE_GENERATOR                         *This,
+  IN        CM_STD_OBJ_SMBIOS_TABLE_INFO           *CONST SmbiosTableInfo,
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL   *CONST CfgMgrProtocol,
+  OUT       SMBIOS_STRUCTURE                               ***Table,
+  OUT       UINTN                                  *CONST  TableCount
+  )
+{
+  EFI_STATUS                 Status;
+  UINT32                     NumMemDevices;
+  SMBIOS_STRUCTURE           **TableList;
+  CM_ARM_MEMORY_DEVICE_INFO  *MemoryDevicesInfo;
+  UINTN                      Index;
+  UINTN                      SerialNumLen;
+  CHAR8                      *SerialNum;
+  UINTN                      AssetTagLen;
+  CHAR8                      *AssetTag;
+  UINTN                      DeviceLocatorLen;
+  CHAR8                      *DeviceLocator;
+  UINTN                      BankLocatorLen;
+  CHAR8                      *BankLocator;
+  UINTN                      FirmwareVersionLen;
+  CHAR8                      *FirmwareVersion;
+  CHAR8                      *OptionalStrings;
+  SMBIOS_TABLE_TYPE17        *SmbiosRecord;
+
+  ASSERT (This != NULL);
+  ASSERT (SmbiosTableInfo != NULL);
+  ASSERT (CfgMgrProtocol != NULL);
+  ASSERT (Table != NULL);
+  ASSERT (TableCount != NULL);
+  ASSERT (SmbiosTableInfo->TableGeneratorId == This->GeneratorID);
+
+  DEBUG ((DEBUG_ERROR, "%a : Start \n", __FUNCTION__));
+  *Table = NULL;
+  Status = GetEArmObjMemoryDeviceInfo (
+             CfgMgrProtocol,
+             CM_NULL_TOKEN,
+             &MemoryDevicesInfo,
+             &NumMemDevices
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to get Memory Devices CM Object %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  TableList = (SMBIOS_STRUCTURE **)AllocateZeroPool (sizeof (SMBIOS_STRUCTURE *) * NumMemDevices);
[SAMI] The memory allocated here should be freed in
FreeSmbiosType17TableEx.
+  if (TableList == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to alloc memory for %u devices table\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto exit;
+  }
+
+  for (Index = 0; Index < NumMemDevices; Index++) {
+    if (MemoryDevicesInfo[Index].SerialNum != NULL) {
+      SerialNumLen = AsciiStrLen (MemoryDevicesInfo[Index].SerialNum);
+      SerialNum    = MemoryDevicesInfo[Index].SerialNum;
+    } else {
+      SerialNumLen = AsciiStrLen (UnknownStr);
+      SerialNum    = UnknownStr;
[SAMI] If the serial number is not provided, then should the string
field be set to 0?

See Section 6.1.3, SMBIOS Spec Version 3.6.0 which states

"If a string field references no string, a null (0) is placed in that
string field."

[/SAMI]

+    }
+
+    if (MemoryDevicesInfo[Index].AssetTag != NULL) {
+      AssetTagLen = AsciiStrLen (MemoryDevicesInfo[Index].AssetTag);
+      AssetTag    = MemoryDevicesInfo[Index].AssetTag;
+    } else {
+      AssetTagLen = AsciiStrLen (UnknownStr);
+      AssetTag    = UnknownStr;
+    }
+
+    if (MemoryDevicesInfo[Index].DeviceLocator != NULL) {
+      DeviceLocatorLen = AsciiStrLen (MemoryDevicesInfo[Index].DeviceLocator);
+      DeviceLocator    = MemoryDevicesInfo[Index].DeviceLocator;
+    } else {
+      DeviceLocatorLen = AsciiStrLen (UnknownStr);
+      DeviceLocator    = UnknownStr;
+    }
+
+    if (MemoryDevicesInfo[Index].BankLocator != NULL) {
+      BankLocatorLen = AsciiStrLen (MemoryDevicesInfo[Index].BankLocator);
+      BankLocator    = MemoryDevicesInfo[Index].BankLocator;
+    } else {
+      BankLocatorLen = AsciiStrLen (UnknownStr);
+      BankLocator    = UnknownStr;
+    }
+
+    if (MemoryDevicesInfo[Index].FirmwareVersion != NULL) {
+      FirmwareVersionLen = AsciiStrLen (MemoryDevicesInfo[Index].FirmwareVersion);
+      FirmwareVersion    = MemoryDevicesInfo[Index].FirmwareVersion;
+    } else {
+      FirmwareVersionLen = AsciiStrLen (UnknownStr);
+      FirmwareVersion    = UnknownStr;
+    }
+
+    SmbiosRecord = (SMBIOS_TABLE_TYPE17 *)AllocateZeroPool (
+                                            sizeof (SMBIOS_TABLE_TYPE17) +
+                                            SerialNumLen + 1 +
+                                            AssetTagLen + 1 + DeviceLocatorLen + 1 +
+                                            BankLocatorLen + 1 + FirmwareVersionLen + 1 + 1
+                                            );
[SAMI] The memory allocated here needs to be freed in
FreeSmbiosType17TableEx as  SmbiosProtocol->Add () makes a copy of the
SmbiosTable, see
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FMdeModulePkg%2FUniversal%2FSmbiosDxe%2FSmbiosDxe.c%23L476&;data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=EnTxn07ekjA7bGUeCg2c0BaUMVW0yU5JFjXNOcZuhQA%3D&amp;reserved=0
and
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FMdeModulePkg%2FUniversal%2FSmbiosDxe%2FSmbiosDxe.c%23L516&;data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=d6kU6CdywK4fnOdxE8CyTTM9eQHDE38FzZB7SA2FTqc%3D&amp;reserved=0.

+    if (SmbiosRecord == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      goto exit;
+    }
+
+    CopyMem (SmbiosRecord, &MemDeviceInfoTemplate, sizeof (SMBIOS_TABLE_TYPE17));
+    SmbiosRecord->ExtendedSize         = MemoryDevicesInfo[Index].Size;
[SAMI] CM_ARM_MEMORY_DEVICE_INFO.Size is documented as size in bytes,
while looking at the SMBIOS specification, section 7.18.5 for the
Extended Size Bits 30:0 represent the size of the memory device in
megabytes.

I think it will be useful to create UpdateSmbiosTable17Size() which does
the appropriate encoding and updation of the SMBIOS table fieds.

[/SAMI]

+    SmbiosRecord->DeviceSet            = MemoryDevicesInfo[Index].DeviceSet;
+    SmbiosRecord->ModuleManufacturerID =
+      MemoryDevicesInfo[Index].ModuleManufacturerId;
+    SmbiosRecord->ModuleProductID =
+      MemoryDevicesInfo[Index].ModuleProductId;
+    SmbiosRecord->Attributes =
+      MemoryDevicesInfo[Index].Attributes;
+    SmbiosRecord->ExtendedSpeed = MemoryDevicesInfo[Index].Speed;
+    OptionalStrings             = (CHAR8 *)(SmbiosRecord + 1);
+    AsciiSPrint (OptionalStrings, DeviceLocatorLen + 1, DeviceLocator);
[SAMI] I think we can simplify the publishing of the SMBIOS strings
using SmbiosStringTableLib. Please see the patch at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F93651&;data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=4xHMP2KfNlcdeBrtH%2FIT1F249uWIoz0XZreF3FSugq8%3D&amp;reserved=0
+    OptionalStrings = OptionalStrings + DeviceLocatorLen + 1;
+    AsciiSPrint (OptionalStrings, BankLocatorLen + 1, BankLocator);
+    OptionalStrings = OptionalStrings + BankLocatorLen + 1;
+    AsciiSPrint (OptionalStrings, SerialNumLen + 1, SerialNum);
+    OptionalStrings = OptionalStrings + SerialNumLen + 1;
+    AsciiSPrint (OptionalStrings, AssetTagLen + 1, AssetTag);
+    OptionalStrings = OptionalStrings + AssetTagLen + 1;
+    AsciiSPrint (OptionalStrings, FirmwareVersionLen + 1, FirmwareVersion);
+    OptionalStrings  = OptionalStrings + FirmwareVersionLen + 1;
+    TableList[Index] = (SMBIOS_STRUCTURE *)SmbiosRecord;
+  }
+
+  *Table      = TableList;
+  *TableCount = NumMemDevices;
+
+exit:
+  DEBUG ((DEBUG_ERROR, "%a : Done \n", __FUNCTION__));
+  return Status;
+}
+
+/** The interface for the SMBIOS Type17 Table Generator.
+*/
+STATIC
+CONST
+SMBIOS_TABLE_GENERATOR  SmbiosType17Generator = {
+  // Generator ID
+  CREATE_STD_SMBIOS_TABLE_GEN_ID (EStdSmbiosTableIdType17),
+  // Generator Description
+  L"SMBIOS.TYPE17.GENERATOR",
+  // SMBIOS Table Type
+  EFI_SMBIOS_TYPE_MEMORY_DEVICE,
+  NULL,
+  NULL,
+  // Build table function Extended.
+  BuildSmbiosType17TableEx,
+  // Free function Extended.
+  FreeSmbiosType17TableEx
+};
+
+/** Register the Generator with the SMBIOS Table Factory.
+
+  @param [in]  ImageHandle  The handle to the image.
+  @param [in]  SystemTable  Pointer to the System Table.
+
+  @retval EFI_SUCCESS           The Generator is registered.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_ALREADY_STARTED   The Generator for the Table ID
+                                is already registered.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosType17LibConstructor (
+  IN  EFI_HANDLE        ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = RegisterSmbiosTableGenerator (&SmbiosType17Generator);
+  DEBUG ((
+    DEBUG_INFO,
+    "SMBIOS Type 17: Register Generator. Status = %r\n",
+    Status
+    ));
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
+/** Deregister the Generator from the SMBIOS Table Factory.
+
+  @param [in]  ImageHandle  The handle to the image.
+  @param [in]  SystemTable  Pointer to the System Table.
+
+  @retval EFI_SUCCESS           The Generator is deregistered.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The Generator is not registered.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosType17LibDestructor (
+  IN  EFI_HANDLE        ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = DeregisterSmbiosTableGenerator (&SmbiosType17Generator);
+  DEBUG ((
+    DEBUG_INFO,
+    "SMBIOS Type17: Deregister Generator. Status = %r\n",
+    Status
+    ));
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
diff --git a/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf
new file mode 100644
index 0000000000..78a80b75f0
--- /dev/null
+++ b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf
@@ -0,0 +1,32 @@
+## @file
+# SMBIOS Type17 Table Generator
+#
+#  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = SmbiosType17LibArm
+  FILE_GUID      = 1f063bac-f8f1-4e08-8ffd-9aae52c75497
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER
+  CONSTRUCTOR    = SmbiosType17LibConstructor
+  DESTRUCTOR     = SmbiosType17LibDestructor
+
+[Sources]
+  SmbiosType17Generator.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib




Re: [PATCH] MdeModulePkg: Correct memory type in PrePiDxeCis.h

Dionna Glaze
 

BTW, you need to add below guys in the CC list because they're the reviewers/maintainers of the pkgs.

MdePkg:
M: Michael D Kinney <michael.d.kinney@...> [mdkinney]
M: Liming Gao <gaoliming@...> [lgao4]
R: Zhiguang Liu <zhiguang.liu@...> [LiuZhiguang001]

MdeModulePkg
M: Jian J Wang <jian.j.wang@...> [jwang36]
M: Liming Gao <gaoliming@...> [lgao4]
CC'd, thanks.

--
-Dionna Glaze, PhD (she/her)


[PATCH v2 1/1] DynamicTablesPkg: Add SMBIOS table generation

Girish Mahadevan
 

Add the SMBIOS Table generator code to the DynamicTablesPkg.

This change includes adding new logic to the DynamicTableManager
to process and add SMBIOS tables and augmenting the existing SMBIOS
Factory generator to include installing multiple SMBIOS tables .
Also included is running the SMBIOS and ACPI table generation as part
of the SMBIOS and ACPI protocol GUID callback notifications respectively.

The change can be seen at
https://github.com/gmahadevan/edk2-upstream/commits/RFC/smbios-dyntables

Signed-off-by: Girish Mahadevan <gmahadevan@...>
Reviewed-by: Jeff Brasen <jbrasen@...>
---
.../DynamicTableFactory.h | 5 +
.../DynamicTableFactoryDxe.c | 7 +
.../SmbiosTableFactory/SmbiosTableFactory.c | 66 ++
.../DynamicTableManagerDxe/AcpiTableBuilder.c | 732 ++++++++++++++++
.../DynamicTableManagerDxe.c | 794 +-----------------
.../DynamicTableManagerDxe.inf | 5 +-
.../SmbiosTableBuilder.c | 608 ++++++++++++++
.../Include/ConfigurationManagerObject.h | 18 +-
.../Protocol/DynamicTableFactoryProtocol.h | 8 +
.../Include/SmbiosTableGenerator.h | 165 +++-
10 files changed, 1653 insertions(+), 755 deletions(-)
create mode 100644 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/AcpiTableBuilder.c
create mode 100644 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c

diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory.h b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory.h
index b160dcf8ad..20e438ea70 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory.h
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory.h
@@ -49,6 +49,11 @@ typedef struct DynamicTableFactoryInfo {
CustomDtTableGeneratorList[FixedPcdGet16 (
PcdMaxCustomDTGenerators
)];
+
+ /// An array for holding a map of SMBIOS handles and the CM Object
+ /// token used to build the SMBIOS record.
+ SMBIOS_HANDLE_MAP
+ SmbiosHandleMap[MAX_SMBIOS_HANDLES];
} EDKII_DYNAMIC_TABLE_FACTORY_INFO;

/** Return a pointer to the ACPI table generator.
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
index 6d6d3fa746..d0ee5d7b3b 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
@@ -44,6 +44,8 @@ EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL DynamicTableFactoryProtocol = {
GetDtTableGenerator,
RegisterDtTableGenerator,
DeregisterDtTableGenerator,
+ AddSmbiosHandle,
+ FindSmbiosHandle,
&TableFactoryInfo
};

@@ -65,7 +67,12 @@ DynamicTableFactoryDxeInitialize (
)
{
EFI_STATUS Status;
+ UINTN Idx;

+ for (Idx = 0; Idx < MAX_SMBIOS_HANDLES; Idx++) {
+ TableFactoryInfo.SmbiosHandleMap[Idx].SmbiosTblHandle = SMBIOS_HANDLE_PI_RESERVED;
+ TableFactoryInfo.SmbiosHandleMap[Idx].SmbiosCmToken = 0;
+ }
Status = gBS->InstallProtocolInterface (
&ImageHandle,
&gEdkiiDynamicTableFactoryProtocolGuid,
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c
index 87795919f8..b81724d2a0 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c
@@ -12,6 +12,7 @@
#include <IndustryStandard/SmBios.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
#include <Library/DebugLib.h>

// Module specific include files.
@@ -24,6 +25,71 @@

extern EDKII_DYNAMIC_TABLE_FACTORY_INFO TableFactoryInfo;

+/** Add a new entry to the SMBIOS table Map.
+
+ @param [in] Smbios SMBIOS Protocol pointer.
+ @param [in] SmbiosHandle SMBIOS Handle to be added, if the value SMBIOS_HANDLE_PI_RESERVED
+ is passed then a new SmbiosHandle is assigned.
+ @param [in] CmObjectToken CmObjectToken of the CM_OBJECT used to build the SMBIOS Table
+ @param [in] SmbiosId Smbios Table Generator Id.
+
+ @retval EFI_SUCCESS Successfully added/generated the handle.
+ @retval EFI_OUT_OF_RESOURCESNULL Failure to add/generate the handle.
+**/
+EFI_STATUS
+EFIAPI
+AddSmbiosHandle (
+ IN EFI_SMBIOS_PROTOCOL *Smbios,
+ IN SMBIOS_HANDLE *SmbiosHandle,
+ IN CM_OBJECT_TOKEN CmObjectToken,
+ IN ESTD_SMBIOS_TABLE_ID SmbiosId
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+
+ Status = EFI_OUT_OF_RESOURCES;
+
+ for (Index = 0; Index < MAX_SMBIOS_HANDLES; Index++) {
+ if (TableFactoryInfo.SmbiosHandleMap[Index].SmbiosTblHandle == SMBIOS_HANDLE_PI_RESERVED) {
+ TableFactoryInfo.SmbiosHandleMap[Index].SmbiosTblHandle = *SmbiosHandle;
+ TableFactoryInfo.SmbiosHandleMap[Index].SmbiosCmToken = CmObjectToken;
+ TableFactoryInfo.SmbiosHandleMap[Index].SmbiosTableId = SmbiosId;
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+
+ return Status;
+}
+
+/** Return a pointer to the SMBIOS table Map.
+
+ @param [in] GeneratorId The CmObjectToken to look up an SMBIOS Handle.
+
+ @retval SMBIOS_HANDLE_MAP if the CmObjectToken is found.
+ @retval NULL if not found.
+**/
+SMBIOS_HANDLE_MAP *
+EFIAPI
+FindSmbiosHandle (
+ CM_OBJECT_TOKEN CmObjectToken
+ )
+{
+ UINTN Index;
+ SMBIOS_HANDLE_MAP *SmbiosHandleMap;
+
+ SmbiosHandleMap = NULL;
+ for (Index = 0; Index < MAX_SMBIOS_HANDLES; Index++) {
+ if (TableFactoryInfo.SmbiosHandleMap[Index].SmbiosCmToken == CmObjectToken) {
+ SmbiosHandleMap = &TableFactoryInfo.SmbiosHandleMap[Index];
+ break;
+ }
+ }
+
+ return SmbiosHandleMap;
+}
+
/** Return a pointer to the SMBIOS table generator.

@param [in] This Pointer to the Dynamic Table Factory Protocol.
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/AcpiTableBuilder.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/AcpiTableBuilder.c
new file mode 100644
index 0000000000..5021eab748
--- /dev/null
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/AcpiTableBuilder.c
@@ -0,0 +1,732 @@
+/** @file
+ Dynamic Table Manager Dxe
+
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2022 - 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
+#include <Library/BaseMemoryLib.h>
+
+// Module specific include files.
+#include <AcpiTableGenerator.h>
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <DeviceTreeTableGenerator.h>
+#include <Library/TableHelperLib.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+#include <Protocol/DynamicTableFactoryProtocol.h>
+
+/** This macro expands to a function that retrieves the ACPI Table
+ List from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceStandard,
+ EStdObjAcpiTableList,
+ CM_STD_OBJ_ACPI_TABLE_INFO
+ )
+
+/** A helper function to build and install a single ACPI table.
+
+ This is a helper function that invokes the Table generator interface
+ for building an ACPI table. It uses the AcpiTableProtocol to install the
+ table, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] Generator Pointer to the AcpiTable generator.
+ @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
+ @param [in] AcpiTableInfo Pointer to the ACPI table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildAndInstallSingleAcpiTable (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST ACPI_TABLE_GENERATOR *CONST Generator,
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
+ )
+{
+ EFI_STATUS Status;
+ EFI_STATUS Status1;
+ EFI_ACPI_DESCRIPTION_HEADER *AcpiTable;
+ UINTN TableHandle;
+
+ AcpiTable = NULL;
+ Status = Generator->BuildAcpiTable (
+ Generator,
+ AcpiTableInfo,
+ CfgMgrProtocol,
+ &AcpiTable
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Build Table." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ if (AcpiTable == NULL) {
+ Status = EFI_NOT_FOUND;
+ goto exit_handler;
+ }
+
+ // Dump ACPI Table Header
+ DUMP_ACPI_TABLE_HEADER (AcpiTable);
+
+ // Install ACPI table
+ Status = AcpiTableProtocol->InstallAcpiTable (
+ AcpiTableProtocol,
+ AcpiTable,
+ AcpiTable->Length,
+ &TableHandle
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Install ACPI Table. Status = %r\n",
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: ACPI Table installed. Status = %r\n",
+ Status
+ ));
+
+exit_handler:
+ // Free any resources allocated for generating the tables.
+ if (Generator->FreeTableResources != NULL) {
+ Status1 = Generator->FreeTableResources (
+ Generator,
+ AcpiTableInfo,
+ CfgMgrProtocol,
+ &AcpiTable
+ );
+ if (EFI_ERROR (Status1)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Free Table Resources." \
+ "TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status1
+ ));
+ }
+
+ // Return the first error status in case of failure
+ if (!EFI_ERROR (Status)) {
+ Status = Status1;
+ }
+ }
+
+ return Status;
+}
+
+/** A helper function to build and install multiple ACPI tables.
+
+ This is a helper function that invokes the Table generator interface
+ for building an ACPI table. It uses the AcpiTableProtocol to install the
+ table, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] Generator Pointer to the AcpiTable generator.
+ @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
+ @param [in] AcpiTableInfo Pointer to the ACPI table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildAndInstallMultipleAcpiTable (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST ACPI_TABLE_GENERATOR *CONST Generator,
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
+ )
+{
+ EFI_STATUS Status;
+ EFI_STATUS Status1;
+ EFI_ACPI_DESCRIPTION_HEADER **AcpiTable;
+ UINTN TableCount;
+ UINTN TableHandle;
+ UINTN Index;
+
+ AcpiTable = NULL;
+ TableCount = 0;
+ Status = Generator->BuildAcpiTableEx (
+ Generator,
+ AcpiTableInfo,
+ CfgMgrProtocol,
+ &AcpiTable,
+ &TableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Build Table." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ if ((AcpiTable == NULL) || (TableCount == 0)) {
+ Status = EFI_NOT_FOUND;
+ goto exit_handler;
+ }
+
+ for (Index = 0; Index < TableCount; Index++) {
+ // Dump ACPI Table Header
+ DUMP_ACPI_TABLE_HEADER (AcpiTable[Index]);
+ // Install ACPI table
+ Status = AcpiTableProtocol->InstallAcpiTable (
+ AcpiTableProtocol,
+ AcpiTable[Index],
+ AcpiTable[Index]->Length,
+ &TableHandle
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Install ACPI Table. Status = %r\n",
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: ACPI Table installed. Status = %r\n",
+ Status
+ ));
+ }
+
+exit_handler:
+ // Free any resources allocated for generating the tables.
+ if (Generator->FreeTableResourcesEx != NULL) {
+ Status1 = Generator->FreeTableResourcesEx (
+ Generator,
+ AcpiTableInfo,
+ CfgMgrProtocol,
+ &AcpiTable,
+ TableCount
+ );
+ if (EFI_ERROR (Status1)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Free Table Resources." \
+ "TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status1
+ ));
+ }
+
+ // Return the first error status in case of failure
+ if (!EFI_ERROR (Status)) {
+ Status = Status1;
+ }
+ }
+
+ return Status;
+}
+
+/** A helper function to invoke a Table generator
+
+ This is a helper function that invokes the Table generator interface
+ for building an ACPI table. It uses the AcpiTableProtocol to install the
+ table, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
+ @param [in] AcpiTableInfo Pointer to the ACPI table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildAndInstallAcpiTable (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
+ )
+{
+ EFI_STATUS Status;
+ CONST ACPI_TABLE_GENERATOR *Generator;
+
+ ASSERT (TableFactoryProtocol != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (AcpiTableProtocol != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: EStdObjAcpiTableList: Address = 0x%p," \
+ " TableGeneratorId = 0x%x\n",
+ AcpiTableInfo,
+ AcpiTableInfo->TableGeneratorId
+ ));
+
+ Generator = NULL;
+ Status = TableFactoryProtocol->GetAcpiTableGenerator (
+ TableFactoryProtocol,
+ AcpiTableInfo->TableGeneratorId,
+ &Generator
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Table Generator not found." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status
+ ));
+ return Status;
+ }
+
+ if (Generator == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Generator found : %s\n",
+ Generator->Description
+ ));
+
+ if (Generator->BuildAcpiTableEx != NULL) {
+ Status = BuildAndInstallMultipleAcpiTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ Generator,
+ AcpiTableProtocol,
+ AcpiTableInfo
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install ACPI Table." \
+ " Status = %r\n",
+ Status
+ ));
+ }
+ } else if (Generator->BuildAcpiTable != NULL) {
+ Status = BuildAndInstallSingleAcpiTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ Generator,
+ AcpiTableProtocol,
+ AcpiTableInfo
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install ACPI Table." \
+ " Status = %r\n",
+ Status
+ ));
+ }
+ } else {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Table Generator does not implement the" \
+ " ACPI_TABLE_GENERATOR_BUILD_TABLE interface." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ AcpiTableInfo->TableGeneratorId,
+ Status
+ ));
+ }
+
+ return Status;
+}
+
+/** The function checks if the Configuration Manager has provided the
+ mandatory ACPI tables for installation.
+
+ @param [in] AcpiTableInfo Pointer to the ACPI Table Info list.
+ @param [in] AcpiTableCount Count of ACPI Table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND If mandatory table is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+VerifyMandatoryTablesArePresent (
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo,
+ IN UINT32 AcpiTableCount
+ )
+{
+ EFI_STATUS Status;
+ BOOLEAN FadtFound;
+ BOOLEAN MadtFound;
+ BOOLEAN GtdtFound;
+ BOOLEAN DsdtFound;
+ BOOLEAN Dbg2Found;
+ BOOLEAN SpcrFound;
+
+ Status = EFI_SUCCESS;
+ FadtFound = FALSE;
+ MadtFound = FALSE;
+ GtdtFound = FALSE;
+ DsdtFound = FALSE;
+ Dbg2Found = FALSE;
+ SpcrFound = FALSE;
+ ASSERT (AcpiTableInfo != NULL);
+
+ while (AcpiTableCount-- != 0) {
+ switch (AcpiTableInfo[AcpiTableCount].AcpiTableSignature) {
+ case EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
+ FadtFound = TRUE;
+ break;
+ case EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
+ MadtFound = TRUE;
+ break;
+ case EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE:
+ GtdtFound = TRUE;
+ break;
+ case EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE:
+ DsdtFound = TRUE;
+ break;
+ case EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE:
+ Dbg2Found = TRUE;
+ break;
+ case EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE:
+ SpcrFound = TRUE;
+ break;
+ default:
+ break;
+ }
+ }
+
+ // We need at least the FADT, MADT, GTDT and the DSDT tables to boot
+ if (!FadtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found\n"));
+ Status = EFI_NOT_FOUND;
+ }
+
+ if (!MadtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: MADT Table not found.\n"));
+ Status = EFI_NOT_FOUND;
+ }
+
+ if (!GtdtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table not found.\n"));
+ Status = EFI_NOT_FOUND;
+ }
+
+ if (!DsdtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table not found.\n"));
+ Status = EFI_NOT_FOUND;
+ }
+
+ if (!Dbg2Found) {
+ DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
+ }
+
+ if (!SpcrFound) {
+ DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
+ }
+
+ return Status;
+}
+
+/** Generate and install ACPI tables.
+
+ The function gathers the information necessary for installing the
+ ACPI tables from the Configuration Manager, invokes the generators
+ and installs them (via BuildAndInstallAcpiTable).
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND If a mandatory table or a generator is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ProcessAcpiTables (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
+ CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
+ UINT32 AcpiTableCount;
+ UINT32 Idx;
+
+ ASSERT (TableFactoryProtocol != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+
+ // Find the AcpiTable protocol
+ Status = gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID **)&AcpiTableProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find AcpiTable protocol. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = GetEStdObjAcpiTableList (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &AcpiTableInfo,
+ &AcpiTableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to get ACPI Table List. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ if (0 == AcpiTableCount) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: EStdObjAcpiTableList: AcpiTableCount = %d\n",
+ AcpiTableCount
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: EStdObjAcpiTableList: AcpiTableCount = %d\n",
+ AcpiTableCount
+ ));
+
+ // Check if mandatory ACPI tables are present.
+ Status = VerifyMandatoryTablesArePresent (
+ AcpiTableInfo,
+ AcpiTableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find mandatory ACPI Table(s)."
+ " Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ // Add the FADT Table first.
+ for (Idx = 0; Idx < AcpiTableCount; Idx++) {
+ if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
+ AcpiTableInfo[Idx].TableGeneratorId)
+ {
+ Status = BuildAndInstallAcpiTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ AcpiTableProtocol,
+ &AcpiTableInfo[Idx]
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install ACPI FADT Table." \
+ " Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ break;
+ }
+ } // for
+
+ // Add remaining ACPI Tables
+ for (Idx = 0; Idx < AcpiTableCount; Idx++) {
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: AcpiTableInfo[%d].TableGeneratorId = 0x%x\n",
+ Idx,
+ AcpiTableInfo[Idx].TableGeneratorId
+ ));
+
+ // Skip FADT Table since we have already added
+ if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
+ AcpiTableInfo[Idx].TableGeneratorId)
+ {
+ continue;
+ }
+
+ // Skip the Reserved table Generator ID for standard generators
+ if ((IS_GENERATOR_NAMESPACE_STD (AcpiTableInfo[Idx].TableGeneratorId)) &&
+ ((CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdReserved) >=
+ AcpiTableInfo[Idx].TableGeneratorId) ||
+ (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMax) <=
+ AcpiTableInfo[Idx].TableGeneratorId)))
+ {
+ DEBUG ((
+ DEBUG_WARN,
+ "WARNING: Invalid ACPI Generator table ID = 0x%x, Skipping...\n",
+ AcpiTableInfo[Idx].TableGeneratorId
+ ));
+ continue;
+ }
+
+ Status = BuildAndInstallAcpiTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ AcpiTableProtocol,
+ &AcpiTableInfo[Idx]
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find, build, and install ACPI Table." \
+ " Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+ } // for
+
+ return Status;
+}
+
+/** Callback Function for Smbios table generation.
+
+ Callback function when SMBIOS protocol is installed, this function will then
+ invoke the code to install the available SMBIOS tables.
+
+ @param Event
+ @param Context
+
+ @retval None.
+**/
+VOID
+AcpiTableProtocolReady (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+ EDKII_CONFIGURATION_MANAGER_PROTOCOL *CfgMgrProtocol;
+ CM_STD_OBJ_CONFIGURATION_MANAGER_INFO *CfgMfrInfo;
+ EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *TableFactoryProtocol;
+
+ // Locate the Dynamic Table Factory
+ Status = gBS->LocateProtocol (
+ &gEdkiiDynamicTableFactoryProtocolGuid,
+ NULL,
+ (VOID **)&TableFactoryProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find Dynamic Table Factory protocol." \
+ " Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ // Locate the Configuration Manager for the Platform
+ Status = gBS->LocateProtocol (
+ &gEdkiiConfigurationManagerProtocolGuid,
+ NULL,
+ (VOID **)&CfgMgrProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find Configuration Manager protocol. Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ Status = GetCgfMgrInfo (CfgMgrProtocol, &CfgMfrInfo);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to get Configuration Manager info. Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Configuration Manager Version = 0x%x, OemID = %c%c%c%c%c%c\n",
+ CfgMfrInfo->Revision,
+ CfgMfrInfo->OemId[0],
+ CfgMfrInfo->OemId[1],
+ CfgMfrInfo->OemId[2],
+ CfgMfrInfo->OemId[3],
+ CfgMfrInfo->OemId[4],
+ CfgMfrInfo->OemId[5]
+ ));
+
+ Status = ProcessAcpiTables (TableFactoryProtocol, CfgMgrProtocol);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: ACPI Table processing failure. Status = %r\n",
+ Status
+ ));
+ }
+}
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index 1e9b811c40..242c40be3b 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -2,704 +2,39 @@
Dynamic Table Manager Dxe

Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2022 - 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.

SPDX-License-Identifier: BSD-2-Clause-Patent

**/

#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
-#include <Protocol/AcpiSystemDescriptionTable.h>
-#include <Protocol/AcpiTable.h>

// Module specific include files.
-#include <AcpiTableGenerator.h>
#include <ConfigurationManagerObject.h>
#include <ConfigurationManagerHelper.h>
-#include <DeviceTreeTableGenerator.h>
#include <Library/TableHelperLib.h>
#include <Protocol/ConfigurationManagerProtocol.h>
-#include <Protocol/DynamicTableFactoryProtocol.h>
-#include <SmbiosTableGenerator.h>

-///
-/// Bit definitions for acceptable ACPI table presence formats.
-/// Currently only ACPI tables present in the ACPI info list and
-/// already installed will count towards "Table Present" during
-/// verification routine.
-///
-#define ACPI_TABLE_PRESENT_INFO_LIST BIT0
-#define ACPI_TABLE_PRESENT_INSTALLED BIT1
+STATIC VOID *AcpiTableProtocolRegistration;
+STATIC VOID *SmbiosProtocolRegistration;

-///
-/// Order of ACPI table being verified during presence inspection.
-///
-#define ACPI_TABLE_VERIFY_FADT 0
-#define ACPI_TABLE_VERIFY_MADT 1
-#define ACPI_TABLE_VERIFY_GTDT 2
-#define ACPI_TABLE_VERIFY_DSDT 3
-#define ACPI_TABLE_VERIFY_DBG2 4
-#define ACPI_TABLE_VERIFY_SPCR 5
-#define ACPI_TABLE_VERIFY_COUNT 6
+extern
+VOID
+SmbiosProtocolReady (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ );

-///
-/// Private data structure to verify the presence of mandatory
-/// or optional ACPI tables.
-///
-typedef struct {
- /// ESTD ID for the ACPI table of interest.
- ESTD_ACPI_TABLE_ID EstdTableId;
- /// Standard UINT32 ACPI signature.
- UINT32 AcpiTableSignature;
- /// 4 character ACPI table name (the 5th char8 is for null terminator).
- CHAR8 AcpiTableName[sizeof (UINT32) + 1];
- /// Indicator on whether the ACPI table is required.
- BOOLEAN IsMandatory;
- /// Formats of verified presences, as defined by ACPI_TABLE_PRESENT_*
- /// This field should be initialized to 0 and will be populated during
- /// verification routine.
- UINT16 Presence;
-} ACPI_TABLE_PRESENCE_INFO;
-
-///
-/// We require the FADT, MADT, GTDT and the DSDT tables to boot.
-/// This list also include optional ACPI tables: DBG2, SPCR.
-///
-ACPI_TABLE_PRESENCE_INFO mAcpiVerifyTables[ACPI_TABLE_VERIFY_COUNT] = {
- { EStdAcpiTableIdFadt, EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, "FADT", TRUE, 0 },
- { EStdAcpiTableIdMadt, EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, "MADT", TRUE, 0 },
- { EStdAcpiTableIdGtdt, EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, "GTDT", TRUE, 0 },
- { EStdAcpiTableIdDsdt, EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, "DSDT", TRUE, 0 },
- { EStdAcpiTableIdDbg2, EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE, "DBG2", FALSE, 0 },
- { EStdAcpiTableIdSpcr, EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE, "SPCR", FALSE, 0 },
-};
-
-/** This macro expands to a function that retrieves the ACPI Table
- List from the Configuration Manager.
-*/
-GET_OBJECT_LIST (
- EObjNameSpaceStandard,
- EStdObjAcpiTableList,
- CM_STD_OBJ_ACPI_TABLE_INFO
- )
-
-/** A helper function to build and install a single ACPI table.
-
- This is a helper function that invokes the Table generator interface
- for building an ACPI table. It uses the AcpiTableProtocol to install the
- table, then frees the resources allocated for generating it.
-
- @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
- interface.
- @param [in] CfgMgrProtocol Pointer to the Configuration Manager
- Protocol Interface.
- @param [in] Generator Pointer to the AcpiTable generator.
- @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
- @param [in] AcpiTableInfo Pointer to the ACPI table Info.
-
- @retval EFI_SUCCESS Success.
- @retval EFI_INVALID_PARAMETER A parameter is invalid.
- @retval EFI_NOT_FOUND Required object is not found.
- @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
- is less than the Object size for the
- requested object.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-BuildAndInstallSingleAcpiTable (
- IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
- IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN CONST ACPI_TABLE_GENERATOR *CONST Generator,
- IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
- IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
- )
-{
- EFI_STATUS Status;
- EFI_STATUS Status1;
- EFI_ACPI_DESCRIPTION_HEADER *AcpiTable;
- UINTN TableHandle;
-
- AcpiTable = NULL;
- Status = Generator->BuildAcpiTable (
- Generator,
- AcpiTableInfo,
- CfgMgrProtocol,
- &AcpiTable
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Build Table." \
- " TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status
- ));
- // Free any allocated resources.
- goto exit_handler;
- }
-
- if (AcpiTable == NULL) {
- Status = EFI_NOT_FOUND;
- goto exit_handler;
- }
-
- // Dump ACPI Table Header
- DUMP_ACPI_TABLE_HEADER (AcpiTable);
-
- // Install ACPI table
- Status = AcpiTableProtocol->InstallAcpiTable (
- AcpiTableProtocol,
- AcpiTable,
- AcpiTable->Length,
- &TableHandle
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Install ACPI Table. Status = %r\n",
- Status
- ));
- // Free any allocated resources.
- goto exit_handler;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: ACPI Table installed. Status = %r\n",
- Status
- ));
-
-exit_handler:
- // Free any resources allocated for generating the tables.
- if (Generator->FreeTableResources != NULL) {
- Status1 = Generator->FreeTableResources (
- Generator,
- AcpiTableInfo,
- CfgMgrProtocol,
- &AcpiTable
- );
- if (EFI_ERROR (Status1)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Free Table Resources." \
- "TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status1
- ));
- }
-
- // Return the first error status in case of failure
- if (!EFI_ERROR (Status)) {
- Status = Status1;
- }
- }
-
- return Status;
-}
-
-/** A helper function to build and install multiple ACPI tables.
-
- This is a helper function that invokes the Table generator interface
- for building an ACPI table. It uses the AcpiTableProtocol to install the
- table, then frees the resources allocated for generating it.
-
- @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
- interface.
- @param [in] CfgMgrProtocol Pointer to the Configuration Manager
- Protocol Interface.
- @param [in] Generator Pointer to the AcpiTable generator.
- @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
- @param [in] AcpiTableInfo Pointer to the ACPI table Info.
-
- @retval EFI_SUCCESS Success.
- @retval EFI_INVALID_PARAMETER A parameter is invalid.
- @retval EFI_NOT_FOUND Required object is not found.
- @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
- is less than the Object size for the
- requested object.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-BuildAndInstallMultipleAcpiTable (
- IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
- IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN CONST ACPI_TABLE_GENERATOR *CONST Generator,
- IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
- IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
- )
-{
- EFI_STATUS Status;
- EFI_STATUS Status1;
- EFI_ACPI_DESCRIPTION_HEADER **AcpiTable;
- UINTN TableCount;
- UINTN TableHandle;
- UINTN Index;
-
- AcpiTable = NULL;
- TableCount = 0;
- Status = Generator->BuildAcpiTableEx (
- Generator,
- AcpiTableInfo,
- CfgMgrProtocol,
- &AcpiTable,
- &TableCount
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Build Table." \
- " TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status
- ));
- // Free any allocated resources.
- goto exit_handler;
- }
-
- if ((AcpiTable == NULL) || (TableCount == 0)) {
- Status = EFI_NOT_FOUND;
- goto exit_handler;
- }
-
- for (Index = 0; Index < TableCount; Index++) {
- // Dump ACPI Table Header
- DUMP_ACPI_TABLE_HEADER (AcpiTable[Index]);
- // Install ACPI table
- Status = AcpiTableProtocol->InstallAcpiTable (
- AcpiTableProtocol,
- AcpiTable[Index],
- AcpiTable[Index]->Length,
- &TableHandle
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Install ACPI Table. Status = %r\n",
- Status
- ));
- // Free any allocated resources.
- goto exit_handler;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: ACPI Table installed. Status = %r\n",
- Status
- ));
- }
-
-exit_handler:
- // Free any resources allocated for generating the tables.
- if (Generator->FreeTableResourcesEx != NULL) {
- Status1 = Generator->FreeTableResourcesEx (
- Generator,
- AcpiTableInfo,
- CfgMgrProtocol,
- &AcpiTable,
- TableCount
- );
- if (EFI_ERROR (Status1)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to Free Table Resources." \
- "TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status1
- ));
- }
-
- // Return the first error status in case of failure
- if (!EFI_ERROR (Status)) {
- Status = Status1;
- }
- }
-
- return Status;
-}
-
-/** A helper function to invoke a Table generator
-
- This is a helper function that invokes the Table generator interface
- for building an ACPI table. It uses the AcpiTableProtocol to install the
- table, then frees the resources allocated for generating it.
-
- @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
- interface.
- @param [in] CfgMgrProtocol Pointer to the Configuration Manager
- Protocol Interface.
- @param [in] AcpiTableProtocol Pointer to the AcpiTable protocol.
- @param [in] AcpiTableInfo Pointer to the ACPI table Info.
-
- @retval EFI_SUCCESS Success.
- @retval EFI_INVALID_PARAMETER A parameter is invalid.
- @retval EFI_NOT_FOUND Required object is not found.
- @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
- is less than the Object size for the
- requested object.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-BuildAndInstallAcpiTable (
- IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
- IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol,
- IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo
- )
-{
- EFI_STATUS Status;
- CONST ACPI_TABLE_GENERATOR *Generator;
-
- ASSERT (TableFactoryProtocol != NULL);
- ASSERT (CfgMgrProtocol != NULL);
- ASSERT (AcpiTableProtocol != NULL);
- ASSERT (AcpiTableInfo != NULL);
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: EStdObjAcpiTableList: Address = 0x%p," \
- " TableGeneratorId = 0x%x\n",
- AcpiTableInfo,
- AcpiTableInfo->TableGeneratorId
- ));
-
- Generator = NULL;
- Status = TableFactoryProtocol->GetAcpiTableGenerator (
- TableFactoryProtocol,
- AcpiTableInfo->TableGeneratorId,
- &Generator
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Table Generator not found." \
- " TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status
- ));
- return Status;
- }
-
- if (Generator == NULL) {
- return EFI_NOT_FOUND;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: Generator found : %s\n",
- Generator->Description
- ));
-
- if (Generator->BuildAcpiTableEx != NULL) {
- Status = BuildAndInstallMultipleAcpiTable (
- TableFactoryProtocol,
- CfgMgrProtocol,
- Generator,
- AcpiTableProtocol,
- AcpiTableInfo
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find build and install ACPI Table." \
- " Status = %r\n",
- Status
- ));
- }
- } else if (Generator->BuildAcpiTable != NULL) {
- Status = BuildAndInstallSingleAcpiTable (
- TableFactoryProtocol,
- CfgMgrProtocol,
- Generator,
- AcpiTableProtocol,
- AcpiTableInfo
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find build and install ACPI Table." \
- " Status = %r\n",
- Status
- ));
- }
- } else {
- Status = EFI_INVALID_PARAMETER;
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Table Generator does not implement the" \
- " ACPI_TABLE_GENERATOR_BUILD_TABLE interface." \
- " TableGeneratorId = 0x%x. Status = %r\n",
- AcpiTableInfo->TableGeneratorId,
- Status
- ));
- }
-
- return Status;
-}
-
-/** The function checks if the Configuration Manager has provided the
- mandatory ACPI tables for installation.
-
- @param [in] AcpiTableInfo Pointer to the ACPI Table Info list.
- @param [in] AcpiTableCount Count of ACPI Table Info.
-
- @retval EFI_SUCCESS Success.
- @retval EFI_NOT_FOUND If mandatory table is not found.
- @retval EFI_ALREADY_STARTED If mandatory table found in AcpiTableInfo is already installed.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-VerifyMandatoryTablesArePresent (
- IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo,
- IN UINT32 AcpiTableCount
- )
-{
- EFI_STATUS Status;
- UINTN Handle;
- UINTN Index;
- UINTN InstalledTableIndex;
- EFI_ACPI_DESCRIPTION_HEADER *DescHeader;
- EFI_ACPI_TABLE_VERSION Version;
- EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
-
- ASSERT (AcpiTableInfo != NULL);
-
- Status = EFI_SUCCESS;
-
- // Check against the statically initialized ACPI tables to see if they are in ACPI info list
- while (AcpiTableCount-- != 0) {
- for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
- if (AcpiTableInfo[AcpiTableCount].AcpiTableSignature == mAcpiVerifyTables[Index].AcpiTableSignature) {
- mAcpiVerifyTables[Index].Presence |= ACPI_TABLE_PRESENT_INFO_LIST;
- // Found this table, skip the rest.
- break;
- }
- }
- }
-
- // They also might be published already, so we can search from there
- if (FeaturePcdGet (PcdInstallAcpiSdtProtocol)) {
- AcpiSdt = NULL;
- Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)&AcpiSdt);
-
- if (EFI_ERROR (Status) || (AcpiSdt == NULL)) {
- DEBUG ((DEBUG_ERROR, "ERROR: Failed to locate ACPI SDT protocol (0x%p) - %r\n", AcpiSdt, Status));
- return Status;
- }
-
- for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
- Handle = 0;
- InstalledTableIndex = 0;
- do {
- Status = AcpiSdt->GetAcpiTable (InstalledTableIndex, (EFI_ACPI_SDT_HEADER **)&DescHeader, &Version, &Handle);
- if (EFI_ERROR (Status)) {
- break;
- }
-
- InstalledTableIndex++;
- } while (DescHeader->Signature != mAcpiVerifyTables[Index].AcpiTableSignature);
-
- if (!EFI_ERROR (Status)) {
- mAcpiVerifyTables[Index].Presence |= ACPI_TABLE_PRESENT_INSTALLED;
- }
- }
- }
-
- // Reset the return Status value to EFI_SUCCESS. We do not fully care if the table look up has failed.
- Status = EFI_SUCCESS;
- for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
- if (mAcpiVerifyTables[Index].Presence == 0) {
- if (mAcpiVerifyTables[Index].IsMandatory) {
- DEBUG ((DEBUG_ERROR, "ERROR: %a Table not found.\n", mAcpiVerifyTables[Index].AcpiTableName));
- Status = EFI_NOT_FOUND;
- } else {
- DEBUG ((DEBUG_WARN, "WARNING: %a Table not found.\n", mAcpiVerifyTables[Index].AcpiTableName));
- }
- } else if (mAcpiVerifyTables[Index].Presence ==
- (ACPI_TABLE_PRESENT_INFO_LIST | ACPI_TABLE_PRESENT_INSTALLED))
- {
- DEBUG ((DEBUG_ERROR, "ERROR: %a Table found while already published.\n", mAcpiVerifyTables[Index].AcpiTableName));
- Status = EFI_ALREADY_STARTED;
- }
- }
-
- return Status;
-}
-
-/** Generate and install ACPI tables.
-
- The function gathers the information necessary for installing the
- ACPI tables from the Configuration Manager, invokes the generators
- and installs them (via BuildAndInstallAcpiTable).
-
- @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
- interface.
- @param [in] CfgMgrProtocol Pointer to the Configuration Manager
- Protocol Interface.
-
- @retval EFI_SUCCESS Success.
- @retval EFI_NOT_FOUND If a mandatory table or a generator is not found.
- @retval EFI_ALREADY_STARTED If mandatory table found in AcpiTableInfo is already installed.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-ProcessAcpiTables (
- IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
- IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
- )
-{
- EFI_STATUS Status;
- EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
- CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
- UINT32 AcpiTableCount;
- UINT32 Idx;
-
- ASSERT (TableFactoryProtocol != NULL);
- ASSERT (CfgMgrProtocol != NULL);
-
- // Find the AcpiTable protocol
- Status = gBS->LocateProtocol (
- &gEfiAcpiTableProtocolGuid,
- NULL,
- (VOID **)&AcpiTableProtocol
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find AcpiTable protocol. Status = %r\n",
- Status
- ));
- return Status;
- }
-
- Status = GetEStdObjAcpiTableList (
- CfgMgrProtocol,
- CM_NULL_TOKEN,
- &AcpiTableInfo,
- &AcpiTableCount
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to get ACPI Table List. Status = %r\n",
- Status
- ));
- return Status;
- }
-
- if (0 == AcpiTableCount) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: EStdObjAcpiTableList: AcpiTableCount = %d\n",
- AcpiTableCount
- ));
- return EFI_NOT_FOUND;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: EStdObjAcpiTableList: AcpiTableCount = %d\n",
- AcpiTableCount
- ));
-
- // Check if mandatory ACPI tables are present.
- Status = VerifyMandatoryTablesArePresent (
- AcpiTableInfo,
- AcpiTableCount
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to verify mandatory ACPI Table(s) presence."
- " Status = %r\n",
- Status
- ));
- return Status;
- }
-
- // Add the FADT Table first.
- if ((mAcpiVerifyTables[ACPI_TABLE_VERIFY_FADT].Presence & ACPI_TABLE_PRESENT_INSTALLED) == 0) {
- // FADT is not yet installed
- for (Idx = 0; Idx < AcpiTableCount; Idx++) {
- if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
- AcpiTableInfo[Idx].TableGeneratorId)
- {
- Status = BuildAndInstallAcpiTable (
- TableFactoryProtocol,
- CfgMgrProtocol,
- AcpiTableProtocol,
- &AcpiTableInfo[Idx]
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find build and install ACPI FADT Table." \
- " Status = %r\n",
- Status
- ));
- return Status;
- }
-
- break;
- }
- } // for
- }
-
- // Add remaining ACPI Tables
- for (Idx = 0; Idx < AcpiTableCount; Idx++) {
- DEBUG ((
- DEBUG_INFO,
- "INFO: AcpiTableInfo[%d].TableGeneratorId = 0x%x\n",
- Idx,
- AcpiTableInfo[Idx].TableGeneratorId
- ));
-
- // Skip FADT Table since we have already added
- if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
- AcpiTableInfo[Idx].TableGeneratorId)
- {
- continue;
- }
-
- // Skip the Reserved table Generator ID for standard generators
- if ((IS_GENERATOR_NAMESPACE_STD (AcpiTableInfo[Idx].TableGeneratorId)) &&
- ((CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdReserved) >=
- AcpiTableInfo[Idx].TableGeneratorId) ||
- (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMax) <=
- AcpiTableInfo[Idx].TableGeneratorId)))
- {
- DEBUG ((
- DEBUG_WARN,
- "WARNING: Invalid ACPI Generator table ID = 0x%x, Skipping...\n",
- AcpiTableInfo[Idx].TableGeneratorId
- ));
- continue;
- }
-
- Status = BuildAndInstallAcpiTable (
- TableFactoryProtocol,
- CfgMgrProtocol,
- AcpiTableProtocol,
- &AcpiTableInfo[Idx]
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find, build, and install ACPI Table." \
- " Status = %r\n",
- Status
- ));
- return Status;
- }
- } // for
-
- return Status;
-}
+extern
+VOID
+AcpiTableProtocolReady (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ );

/** Entrypoint of Dynamic Table Manager Dxe.

@@ -727,72 +62,33 @@ DynamicTableManagerDxeInitialize (
IN EFI_SYSTEM_TABLE *SystemTable
)
{
- EFI_STATUS Status;
- EDKII_CONFIGURATION_MANAGER_PROTOCOL *CfgMgrProtocol;
- CM_STD_OBJ_CONFIGURATION_MANAGER_INFO *CfgMfrInfo;
- EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *TableFactoryProtocol;
-
- // Locate the Dynamic Table Factory
- Status = gBS->LocateProtocol (
- &gEdkiiDynamicTableFactoryProtocolGuid,
- NULL,
- (VOID **)&TableFactoryProtocol
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find Dynamic Table Factory protocol." \
- " Status = %r\n",
- Status
- ));
- return Status;
- }
-
- // Locate the Configuration Manager for the Platform
- Status = gBS->LocateProtocol (
- &gEdkiiConfigurationManagerProtocolGuid,
- NULL,
- (VOID **)&CfgMgrProtocol
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find Configuration Manager protocol. Status = %r\n",
- Status
- ));
- return Status;
- }
-
- Status = GetCgfMgrInfo (CfgMgrProtocol, &CfgMfrInfo);
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to get Configuration Manager info. Status = %r\n",
- Status
- ));
- return Status;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "INFO: Configuration Manager Version = 0x%x, OemID = %c%c%c%c%c%c\n",
- CfgMfrInfo->Revision,
- CfgMfrInfo->OemId[0],
- CfgMfrInfo->OemId[1],
- CfgMfrInfo->OemId[2],
- CfgMfrInfo->OemId[3],
- CfgMfrInfo->OemId[4],
- CfgMfrInfo->OemId[5]
- ));
-
- Status = ProcessAcpiTables (TableFactoryProtocol, CfgMgrProtocol);
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: ACPI Table processing failure. Status = %r\n",
- Status
- ));
- }
-
- return Status;
+ EFI_EVENT AcpiEvent;
+ EFI_EVENT SmbiosEvent;
+
+ AcpiEvent = EfiCreateProtocolNotifyEvent (
+ &gEfiAcpiTableProtocolGuid,
+ TPL_CALLBACK,
+ AcpiTableProtocolReady,
+ NULL,
+ &AcpiTableProtocolRegistration
+ );
+ if (AcpiEvent == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to ACPI create protocol event\r\n", __FUNCTION__));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ SmbiosEvent = EfiCreateProtocolNotifyEvent (
+ &gEfiSmbiosProtocolGuid,
+ TPL_CALLBACK,
+ SmbiosProtocolReady,
+ NULL,
+ &SmbiosProtocolRegistration
+ );
+ if (SmbiosEvent == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to SMBIOS create protocol event\r\n", __FUNCTION__));
+ gBS->CloseEvent (AcpiEvent);
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ return EFI_SUCCESS;
}
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
index b09272d883..9d020f0264 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
@@ -22,6 +22,8 @@

[Sources]
DynamicTableManagerDxe.c
+ SmbiosTableBuilder.c
+ AcpiTableBuilder.c
SmbiosTableDispatcher.c
SmbiosTableDispatcher.h

@@ -33,6 +35,7 @@
[LibraryClasses]
PrintLib
TableHelperLib
+ UefiLib
UefiBootServicesTableLib
UefiDriverEntryPoint

@@ -42,12 +45,12 @@
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEfiAcpiSdtProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+ gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED

gEdkiiConfigurationManagerProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEdkiiDynamicTableFactoryProtocolGuid # PROTOCOL ALWAYS_CONSUMED

[Depex]
- gEfiAcpiTableProtocolGuid AND
gEdkiiConfigurationManagerProtocolGuid AND
gEdkiiDynamicTableFactoryProtocolGuid

diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c
new file mode 100644
index 0000000000..9ca690c16b
--- /dev/null
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c
@@ -0,0 +1,608 @@
+/** @file
+ SMBIOS Table Builder
+
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2022 - 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/Smbios.h>
+#include <Library/BaseMemoryLib.h>
+
+// Module specific include files.
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <Library/TableHelperLib.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+#include <Protocol/DynamicTableFactoryProtocol.h>
+#include <SmbiosTableGenerator.h>
+#include <SmbiosTableDispatcher.h>
+
+/** This macro expands to a function that retrieves the SMBIOS Table
+ List from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceStandard,
+ EStdObjSmbiosTableList,
+ CM_STD_OBJ_SMBIOS_TABLE_INFO
+ )
+
+/** A helper function to build and install an SMBIOS table.
+
+ This is a helper function that invokes the Table generator interface
+ for building an SMBIOS table. It uses the SmbiosProtocol to install the
+ table, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] SmbiosProtocol Pointer to the SMBIOS protocol.
+ @param [in] SmbiosTableInfo Pointer to the SMBIOS table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildAndInstallSingleSmbiosTable (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST SMBIOS_TABLE_GENERATOR *CONST Generator,
+ IN EFI_SMBIOS_PROTOCOL *SmbiosProtocol,
+ IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo
+ )
+{
+ EFI_STATUS Status;
+ EFI_STATUS Status1;
+ SMBIOS_STRUCTURE *SmbiosTable;
+ CM_OBJECT_TOKEN CmObjToken;
+ EFI_SMBIOS_HANDLE TableHandle;
+ SMBIOS_HANDLE_MAP *HandleMap;
+
+ SmbiosTable = NULL;
+ Status = Generator->BuildSmbiosTable (
+ Generator,
+ TableFactoryProtocol,
+ SmbiosTableInfo,
+ CfgMgrProtocol,
+ &SmbiosTable,
+ &CmObjToken
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Build Table." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ if (SmbiosTable == NULL) {
+ Status = EFI_NOT_FOUND;
+ goto exit_handler;
+ }
+
+ HandleMap = TableFactoryProtocol->GetSmbiosHandle (CmObjToken);
+ if (HandleMap == NULL) {
+ TableHandle = SMBIOS_HANDLE_PI_RESERVED;
+ } else {
+ TableHandle = HandleMap->SmbiosTblHandle;
+ }
+
+ // Install SMBIOS table
+ Status = SmbiosProtocol->Add (
+ SmbiosProtocol,
+ NULL,
+ &TableHandle,
+ SmbiosTable
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Install SMBIOS Table. Status = %r\n",
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ // If a new handle was generated then add it to the handle map.
+ if (HandleMap == NULL) {
+ TableFactoryProtocol->AddSmbiosHandle (
+ SmbiosProtocol,
+ &TableHandle,
+ CmObjToken,
+ SmbiosTableInfo->TableGeneratorId
+ );
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: SMBIOS Table installed. Status = %r\n",
+ Status
+ ));
+
+exit_handler:
+ // Free any resources allocated for generating the tables.
+ if (Generator->FreeTableResources != NULL) {
+ Status1 = Generator->FreeTableResources (
+ Generator,
+ TableFactoryProtocol,
+ SmbiosTableInfo,
+ CfgMgrProtocol,
+ &SmbiosTable
+ );
+ if (EFI_ERROR (Status1)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Free Table Resources." \
+ "TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status1
+ ));
+ }
+
+ // Return the first error status in case of failure
+ if (!EFI_ERROR (Status)) {
+ Status = Status1;
+ }
+ }
+
+ return Status;
+}
+
+/** A helper function to build and install multiple SMBIOS tables.
+
+ This is a helper function that invokes the Table generator interface
+ for building SMBIOS tables. It uses the SmbiosProtocol to install the
+ tables, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] Generator Pointer to the SmbiosTable generator.
+ @param [in] SmbiosProtocol Pointer to the Smbios protocol.
+ @param [in] AcpiTableInfo Pointer to the SMBIOS table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildAndInstallMultipleSmbiosTables (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST SMBIOS_TABLE_GENERATOR *CONST Generator,
+ IN EFI_SMBIOS_PROTOCOL *SmbiosProtocol,
+ IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo
+ )
+{
+ EFI_STATUS Status;
+ EFI_STATUS Status1;
+ SMBIOS_STRUCTURE **SmbiosTable;
+ CM_OBJECT_TOKEN *CmObjToken;
+ EFI_SMBIOS_HANDLE TableHandle;
+ UINTN TableCount;
+ UINTN Index;
+ SMBIOS_HANDLE_MAP *HandleMap;
+
+ TableCount = 0;
+ Status = Generator->BuildSmbiosTableEx (
+ Generator,
+ TableFactoryProtocol,
+ SmbiosTableInfo,
+ CfgMgrProtocol,
+ &SmbiosTable,
+ &CmObjToken,
+ &TableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Build Table." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ if ((SmbiosTable == NULL) || (TableCount == 0)) {
+ Status = EFI_NOT_FOUND;
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: TableCount %u SmbiosTable %p \n",
+ __FUNCTION__,
+ TableCount,
+ SmbiosTable
+ ));
+ goto exit_handler;
+ }
+
+ for (Index = 0; Index < TableCount; Index++) {
+ HandleMap = TableFactoryProtocol->GetSmbiosHandle (CmObjToken[Index]);
+ if (HandleMap == NULL) {
+ TableHandle = SMBIOS_HANDLE_PI_RESERVED;
+ } else {
+ TableHandle = HandleMap->SmbiosTblHandle;
+ }
+
+ // Install SMBIOS table
+ Status = SmbiosProtocol->Add (
+ SmbiosProtocol,
+ NULL,
+ &TableHandle,
+ SmbiosTable[Index]
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Install SMBIOS Table. Status = %r\n",
+ Status
+ ));
+ // Free any allocated resources.
+ goto exit_handler;
+ }
+
+ // If a new handle was generated then add it to the handle map.
+ if (HandleMap == NULL) {
+ TableFactoryProtocol->AddSmbiosHandle (
+ SmbiosProtocol,
+ &TableHandle,
+ CmObjToken[Index],
+ SmbiosTableInfo->TableGeneratorId
+ );
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: SMBIOS Table installed. Status = %r\n",
+ Status
+ ));
+ }
+
+exit_handler:
+ // Free any resources allocated for generating the tables.
+ if (Generator->FreeTableResourcesEx != NULL) {
+ Status1 = Generator->FreeTableResourcesEx (
+ Generator,
+ TableFactoryProtocol,
+ SmbiosTableInfo,
+ CfgMgrProtocol,
+ &SmbiosTable,
+ &CmObjToken,
+ TableCount
+ );
+ if (EFI_ERROR (Status1)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to Free Table Resources." \
+ "TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status1
+ ));
+ }
+
+ // Return the first error status in case of failure
+ if (!EFI_ERROR (Status)) {
+ Status = Status1;
+ }
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
+ return Status;
+}
+
+/** A helper function to invoke a Table generator
+
+ This is a helper function that invokes the Table generator interface
+ for building an SMBIOS table. It uses the SmbiosProtocol to install the
+ table, then frees the resources allocated for generating it.
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] SmbiosProtocol Pointer to the SMBIOS protocol.
+ @param [in] SmbiosTableInfo Pointer to the SMBIOS table Info.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Required object is not found.
+ @retval EFI_BAD_BUFFER_SIZE Size returned by the Configuration Manager
+ is less than the Object size for the
+ requested object.
+**/
+EFI_STATUS
+EFIAPI
+BuildAndInstallSmbiosTable (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN EFI_SMBIOS_PROTOCOL *SmbiosProtocol,
+ IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo
+ )
+{
+ EFI_STATUS Status;
+ CONST SMBIOS_TABLE_GENERATOR *Generator;
+
+ ASSERT (TableFactoryProtocol != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (SmbiosProtocol != NULL);
+ ASSERT (SmbiosTableInfo != NULL);
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: EStdObjSmbiosTableList: Address = 0x%p," \
+ " TableGeneratorId = 0x%x\n",
+ SmbiosTableInfo,
+ SmbiosTableInfo->TableGeneratorId
+ ));
+
+ Generator = NULL;
+ Status = TableFactoryProtocol->GetSmbiosTableGenerator (
+ TableFactoryProtocol,
+ SmbiosTableInfo->TableGeneratorId,
+ &Generator
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Table Generator not found." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status
+ ));
+ return Status;
+ }
+
+ if (Generator == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Generator found : %s\n",
+ Generator->Description
+ ));
+
+ if (Generator->BuildSmbiosTableEx != NULL) {
+ Status = BuildAndInstallMultipleSmbiosTables (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ Generator,
+ SmbiosProtocol,
+ SmbiosTableInfo
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install SMBIOS Tables." \
+ " Status = %r\n",
+ Status
+ ));
+ }
+ } else if (Generator->BuildSmbiosTable != NULL) {
+ Status = BuildAndInstallSingleSmbiosTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ Generator,
+ SmbiosProtocol,
+ SmbiosTableInfo
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install SMBIOS Table." \
+ " Status = %r\n",
+ Status
+ ));
+ }
+ } else {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Table Generator does not implement the" \
+ "SMBIOS_TABLE_GENERATOR_BUILD_TABLE interface." \
+ " TableGeneratorId = 0x%x. Status = %r\n",
+ SmbiosTableInfo->TableGeneratorId,
+ Status
+ ));
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
+ return Status;
+}
+
+/** Generate and install SMBIOS tables.
+
+ The function gathers the information necessary for installing the
+ SMBIOS tables from the Configuration Manager, invokes the generators
+ and installs them (via BuildAndInstallAcpiTable).
+
+ @param [in] TableFactoryProtocol Pointer to the Table Factory Protocol
+ interface.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND If a mandatory table or a generator is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ProcessSmbiosTables (
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_PROTOCOL *SmbiosProtocol;
+ CM_STD_OBJ_SMBIOS_TABLE_INFO *SmbiosTableInfo;
+ UINT32 SmbiosTableCount;
+ UINT32 Idx;
+
+ Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID **)&SmbiosProtocol);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Could not locate SMBIOS protocol. %r\n", Status));
+ return Status;
+ }
+
+ Status = GetEStdObjSmbiosTableList (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &SmbiosTableInfo,
+ &SmbiosTableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to get SMBIOS Table List. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ if (SmbiosTableCount == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: EStdObjSmbiosTableList: SmbiosTableCount = %d\n",
+ SmbiosTableCount
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: EStdObjSmbiosTableList: SmbiosTableCount = %d\n",
+ SmbiosTableCount
+ ));
+
+ for (Idx = 0; Idx < SmbiosTableCount; Idx++) {
+ Status = DispatchSmbiosTable (
+ SmbiosTableInfo[Idx].TableType,
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ SmbiosProtocol,
+ SmbiosTableInfo,
+ SmbiosTableCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to install SMBIOS Table." \
+ " Id = %u Status = %r\n",
+ SmbiosTableInfo[Idx].TableGeneratorId,
+ Status
+ ));
+ }
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
+ return Status;
+}
+
+/** Callback Function for Smbios table generation.
+
+ Callback function when SMBIOS protocol is installed, this function will then
+ invoke the code to install the available SMBIOS tables.
+
+ @param Event
+ @param Context
+
+ @retval None.
+**/
+VOID
+SmbiosProtocolReady (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+ EDKII_CONFIGURATION_MANAGER_PROTOCOL *CfgMgrProtocol;
+ CM_STD_OBJ_CONFIGURATION_MANAGER_INFO *CfgMfrInfo;
+ EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *TableFactoryProtocol;
+
+ // Locate the Dynamic Table Factory
+ Status = gBS->LocateProtocol (
+ &gEdkiiDynamicTableFactoryProtocolGuid,
+ NULL,
+ (VOID **)&TableFactoryProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find Dynamic Table Factory protocol." \
+ " Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ // Locate the Configuration Manager for the Platform
+ Status = gBS->LocateProtocol (
+ &gEdkiiConfigurationManagerProtocolGuid,
+ NULL,
+ (VOID **)&CfgMgrProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find Configuration Manager protocol. Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ Status = GetCgfMgrInfo (CfgMgrProtocol, &CfgMfrInfo);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to get Configuration Manager info. Status = %r\n",
+ Status
+ ));
+ return;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Configuration Manager Version = 0x%x, OemID = %c%c%c%c%c%c\n",
+ CfgMfrInfo->Revision,
+ CfgMfrInfo->OemId[0],
+ CfgMfrInfo->OemId[1],
+ CfgMfrInfo->OemId[2],
+ CfgMfrInfo->OemId[3],
+ CfgMfrInfo->OemId[4],
+ CfgMfrInfo->OemId[5]
+ ));
+
+ Status = ProcessSmbiosTables (TableFactoryProtocol, CfgMgrProtocol);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: SMBIOS Table processing failure. Status = %r\n",
+ Status
+ ));
+ }
+}
diff --git a/DynamicTablesPkg/Include/ConfigurationManagerObject.h b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
index 74ad25d5d9..0c97e70996 100644
--- a/DynamicTablesPkg/Include/ConfigurationManagerObject.h
+++ b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
@@ -31,6 +31,7 @@ Bits: [31:28] - Name Space ID
0000 - Standard
0001 - ARM
1000 - Custom/OEM
+ 1010 - SMBIOS
All other values are reserved.

Bits: [27:16] - Reserved.
@@ -105,9 +106,10 @@ typedef UINT32 CM_OBJECT_ID;
for the Configuration Manager Objects.
*/
typedef enum ObjectNameSpaceID {
- EObjNameSpaceStandard, ///< Standard Objects Namespace
- EObjNameSpaceArm, ///< ARM Objects Namespace
- EObjNameSpaceOem = 0x8, ///< OEM Objects Namespace
+ EObjNameSpaceStandard, ///< Standard Objects Namespace
+ EObjNameSpaceArm, ///< ARM Objects Namespace
+ EObjNameSpaceOem = 0x8, ///< OEM Objects Namespace
+ EObjNameSpaceSmbios = 0xA, ///< SMBIOS Objects Namespace
EObjNameSpaceMax
} EOBJECT_NAMESPACE_ID;

@@ -192,4 +194,14 @@ typedef struct CmObjDescriptor {
#define CREATE_CM_OEM_OBJECT_ID(ObjectId) \
(CREATE_CM_OBJECT_ID (EObjNameSpaceOem, ObjectId))

+/** This macro returns a Configuration Manager Object ID
+ in the SMBIOS Object Namespace.
+
+ @param [in] ObjectId The Object ID.
+
+ @retval Returns an SMBIOS Configuration Manager Object ID.
+**/
+#define CREATE_CM_SMBIOS_OBJECT_ID(ObjectId) \
+ (CREATE_CM_OBJECT_ID (EObjNameSpaceSmbios, ObjectId))
+
#endif // CONFIGURATION_MANAGER_OBJECT_H_
diff --git a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
index b11fc0c9f1..0670a8fb27 100644
--- a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
+++ b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
@@ -239,12 +239,20 @@ typedef struct DynamicTableFactoryProtocol {
EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_DT_TABLE_GENERATOR
DeregisterDtTableGenerator;

+ EDKII_DYNAMIC_TABLE_FACTORY_SMBIOS_TABLE_ADD_HANDLE
+ AddSmbiosHandle;
+
+ EDKII_DYNAMIC_TABLE_FACTORY_SMBIOS_TABLE_GET_HANDLE
+ GetSmbiosHandle;
+
/** Pointer to the data structure that holds the
list of registered table generators
*/
EDKII_DYNAMIC_TABLE_FACTORY_INFO *TableFactoryInfo;
} EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL;

+
+
/** The Dynamic Table Factory Protocol GUID.
*/
extern EFI_GUID gEdkiiDynamicTableFactoryProtocolGuid;
diff --git a/DynamicTablesPkg/Include/SmbiosTableGenerator.h b/DynamicTablesPkg/Include/SmbiosTableGenerator.h
index 934d56c90d..d81eb8f124 100644
--- a/DynamicTablesPkg/Include/SmbiosTableGenerator.h
+++ b/DynamicTablesPkg/Include/SmbiosTableGenerator.h
@@ -10,7 +10,7 @@
#define SMBIOS_TABLE_GENERATOR_H_

#include <IndustryStandard/SmBios.h>
-
+#include <Protocol/Smbios.h>
#include <TableGenerator.h>

#pragma pack(1)
@@ -121,12 +121,21 @@ typedef enum StdSmbiosTableGeneratorId {
ETableGeneratorNameSpaceStd, \
TableId \
)
+#define MAX_SMBIOS_HANDLES (255)

/** Forward declarations.
*/
typedef struct ConfigurationManagerProtocol EDKII_CONFIGURATION_MANAGER_PROTOCOL;
typedef struct CmStdObjSmbiosTableInfo CM_STD_OBJ_SMBIOS_TABLE_INFO;
typedef struct SmbiosTableGenerator SMBIOS_TABLE_GENERATOR;
+typedef struct DynamicTableFactoryProtocol EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL;
+typedef UINTN CM_OBJECT_TOKEN;
+
+typedef struct SmbiosHandleCmObjMap {
+ ESTD_SMBIOS_TABLE_ID SmbiosTableId;
+ SMBIOS_HANDLE SmbiosTblHandle;
+ CM_OBJECT_TOKEN SmbiosCmToken;
+} SMBIOS_HANDLE_MAP;

/** This function pointer describes the interface to SMBIOS table build
functions provided by the SMBIOS table generator and called by the
@@ -143,9 +152,11 @@ typedef struct SmbiosTableGenerator SMBIOS_TABLE_GENERATOR;
**/
typedef EFI_STATUS (*SMBIOS_TABLE_GENERATOR_BUILD_TABLE) (
IN CONST SMBIOS_TABLE_GENERATOR *Generator,
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- OUT SMBIOS_STRUCTURE **Table
+ OUT SMBIOS_STRUCTURE **Table,
+ OUT CM_OBJECT_TOKEN *CmObjToken
);

/** This function pointer describes the interface to used by the
@@ -163,11 +174,105 @@ typedef EFI_STATUS (*SMBIOS_TABLE_GENERATOR_BUILD_TABLE) (
**/
typedef EFI_STATUS (*SMBIOS_TABLE_GENERATOR_FREE_TABLE) (
IN CONST SMBIOS_TABLE_GENERATOR *Generator,
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
IN CONST CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
IN SMBIOS_STRUCTURE **Table
);

+/** This function pointer describes the interface to SMBIOS table build
+ functions provided by the SMBIOS table generator and called by the
+ Table Manager to build an SMBIOS table.
+
+ @param [in] Generator Pointer to the SMBIOS table generator.
+ @param [in] SmbiosTableInfo Pointer to the SMBIOS table information.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol interface.
+ @param [out] Table Pointer to the generated SMBIOS table.
+
+ @return EFI_SUCCESS If the table is generated successfully or other
+ failure codes as returned by the generator.
+**/
+typedef EFI_STATUS (*SMBIOS_TABLE_GENERATOR_BUILD_TABLEEX) (
+ IN CONST SMBIOS_TABLE_GENERATOR *Generator,
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ OUT SMBIOS_STRUCTURE ***Table,
+ OUT CM_OBJECT_TOKEN **CmObjectToken,
+ OUT UINTN *CONST TableCount
+ );
+
+/** This function pointer describes the interface to used by the
+ Table Manager to give the generator an opportunity to free
+ any resources allocated for building the SMBIOS table.
+
+ @param [in] Generator Pointer to the SMBIOS table generator.
+ @param [in] SmbiosTableInfo Pointer to the SMBIOS table information.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol interface.
+ @param [in] Table Pointer to the generated SMBIOS table.
+
+ @return EFI_SUCCESS If freed successfully or other failure codes
+ as returned by the generator.
+**/
+typedef EFI_STATUS (*SMBIOS_TABLE_GENERATOR_FREE_TABLEEX) (
+ IN CONST SMBIOS_TABLE_GENERATOR *Generator,
+ IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
+ IN CONST CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN SMBIOS_STRUCTURE ***Table,
+ IN CM_OBJECT_TOKEN **CmObjectToken,
+ IN CONST UINTN TableCount
+ );
+
+/** This function pointer describes the interface to used by the
+ Table Manager to give the generator an opportunity to add
+ an SMBIOS Handle.
+
+ This function is called by the Dynamic Table Manager to add a newly added
+ SMBIOS Table OR it can be called by an SMBIOS Table generator to create
+ and add a new SMBIOS Handle if there is a reference to another table and
+ if there is a generator for that table that hasn't been called yet.
+
+ @param [in] Smbios Pointer to the SMBIOS protocol.
+ @param [in] SmbiosHandle Pointer to an SMBIOS handle (either generated by
+ SmbiosDxe Driver or SMBIOS_HANDLE_PI_RESERVED
+ if a handle needs to be generated).
+ @param [in] CmObjectToken The CM Object token for that is used to generate
+ SMBIOS record.
+ @param [in] SmbiosId The SMBIOS table generator Id.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_OUT_OF_RESOURCES Unable to add the handle.
+ @retval EFI_NOT_FOUND The requested generator is not found
+ in the list of registered generators.
+**/
+typedef EFI_STATUS (*EDKII_DYNAMIC_TABLE_FACTORY_SMBIOS_TABLE_ADD_HANDLE) (
+ IN EFI_SMBIOS_PROTOCOL *Smbios,
+ IN SMBIOS_HANDLE *SmbiosHandle,
+ IN CM_OBJECT_TOKEN CmObjectToken,
+ IN ESTD_SMBIOS_TABLE_ID SmbiosId
+ );
+
+/** This function pointer describes the interface to used by the
+ Table Manager to give the generator an opportunity to find
+ an SMBIOS Handle.
+
+ This function is called by the SMBIOS table generator to find an SMBIOS
+ handle needed as part of generating an SMBIOS Table.
+
+ @param [in] CmObjectToken The CM Object token used to generate the SMBIOS
+ record.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND The requested generator is not found
+ in the list of registered generators.
+**/
+typedef SMBIOS_HANDLE_MAP * (*EDKII_DYNAMIC_TABLE_FACTORY_SMBIOS_TABLE_GET_HANDLE) (
+ IN CM_OBJECT_TOKEN CmObjectToken
+ );
+
/** The SMBIOS_TABLE_GENERATOR structure provides an interface that the
Table Manager can use to invoke the functions to build SMBIOS tables.
*/
@@ -189,6 +294,14 @@ typedef struct SmbiosTableGenerator {
allocated for building the SMBIOS table.
*/
SMBIOS_TABLE_GENERATOR_FREE_TABLE FreeTableResources;
+
+ /// SMBIOS table extended build function pointer.
+ SMBIOS_TABLE_GENERATOR_BUILD_TABLEEX BuildSmbiosTableEx;
+
+ /** The function to free any resources
+ allocated for building the SMBIOS table.
+ */
+ SMBIOS_TABLE_GENERATOR_FREE_TABLEEX FreeTableResourcesEx;
} SMBIOS_TABLE_GENERATOR;

/** Register SMBIOS table factory generator.
@@ -229,6 +342,54 @@ DeregisterSmbiosTableGenerator (
IN CONST SMBIOS_TABLE_GENERATOR *CONST Generator
);

+/** Add SMBIOS Handle.
+
+ This function is called by the Dynamic Table Manager to add a newly added
+ SMBIOS Table OR it can be called by an SMBIOS Table generator to create
+ and add a new SMBIOS Handle if there is a reference to another table and
+ if there is a generator for that table that hasn't been called yet.
+
+ @param [in] Smbios Pointer to the SMBIOS protocol.
+ @param [in] SmbiosHandle Pointer to an SMBIOS handle (either generated by
+ SmbiosDxe Driver or SMBIOS_HANDLE_PI_RESERVED
+ if a handle needs to be generated).
+ @param [in] CmObjectToken The CM Object token for that is used to generate
+ SMBIOS record.
+ @param [in] SmbiosId The SMBIOS table generator Id.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_OUT_OF_RESOURCES Unable to add the handle.
+ @retval EFI_NOT_FOUND The requested generator is not found
+ in the list of registered generators.
+**/
+EFI_STATUS
+EFIAPI
+AddSmbiosHandle (
+ IN EFI_SMBIOS_PROTOCOL *Smbios,
+ IN SMBIOS_HANDLE *SmbiosHandle,
+ IN CM_OBJECT_TOKEN CmObjectToken,
+ IN ESTD_SMBIOS_TABLE_ID SmbiosId
+ );
+
+/** Find SMBIOS Handle given the CM Object token used to generate the SMBIOS
+ record..
+
+ This function is called by the SMBIOS table generator to find an SMBIOS
+ handle needed as part of generating an SMBIOS Table.
+
+ @param [in] CmObjectToken The CM Object token used to generate the SMBIOS
+ record.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND The requested generator is not found
+ in the list of registered generators.
+**/
+SMBIOS_HANDLE_MAP *
+EFIAPI
+FindSmbiosHandle (
+ IN CM_OBJECT_TOKEN CmObjectToken
+ );
+
#pragma pack()

#endif // SMBIOS_TABLE_GENERATOR_H_
--
2.17.1