RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

OvmfPkg/PlatformPei is updated to support Tdx guest. There are below
major changes.
 - Set Tdx related PCDs
 - Build Tdx PlatformInfoHob
 - Publish Tdx RamRegions

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/OvmfPkg.dec                  |  1 +
 OvmfPkg/PlatformPei/FeatureControl.c |  8 +++-
 OvmfPkg/PlatformPei/IntelTdx.c       | 61 ++++++++++++++++++++++++++++
 OvmfPkg/PlatformPei/MemDetect.c      | 13 +++++-
 OvmfPkg/PlatformPei/Platform.c       |  1 +
 OvmfPkg/PlatformPei/Platform.h       | 11 +++++
 OvmfPkg/PlatformPei/PlatformPei.inf  |  7 ++++
 7 files changed, 100 insertions(+), 2 deletions(-)
 create mode 100644 OvmfPkg/PlatformPei/IntelTdx.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 230d24bc7299..7765962692b6 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -133,6 +133,7 @@
   gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
   gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
   gConfidentialComputingSevSnpBlobGuid  = {0x067b1f5f, 0xcf26, 0x44c5, {0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42}}
+  gUefiOvmfPkgTdxPlatformGuid           = {0xdec9b486, 0x1f16, 0x47c7, {0x8f, 0x68, 0xdf, 0x1a, 0x41, 0x88, 0x8b, 0xa5}}
 
 [Ppis]
   # PPI whose presence in the PPI database signals that the TPM base address
diff --git a/OvmfPkg/PlatformPei/FeatureControl.c b/OvmfPkg/PlatformPei/FeatureControl.c
index 9af58c2655f8..7ef1e9565869 100644
--- a/OvmfPkg/PlatformPei/FeatureControl.c
+++ b/OvmfPkg/PlatformPei/FeatureControl.c
@@ -12,6 +12,8 @@
 #include <Library/QemuFwCfgLib.h>
 #include <Ppi/MpServices.h>
 #include <Register/ArchitecturalMsr.h>
+#include <Library/TdxLib.h>
+#include <IndustryStandard/Tdx.h>
 
 #include "Platform.h"
 
@@ -37,7 +39,11 @@ WriteFeatureControl (
   IN OUT VOID  *WorkSpace
   )
 {
-  AsmWriteMsr64 (MSR_IA32_FEATURE_CONTROL, mFeatureControlValue);
+  if (TdIsEnabled ()) {
+    TdVmCall (TDVMCALL_WRMSR, (UINT64)MSR_IA32_FEATURE_CONTROL, mFeatureControlValue, 0, 0, 0);
+  } else {
+    AsmWriteMsr64 (MSR_IA32_FEATURE_CONTROL, mFeatureControlValue);
+  }
 }
 
 /**
diff --git a/OvmfPkg/PlatformPei/IntelTdx.c b/OvmfPkg/PlatformPei/IntelTdx.c
new file mode 100644
index 000000000000..09e3a4ceeb3f
--- /dev/null
+++ b/OvmfPkg/PlatformPei/IntelTdx.c
@@ -0,0 +1,61 @@
+/** @file
+  Initialize Intel TDX support.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/BaseMemoryLib.h>
+#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 <WorkArea.h>
+#include <ConfidentialComputingGuestAttr.h>
+#include "Platform.h"
+
+/**
+  This Function checks if TDX is available, if present then it sets
+  the dynamic PCDs for Tdx guest. It also builds Guid hob which contains
+  the Host Bridge DevId.
+  **/
+VOID
+IntelTdxInitialize (
+  VOID
+  )
+{
+ #ifdef MDE_CPU_X64
+  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
+  RETURN_STATUS          PcdStatus;
+
+  if (!TdIsEnabled ()) {
+    return;
+  }
+
+  PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrIntelTdx);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  PcdStatus = PcdSetBoolS (PcdIa32EferChangeAllowed, FALSE);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  PcdStatus = PcdSet64S (PcdTdxSharedBitMask, TdSharedPageMask ());
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  PcdStatus = PcdSetBoolS (PcdSetNxForStack, TRUE);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  ZeroMem (&PlatformInfoHob, sizeof (PlatformInfoHob));
+  PlatformInfoHob.HostBridgePciDevId = mHostBridgeDevId;
+
+  BuildGuidDataHob (&gUefiOvmfPkgTdxPlatformGuid, &PlatformInfoHob, sizeof (EFI_HOB_PLATFORM_INFO));
+ #endif
+}
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 0c930ab1d70e..fcd17e256ad0 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -37,6 +37,7 @@ Module Name:
 #include <Library/QemuFwCfgLib.h>
 #include <Library/QemuFwCfgSimpleParserLib.h>
 #include <Library/PlatformInitLib.h>
+#include <Library/TdxLib.h>
 #include "Platform.h"
 
 UINT8  mPhysMemAddressWidth;
@@ -218,7 +219,12 @@ GetPeiMemoryCap (
     PdpEntries  = 1 << (mPhysMemAddressWidth - 30);
     ASSERT (PdpEntries <= 0x200);
   } else {
-    Pml4Entries = 1 << (mPhysMemAddressWidth - 39);
+    if (TdIsEnabled ()) {
+      Pml4Entries = 0x200;
+    } else {
+      Pml4Entries = 1 << (mPhysMemAddressWidth - 39);
+    }
+
     ASSERT (Pml4Entries <= 0x200);
     PdpEntries = 512;
   }
@@ -333,6 +339,11 @@ InitializeRamRegions (
   VOID
   )
 {
+  if (TdIsEnabled ()) {
+    PlatformTdxPublishRamRegions ();
+    return;
+  }
+
   PlatformInitializeRamRegions (
     mQemuUc32Base,
     mHostBridgeDevId,
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 05a5c94ce80a..1be8b826227d 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -436,6 +436,7 @@ InitializePlatform (
 
   InstallClearCacheCallback ();
   AmdSevInitialize ();
+  IntelTdxInitialize ();
   MiscInitialization ();
   InstallFeatureControlCallback ();
 
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 64af9cde1002..a8484b3fa374 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -10,6 +10,7 @@
 #define _PLATFORM_PEI_H_INCLUDED_
 
 #include <IndustryStandard/E820.h>
+#include <IndustryStandard/IntelTdx.h>
 
 VOID
 AddressWidthInitialization (
@@ -66,6 +67,16 @@ AmdSevInitialize (
   VOID
   );
 
+/**
+  This Function checks if TDX is available, if present then it sets
+  the dynamic PCDs for Tdx guest. It also builds Guid hob which contains
+  the Host Bridge DevId.
+  **/
+VOID
+IntelTdxInitialize (
+  VOID
+  );
+
 extern EFI_BOOT_MODE  mBootMode;
 
 VOID
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 65e417b2f254..2f9f0280b35d 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -31,6 +31,7 @@
   MemTypeInfo.c
   Platform.c
   Platform.h
+  IntelTdx.c
 
 [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
@@ -43,6 +44,7 @@
 [Guids]
   gEfiMemoryTypeInformationGuid
   gFdtHobGuid
+  gUefiOvmfPkgTdxPlatformGuid
 
 [LibraryClasses]
   BaseLib
@@ -64,6 +66,9 @@
   VmgExitLib
   PlatformInitLib
 
+[LibraryClasses.X64]
+  TdxLib
+
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
@@ -109,6 +114,8 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
   gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
   gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures
+  gEfiMdeModulePkgTokenSpaceGuid.PcdIa32EferChangeAllowed
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask
 
 [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85995): https://edk2.groups.io/g/devel/message/85995
Mute This Topic: https://groups.io/mt/88617550/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

  Hi,

> +/**
> +  This Function checks if TDX is available, if present then it sets
> +  the dynamic PCDs for Tdx guest. It also builds Guid hob which contains
> +  the Host Bridge DevId.
> +  **/
> +VOID
> +IntelTdxInitialize (
> +  VOID
> +  )
> +{
> + #ifdef MDE_CPU_X64
> +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> +  RETURN_STATUS          PcdStatus;
> +
> +  if (!TdIsEnabled ()) {
> +    return;
> +  }
> +
> +  PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrIntelTdx);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  PcdStatus = PcdSetBoolS (PcdIa32EferChangeAllowed, FALSE);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  PcdStatus = PcdSet64S (PcdTdxSharedBitMask, TdSharedPageMask ());
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  PcdStatus = PcdSetBoolS (PcdSetNxForStack, TRUE);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  ZeroMem (&PlatformInfoHob, sizeof (PlatformInfoHob));
> +  PlatformInfoHob.HostBridgePciDevId = mHostBridgeDevId;
> +
> +  BuildGuidDataHob (&gUefiOvmfPkgTdxPlatformGuid, &PlatformInfoHob, sizeof (EFI_HOB_PLATFORM_INFO));
> + #endif
> +}

So, what is the plan for this with pei-less boot?

I think we should move this to PlatformInitLib, then link either into
PlatformPei or the early dxe module for pei-less boot?

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86113): https://edk2.groups.io/g/devel/message/86113
Mute This Topic: https://groups.io/mt/88617550/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi
 
> > +/**
> > +  This Function checks if TDX is available, if present then it sets
> > +  the dynamic PCDs for Tdx guest. It also builds Guid hob which
> > +contains
> > +  the Host Bridge DevId.
> > +  **/
> > +VOID
> > +IntelTdxInitialize (
> > +  VOID
> > +  )
> > +{
> > + #ifdef MDE_CPU_X64
> > +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> > +  RETURN_STATUS          PcdStatus;
> > +
> > +  if (!TdIsEnabled ()) {
> > +    return;
> > +  }
> > +
> > +  PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr,
> > + CCAttrIntelTdx);  ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSetBoolS (PcdIa32EferChangeAllowed, FALSE);
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSet64S (PcdTdxSharedBitMask, TdSharedPageMask ());
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSetBoolS (PcdSetNxForStack, TRUE);
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  ZeroMem (&PlatformInfoHob, sizeof (PlatformInfoHob));
> > + PlatformInfoHob.HostBridgePciDevId = mHostBridgeDevId;
> > +
> > +  BuildGuidDataHob (&gUefiOvmfPkgTdxPlatformGuid, &PlatformInfoHob,
> > +sizeof (EFI_HOB_PLATFORM_INFO));  #endif }
> 
> So, what is the plan for this with pei-less boot?
In Pei-less boot PCDs cannot be set. So these settings are saved in PlatformInfoHob which will be set in early Dxe phase.
> 
> I think we should move this to PlatformInitLib, then link either into
> PlatformPei or the early dxe module for pei-less boot?
> 
1. PlatformInitLib is designed without Dynamic PCDs because it is for both SEC and PEI.

2. When boot with PEI phase, some PCDs are mandatory.
For example, PcdIa32EferChangeAllowed indicates whether IA32_EFER can be modified or not. Because in Tdx guest change of IA32_EFER is not allowed. But in boot with PEI phase, DxeIplPei tries to update IA32_EFER (see IsEnableNonExecNeeded).
Another example is PcdConfidentialComputingGuestAttr. It is used in MpInitLib/MpLib.c (for CpuMpPei).

3. In Pei-less boot, there is no such limitation. Because the PCDs can be set before they're consumed.

Based on above consideration, I would suggest keep above logics.

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86724): https://edk2.groups.io/g/devel/message/86724
Mute This Topic: https://groups.io/mt/88617550/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-