[edk2-devel] [PATCH v4 23/28] OvmfPkg: Enable Choosing Memory Protection Profile via QemuCfg

["Taylor Beebe" <taylor.d.beebe@gmail.com>]

Now that the EDK2 tree uses GetMemoryProtectionsLib to query
the platform memory protection settings, OvmfPkg can be updated
to use QemuCfg to set the entire memory protection profile instead
of just SetNxForStack.

For example, the following will set the DXE memory protection to
the RELEASE preset.
-fw_cfg name=opt/org.tianocore/DxeMemoryProtectionProfile,string=release

The following will set the MM memory protection to
the RELEASE preset.
-fw_cfg name=opt/org.tianocore/MmMemoryProtectionProfile,string=release

For users of Stuart, DXE_MEMORY_PROTECTION_PROFILE=release and
MM_MEMORY_PROTECTION_PROFILE=release are equivalent to the above
examples.

Signed-off-by: Taylor Beebe <taylor.d.beebe@gmail.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Peter Grehan <grehan@freebsd.org>
Cc: Corvin Köhne <corvink@freebsd.org>
---
 OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c      | 21 +++++++-----
 OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c   | 13 +-------
 OvmfPkg/Library/PlatformInitLib/Platform.c              | 15 ---------
 OvmfPkg/PlatformPei/IntelTdx.c                          |  2 --
 OvmfPkg/PlatformPei/Platform.c                          | 35 ++++++++------------
 OvmfPkg/TdxDxe/TdxDxe.c                                 |  7 ++--
 OvmfPkg/Include/Library/PlatformInitLib.h               | 13 --------
 OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf |  2 +-
 OvmfPkg/PlatformCI/PlatformBuildLib.py                  |  8 +++++
 OvmfPkg/PlatformPei/PlatformPei.inf                     |  1 +
 10 files changed, 39 insertions(+), 78 deletions(-)

diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
index cf645aad3246..2f8fd51f3fc5 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartup.c
@@ -20,7 +20,7 @@
 #include <Library/PlatformInitLib.h>
 #include <Library/TdxHelperLib.h>
 #include <Library/SetMemoryProtectionsLib.h>
-#include <Library/QemuFwCfgSimpleParserLib.h>
+#include <Library/MemoryProtectionConfigLib.h>
 #include <ConfidentialComputingGuestAttr.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <OvmfPlatforms.h>
@@ -109,18 +109,23 @@ InitializePlatform (
 
   PlatformMemMapInitialization (PlatformInfoHob);
 
-  DxeSettings                                 = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsPcd].Settings;
-  MmSettings                                  = MmMemoryProtectionProfiles[MmMemoryProtectionSettingsPcd].Settings;
-  DxeSettings.StackExecutionProtectionEnabled = PcdGetBool (PcdSetNxForStack);
-  QemuFwCfgParseBool ("opt/ovmf/PcdSetNxForStack", &DxeSettings.StackExecutionProtectionEnabled);
+  if (EFI_ERROR (ParseFwCfgDxeMemoryProtectionSettings (&DxeSettings))) {
+    DxeSettings = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsRelease].Settings;
+  }
 
-  SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsPcd);
-  SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsPcd);
+  if (EFI_ERROR (ParseFwCfgMmMemoryProtectionSettings (&MmSettings))) {
+    MmSettings = MmMemoryProtectionProfiles[MmMemoryProtectionSettingsOff].Settings;
+  }
+
+  // Always disable NullPointerDetection in EndOfDxe phase for shim compatability
+  DxeSettings.NullPointerDetection.DisableEndOfDxe = TRUE;
+
+  SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsRelease);
+  SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsOff);
 
   if (TdIsEnabled ()) {
     PlatformInfoHob->PcdConfidentialComputingGuestAttr = CCAttrIntelTdx;
     PlatformInfoHob->PcdTdxSharedBitMask               = TdSharedPageMask ();
-    PlatformInfoHob->PcdSetNxForStack                  = TRUE;
   }
 
   PlatformMiscInitialization (PlatformInfoHob);
diff --git a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
index 41521e3d3d71..7ae9b5743810 100644
--- a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
+++ b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
@@ -53,18 +53,7 @@ IsSetNxForStack (
   VOID
   )
 {
-  EFI_HOB_GUID_TYPE      *GuidHob;
-  EFI_HOB_PLATFORM_INFO  *PlatformInfo;
-
-  GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
-  if (GuidHob == NULL) {
-    ASSERT (FALSE);
-    return FALSE;
-  }
-
-  PlatformInfo = (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
-
-  return PlatformInfo->PcdSetNxForStack;
+  return mMps.Dxe.StackExecutionProtectionEnabled;
 }
 
 /**
diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c
index f48bf16ae300..bc9becc4016e 100644
--- a/OvmfPkg/Library/PlatformInitLib/Platform.c
+++ b/OvmfPkg/Library/PlatformInitLib/Platform.c
@@ -249,21 +249,6 @@ PlatformMemMapInitialization (
   PlatformInfoHob->PcdPciIoSize = PciIoSize;
 }
 
-/**
- * Fetch "opt/ovmf/PcdSetNxForStack" from QEMU
- *
- * @param Setting     The pointer to the setting of "/opt/ovmf/PcdSetNxForStack".
- * @return EFI_SUCCESS  Successfully fetch the settings.
- */
-EFI_STATUS
-EFIAPI
-PlatformNoexecDxeInitialization (
-  IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
-  )
-{
-  return QemuFwCfgParseBool ("opt/ovmf/PcdSetNxForStack", &PlatformInfoHob->PcdSetNxForStack);
-}
-
 VOID
 PciExBarInitialization (
   VOID
diff --git a/OvmfPkg/PlatformPei/IntelTdx.c b/OvmfPkg/PlatformPei/IntelTdx.c
index 3d625cabd844..1cb6729e56e6 100644
--- a/OvmfPkg/PlatformPei/IntelTdx.c
+++ b/OvmfPkg/PlatformPei/IntelTdx.c
@@ -48,7 +48,5 @@ IntelTdxInitialize (
   PcdStatus = PcdSet64S (PcdTdxSharedBitMask, TdSharedPageMask ());
   ASSERT_RETURN_ERROR (PcdStatus);
 
-  PcdStatus = PcdSetBoolS (PcdSetNxForStack, TRUE);
-  ASSERT_RETURN_ERROR (PcdStatus);
  #endif
 }
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index bcd8d3a1be14..0df1277f34c4 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -39,6 +39,7 @@
 #include <Library/MemEncryptSevLib.h>
 #include <OvmfPlatforms.h>
 #include <Library/SetMemoryProtectionsLib.h>
+#include <Library/MemoryProtectionConfigLib.h>
 
 #include "Platform.h"
 
@@ -74,21 +75,6 @@ MemMapInitialization (
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
-STATIC
-VOID
-NoexecDxeInitialization (
-  IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
-  )
-{
-  RETURN_STATUS  Status;
-
-  Status = PlatformNoexecDxeInitialization (PlatformInfoHob);
-  if (!RETURN_ERROR (Status)) {
-    Status = PcdSetBoolS (PcdSetNxForStack, PlatformInfoHob->PcdSetNxForStack);
-    ASSERT_RETURN_ERROR (Status);
-  }
-}
-
 static const UINT8  EmptyFdt[] = {
   0xd0, 0x0d, 0xfe, 0xed, 0x00, 0x00, 0x00, 0x48,
   0x00, 0x00, 0x00, 0x38, 0x00, 0x00, 0x00, 0x48,
@@ -345,13 +331,19 @@ InitializePlatform (
 
   PublishPeiMemory (PlatformInfoHob);
 
-  DxeSettings                                 = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsPcd].Settings;
-  MmSettings                                  = MmMemoryProtectionProfiles[MmMemoryProtectionSettingsPcd].Settings;
-  DxeSettings.StackExecutionProtectionEnabled = PcdGetBool (PcdSetNxForStack);
-  QemuFwCfgParseBool ("opt/ovmf/PcdSetNxForStack", &DxeSettings.StackExecutionProtectionEnabled);
+  if (EFI_ERROR (ParseFwCfgDxeMemoryProtectionSettings (&DxeSettings))) {
+    DxeSettings = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsRelease].Settings;
+  }
 
-  SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsPcd);
-  SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsPcd);
+  if (EFI_ERROR (ParseFwCfgMmMemoryProtectionSettings (&MmSettings))) {
+    MmSettings = MmMemoryProtectionProfiles[MmMemoryProtectionSettingsOff].Settings;
+  }
+
+  // Always disable NullPointerDetection in EndOfDxe phase for shim compatability
+  DxeSettings.NullPointerDetection.DisableEndOfDxe = TRUE;
+
+  SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsRelease);
+  SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsOff);
 
   PlatformQemuUc32BaseInitialization (PlatformInfoHob);
 
@@ -365,7 +357,6 @@ InitializePlatform (
     PeiFvInitialization (PlatformInfoHob);
     MemTypeInfoInitialization (PlatformInfoHob);
     MemMapInitialization (PlatformInfoHob);
-    NoexecDxeInitialization (PlatformInfoHob);
   }
 
   InstallClearCacheCallback ();
diff --git a/OvmfPkg/TdxDxe/TdxDxe.c b/OvmfPkg/TdxDxe/TdxDxe.c
index 30732f421bb6..5e497ba66227 100644
--- a/OvmfPkg/TdxDxe/TdxDxe.c
+++ b/OvmfPkg/TdxDxe/TdxDxe.c
@@ -131,15 +131,12 @@ SetPcdSettings (
 
   PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, PlatformInfoHob->PcdConfidentialComputingGuestAttr);
   ASSERT_RETURN_ERROR (PcdStatus);
-  PcdStatus = PcdSetBoolS (PcdSetNxForStack, PlatformInfoHob->PcdSetNxForStack);
-  ASSERT_RETURN_ERROR (PcdStatus);
 
   DEBUG ((
     DEBUG_INFO,
-    "HostBridgeDevId=0x%x, CCAttr=0x%x, SetNxForStack=%x\n",
+    "HostBridgeDevId=0x%x, CCAttr=0x%x\n",
     PlatformInfoHob->HostBridgeDevId,
-    PlatformInfoHob->PcdConfidentialComputingGuestAttr,
-    PlatformInfoHob->PcdSetNxForStack
+    PlatformInfoHob->PcdConfidentialComputingGuestAttr
     ));
 
   PcdStatus = PcdSet32S (PcdCpuBootLogicalProcessorNumber, PlatformInfoHob->PcdCpuBootLogicalProcessorNumber);
diff --git a/OvmfPkg/Include/Library/PlatformInitLib.h b/OvmfPkg/Include/Library/PlatformInitLib.h
index 57b18b94d9b8..b2468f206321 100644
--- a/OvmfPkg/Include/Library/PlatformInitLib.h
+++ b/OvmfPkg/Include/Library/PlatformInitLib.h
@@ -32,7 +32,6 @@ typedef struct {
   UINT32               Uc32Base;
   UINT32               Uc32Size;
 
-  BOOLEAN              PcdSetNxForStack;
   UINT64               PcdTdxSharedBitMask;
 
   UINT64               PcdPciMmio64Base;
@@ -182,18 +181,6 @@ PlatformMemMapInitialization (
   IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
   );
 
-/**
- * Fetch "opt/ovmf/PcdSetNxForStack" from QEMU
- *
- * @param Setting     The pointer to the setting of "/opt/ovmf/PcdSetNxForStack".
- * @return EFI_SUCCESS  Successfully fetch the settings.
- */
-EFI_STATUS
-EFIAPI
-PlatformNoexecDxeInitialization (
-  IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
-  );
-
 VOID
 EFIAPI
 PlatformMiscInitialization (
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
index 47bd42d23d11..a6d7b53f52cf 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
@@ -57,7 +57,7 @@ [LibraryClasses]
   QemuFwCfgLib
   PlatformInitLib
   SetMemoryProtectionsLib
-  QemuFwCfgSimpleParserLib
+  MemoryProtectionConfigLib
 
 [Guids]
   gEfiHobMemoryAllocModuleGuid
diff --git a/OvmfPkg/PlatformCI/PlatformBuildLib.py b/OvmfPkg/PlatformCI/PlatformBuildLib.py
index f829738cdda4..0d5d39c078d0 100644
--- a/OvmfPkg/PlatformCI/PlatformBuildLib.py
+++ b/OvmfPkg/PlatformCI/PlatformBuildLib.py
@@ -183,6 +183,8 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
         VirtualDrive = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "VirtualDrive")
         os.makedirs(VirtualDrive, exist_ok=True)
         OutputPath_FV = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "FV")
+        DxeMemoryProtection = self.env.GetValue("DXE_MEMORY_PROTECTION_PROFILE", "")
+        MmMemoryProtection = self.env.GetValue("MM_MEMORY_PROTECTION_PROFILE", "")
 
         if (self.env.GetValue("QEMU_SKIP") and
             self.env.GetValue("QEMU_SKIP").upper() == "TRUE"):
@@ -199,6 +201,12 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
         args += " -smp 4"
         args += f" -drive file=fat:rw:{VirtualDrive},format=raw,media=disk" # Mount disk with startup.nsh
 
+        if (DxeMemoryProtection.lower() != ""):
+            args += " -fw_cfg name=opt/org.tianocore/DxeMemoryProtectionProfile,string=" + DxeMemoryProtection.lower()
+
+        if (MmMemoryProtection.lower() != ""):
+            args += " -fw_cfg name=opt/org.tianocore/MmMemoryProtectionProfile,string=" + MmMemoryProtection.lower()
+
         if (self.env.GetValue("QEMU_HEADLESS").upper() == "TRUE"):
             args += " -display none"  # no graphics
 
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6b8442d12b2c..fbaa6bdc8ee5 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -66,6 +66,7 @@ [LibraryClasses]
   CcExitLib
   PlatformInitLib
   SetMemoryProtectionsLib
+  MemoryProtectionConfigLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
-- 
2.42.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108885): https://edk2.groups.io/g/devel/message/108885
Mute This Topic: https://groups.io/mt/101469963/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-