From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
"Jordan Justen" <jordan.l.justen@intel.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH 5/5] OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation
Date: Tue, 10 Mar 2020 23:27:39 +0100 [thread overview]
Message-ID: <20200310222739.26717-6-lersek@redhat.com> (raw)
In-Reply-To: <20200310222739.26717-1-lersek@redhat.com>
* In the Intel whitepaper:
--v--
A Tour Beyond BIOS -- Secure SMM Communication
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Security-White-Papers
https://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Secure_SMM_Communication.pdf
--^--
bullet#3 in section "Assumption and Recommendation", and bullet#4 in "Call
for action", recommend enabling the (adaptive) Memory Type Information
feature.
* In the Intel whitepaper:
--v--
A Tour Beyond BIOS -- Memory Map and Practices in UEFI BIOS
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-white-papers
https://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Memory_Map_And_Practices_in_UEFI_BIOS_V2.pdf
--^--
figure#6 describes the Memory Type Information feature in detail; namely
as a feedback loop between the Platform PEIM, the DXE IPL PEIM, the DXE
Core, and BDS.
Implement the missing PlatformPei functionality in OvmfPkg, for fulfilling
the Secure SMM Communication recommendation.
In the longer term, OVMF should install the WSMT ACPI table, and this
patch contributes to that.
Notes:
- the step in figure#6 where the UEFI variable is copied into the HOB is
covered by the DXE IPL PEIM, in the DxeLoadCore() function,
- "PcdResetOnMemoryTypeInformationChange" must be reverted to the DEC
default TRUE value, because both whitepapers indicate that BDS needs to
reset the system if the Memory Type Information changes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
OvmfPkg/OvmfPkgX64.dsc | 2 +
OvmfPkg/PlatformPei/PlatformPei.inf | 2 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/PlatformPei/MemTypeInfo.c | 147 ++++++++++++++++++++
OvmfPkg/PlatformPei/Platform.c | 23 +--
7 files changed, 161 insertions(+), 22 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 4c9ff419c462..02ca17db8b2a 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -448,7 +448,9 @@ [PcdsFeatureFlag]
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
+!if $(SMM_REQUIRE) == FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+!endif
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 56681e9e4580..d08cf558c6aa 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -452,7 +452,9 @@ [PcdsFeatureFlag]
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
+!if $(SMM_REQUIRE) == FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+!endif
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0e74b6f4d26c..b2dccc40a865 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -452,7 +452,9 @@ [PcdsFeatureFlag]
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
+!if $(SMM_REQUIRE) == FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+!endif
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index c51a6176aa2e..8531c63995c1 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -30,6 +30,7 @@ [Sources]
FeatureControl.c
Fv.c
MemDetect.c
+ MemTypeInfo.c
Platform.c
Platform.h
Xen.c
@@ -112,6 +113,7 @@ [FeaturePcd]
[Ppis]
gEfiPeiMasterBootModePpiGuid
gEfiPeiMpServicesPpiGuid
+ gEfiPeiReadOnlyVariable2PpiGuid
[Depex]
TRUE
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 43f20f067f22..0484ec9e6b4c 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -82,6 +82,11 @@ PeiFvInitialization (
VOID
);
+VOID
+MemTypeInfoInitialization (
+ VOID
+ );
+
VOID
InstallFeatureControlCallback (
VOID
diff --git a/OvmfPkg/PlatformPei/MemTypeInfo.c b/OvmfPkg/PlatformPei/MemTypeInfo.c
new file mode 100644
index 000000000000..c709236a457a
--- /dev/null
+++ b/OvmfPkg/PlatformPei/MemTypeInfo.c
@@ -0,0 +1,147 @@
+/** @file
+ Produce a default memory type information HOB unless we can determine, from
+ the existence of the "MemoryTypeInformation" variable, that the DXE IPL PEIM
+ will produce the HOB.
+
+ Copyright (C) 2017-2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Guid/MemoryTypeInformation.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Ppi/ReadOnlyVariable2.h>
+#include <Uefi/UefiMultiPhase.h>
+
+#include "Platform.h"
+
+STATIC EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {
+ { EfiACPIMemoryNVS, 0x004 },
+ { EfiACPIReclaimMemory, 0x008 },
+ { EfiReservedMemoryType, 0x004 },
+ { EfiRuntimeServicesData, 0x024 },
+ { EfiRuntimeServicesCode, 0x030 },
+ { EfiBootServicesCode, 0x180 },
+ { EfiBootServicesData, 0xF00 },
+ { EfiMaxMemoryType, 0x000 }
+};
+
+STATIC
+VOID
+BuildMemTypeInfoHob (
+ VOID
+ )
+{
+ BuildGuidDataHob (
+ &gEfiMemoryTypeInformationGuid,
+ mDefaultMemoryTypeInformation,
+ sizeof mDefaultMemoryTypeInformation
+ );
+ DEBUG ((DEBUG_INFO, "%a: default memory type information HOB built\n",
+ __FUNCTION__));
+}
+
+/**
+ Notification function called when EFI_PEI_READ_ONLY_VARIABLE2_PPI becomes
+ available.
+
+ @param[in] PeiServices Indirect reference to the PEI Services Table.
+ @param[in] NotifyDescriptor Address of the notification descriptor data
+ structure.
+ @param[in] Ppi Address of the PPI that was installed.
+
+ @return Status of the notification. The status code returned from this
+ function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+OnReadOnlyVariable2Available (
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor,
+ IN VOID *Ppi
+ )
+{
+ EFI_PEI_READ_ONLY_VARIABLE2_PPI *ReadOnlyVariable2;
+ UINTN DataSize;
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_VERBOSE, "%a\n", __FUNCTION__));
+
+ //
+ // Check if the "MemoryTypeInformation" variable exists, in the
+ // gEfiMemoryTypeInformationGuid namespace.
+ //
+ ReadOnlyVariable2 = Ppi;
+ DataSize = 0;
+ Status = ReadOnlyVariable2->GetVariable (
+ ReadOnlyVariable2,
+ EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME,
+ &gEfiMemoryTypeInformationGuid,
+ NULL,
+ &DataSize,
+ NULL
+ );
+ switch (Status) {
+ case EFI_BUFFER_TOO_SMALL:
+ //
+ // The variable exists; the DXE IPL PEIM will build the HOB from it.
+ //
+ break;
+ case EFI_NOT_FOUND:
+ //
+ // The variable does not exist; install the default memory type information
+ // HOB.
+ //
+ BuildMemTypeInfoHob ();
+ break;
+ default:
+ DEBUG ((DEBUG_ERROR, "%a: unexpected: GetVariable(): %r\n", __FUNCTION__,
+ Status));
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ break;
+ }
+
+ return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EFI_PEI_READ_ONLY_VARIABLE2_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR mReadOnlyVariable2Notify = {
+ (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_DISPATCH |
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), // Flags
+ &gEfiPeiReadOnlyVariable2PpiGuid, // Guid
+ OnReadOnlyVariable2Available // Notify
+};
+
+VOID
+MemTypeInfoInitialization (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+
+ if (!FeaturePcdGet (PcdSmmSmramRequire)) {
+ //
+ // EFI_PEI_READ_ONLY_VARIABLE2_PPI will never be available; install
+ // the default memory type information HOB right away.
+ //
+ BuildMemTypeInfoHob ();
+ return;
+ }
+
+ Status = PeiServicesNotifyPpi (&mReadOnlyVariable2Notify);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to set up R/O Variable 2 callback: %r\n",
+ __FUNCTION__, Status));
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+}
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 473af102783a..587ca68fc210 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -28,7 +28,6 @@
#include <Library/QemuFwCfgLib.h>
#include <Library/QemuFwCfgS3Lib.h>
#include <Library/ResourcePublicationLib.h>
-#include <Guid/MemoryTypeInformation.h>
#include <Ppi/MasterBootMode.h>
#include <IndustryStandard/I440FxPiix4.h>
#include <IndustryStandard/Pci22.h>
@@ -39,18 +38,6 @@
#include "Platform.h"
#include "Cmos.h"
-EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {
- { EfiACPIMemoryNVS, 0x004 },
- { EfiACPIReclaimMemory, 0x008 },
- { EfiReservedMemoryType, 0x004 },
- { EfiRuntimeServicesData, 0x024 },
- { EfiRuntimeServicesCode, 0x030 },
- { EfiBootServicesCode, 0x180 },
- { EfiBootServicesData, 0xF00 },
- { EfiMaxMemoryType, 0x000 }
-};
-
-
EFI_PEI_PPI_DESCRIPTOR mPpiBootMode[] = {
{
EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
@@ -162,15 +149,6 @@ MemMapInitialization (
PciIoBase = 0xC000;
PciIoSize = 0x4000;
- //
- // Create Memory Type Information HOB
- //
- BuildGuidDataHob (
- &gEfiMemoryTypeInformationGuid,
- mDefaultMemoryTypeInformation,
- sizeof(mDefaultMemoryTypeInformation)
- );
-
//
// Video memory + Legacy BIOS region
//
@@ -811,6 +789,7 @@ InitializePlatform (
ReserveEmuVariableNvStore ();
}
PeiFvInitialization ();
+ MemTypeInfoInitialization ();
MemMapInitialization ();
NoexecDxeInitialization ();
}
--
2.19.1.3.g30247aa5d201
next prev parent reply other threads:[~2020-03-10 22:28 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-10 22:27 [PATCH 0/5] OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation Laszlo Ersek
2020-03-10 22:27 ` [PATCH 1/5] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: drop unused PCDs Laszlo Ersek
2020-03-10 22:27 ` [PATCH 2/5] OvmfPkg/QemuFlashFvbServices: factor out SetPcdFlashNvStorageBaseAddresses Laszlo Ersek
2020-03-10 22:27 ` [PATCH 3/5] OvmfPkg: set fixed FlashNvStorage base addresses with -D SMM_REQUIRE Laszlo Ersek
2020-03-11 15:44 ` [edk2-devel] " Leif Lindholm
2020-03-11 16:14 ` Laszlo Ersek
2020-03-11 16:20 ` Leif Lindholm
2020-03-11 16:41 ` Laszlo Ersek
2020-03-12 14:20 ` Leif Lindholm
2020-03-12 22:19 ` Laszlo Ersek
2020-03-10 22:27 ` [PATCH 4/5] OvmfPkg: include FaultTolerantWritePei and VariablePei " Laszlo Ersek
2020-03-10 22:27 ` Laszlo Ersek [this message]
2020-03-11 16:00 ` [edk2-devel] [PATCH 5/5] OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation Leif Lindholm
2020-03-11 16:22 ` Laszlo Ersek
2020-03-11 19:36 ` Leif Lindholm
2020-03-12 0:30 ` Laszlo Ersek
2020-03-12 10:40 ` Leif Lindholm
2020-03-12 21:19 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200310222739.26717-6-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox