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 | 33 +++++++-------------
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, 37 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..fa76ad2de202 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,17 @@ 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))) {
+ SetDxeMemoryProtectionSettings (NULL, DxeMemoryProtectionSettingsGrubCompat);
+ } else {
+ SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsGrubCompat);
+ }
- SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsPcd);
- SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsPcd);
+ if (EFI_ERROR (ParseFwCfgMmMemoryProtectionSettings (&MmSettings))) {
+ SetMmMemoryProtectionSettings (NULL, MmMemoryProtectionSettingsOff);
+ } else {
+ SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsOff);
+ }
PlatformQemuUc32BaseInitialization (PlatformInfoHob);
@@ -365,7 +355,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 (#109428): https://edk2.groups.io/g/devel/message/109428
Mute This Topic: https://groups.io/mt/101843367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 10/9/23 02:07, Taylor Beebe wrote:
> 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.
both the commit message and the subject say QemuCfg; should be QemuFwCfg perhaps.
>
> 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.
The lowercase "release" above does not match "Release" from patch 20 (MdeModulePkg: Add Additional Profiles to SetMemoryProtectionsLib).
Which makes me notice that you used AsciiStr*i*Cmp() in patch 22.
... I think that's unfortunate; it guarantees that scripts passing in protection profile names will use inconsistent casing, so grepping for profile names will be limited (people will forget to grep case-insensitively). I'd rather be strict with profile names.
>
> 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 | 33 +++++++-------------
> 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, 37 insertions(+), 78 deletions(-)
This patch is way too big IMO, not by line count, but by number of actions taken.
[cut]
> 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/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
One one hand: OK, these look consistent, they keep the HOB interface and the lib header minimal and in sync.
However, removing the "opt/ovmf/PcdSetNxForStack" fw_cfg knob is a compat breaking change. Do we have a plan for announcing that somehow?
[cut]
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index bcd8d3a1be14..fa76ad2de202 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,
OK
> @@ -345,13 +331,17 @@ 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))) {
> + SetDxeMemoryProtectionSettings (NULL, DxeMemoryProtectionSettingsGrubCompat);
> + } else {
> + SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsGrubCompat);
> + }
>
> - SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsPcd);
> - SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsPcd);
> + if (EFI_ERROR (ParseFwCfgMmMemoryProtectionSettings (&MmSettings))) {
> + SetMmMemoryProtectionSettings (NULL, MmMemoryProtectionSettingsOff);
> + } else {
> + SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsOff);
> + }
>
> PlatformQemuUc32BaseInitialization (PlatformInfoHob);
>
OVMF can be built without SMM_REQUIRE; I think setting an MM profile in that case is wasteful (but minimally: confusing). What's more...
> @@ -365,7 +355,6 @@ InitializePlatform (
> PeiFvInitialization (PlatformInfoHob);
> MemTypeInfoInitialization (PlatformInfoHob);
> MemMapInitialization (PlatformInfoHob);
> - NoexecDxeInitialization (PlatformInfoHob);
> }
>
> InstallClearCacheCallback ();
... your patch too shows that the new logic isn't truly compatible, or isn't *compatibly placed*, with the old logic; the old logic is restricted to non-S3 boot paths (in effect just the boot with full config boot path).
> 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
Hmm. I would have expected the QemuFwCfgSimpleParserLib dependency to be removed here.
However, I realize: in patch v5 11/28 ("OvmfPkg: Apply Memory Protections via SetMemoryProtectionsLib"), you do *not* add QemuFwCfgSimpleParserLib to "PlatformPei.inf", even though you introduce a new QemuFwCfgParseBool() call. So it's only fair that, when you now remove the QemuFwCfgParseBool() call, you don't remove the QemuFwCfgSimpleParserLib dependency either.
But that only highlights a preexistent problem: even before your patch set, "OvmfPkg/PlatformPei" doesn't depend on QemuFwCfgSimpleParserLib! There isn't a single QemuFwCfgParse*() call in it!
So that's a preexistent bug, namely from commit 96047b6663ab ("OvmfPkg/PlatformInitLib: Move functions to Platform.c", 2022-04-02). That was when the last QemuFwCfgParse*() reference was removed from OvmfPkg/PlatformPei, but the lib class header inclusion and the lib class dependency weren't cleaned up. Splendid.
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109449): https://edk2.groups.io/g/devel/message/109449
Mute This Topic: https://groups.io/mt/101843367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.