File ovmf-bsc1099193-fix-sev-flash-variables.patch of Package ovmf
From 6dad6774dd17c5d032e1d9f6fd996ee0251cc4f8 Mon Sep 17 00:00:00 2001
From: Brijesh Singh <brijesh.singh@amd.com>
Date: Wed, 4 Jul 2018 10:02:16 +0800
Subject: [PATCH 1/4] MdeModulePkg/Variable: Check EFI_MEMORY_RUNTIME attribute
before setting it
Set the EFI_MEMORY_RUNTIME attribute in FtwNotificationEvent() only if
the attribute is not already present. This will ensure that the attributes
set by the platform drivers (e.g Ovmf pflash) is not lost.
Cc: Dong Eric <eric.dong@intel.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Reviewed-by: Star Zeng <star.zeng@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
.../Universal/Variable/RuntimeDxe/VariableDxe.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index b2a373cf98aa..ef488fde1894 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -412,13 +412,15 @@ FtwNotificationEvent (
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "Variable driver failed to get flash memory attribute.\n"));
} else {
- Status = gDS->SetMemorySpaceAttributes (
- BaseAddress,
- Length,
- GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "Variable driver failed to add EFI_MEMORY_RUNTIME attribute to Flash.\n"));
+ if ((GcdDescriptor.Attributes & EFI_MEMORY_RUNTIME) == 0) {
+ Status = gDS->SetMemorySpaceAttributes (
+ BaseAddress,
+ Length,
+ GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "Variable driver failed to add EFI_MEMORY_RUNTIME attribute to Flash.\n"));
+ }
}
}
--
2.18.0
From eb7f4b8315a7f31db520ea1901c9ec4a51b2078b Mon Sep 17 00:00:00 2001
From: Brijesh Singh <brijesh.singh@amd.com>
Date: Fri, 6 Jul 2018 10:00:40 -0500
Subject: [PATCH 2/4] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory
range as MMIO
The flash memory range is an IO address and should be presented as Memory
Mapped IO in EFI Runtime mapping. This information can be used by OS
when mapping the flash memory range.
It is especially helpful in SEV guest case, in which IO addresses should
be mapped as unencrypted. If memory region is not marked as MMIO then OS
maps the range as encrypted.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
---
.../FwBlockService.c | 30 ++++++++++++++-----
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index 558b395dff4a..b3f428bb4284 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -831,12 +831,13 @@ ValidateFvHeader (
STATIC
EFI_STATUS
-MarkMemoryRangeForRuntimeAccess (
+MarkIoMemoryRangeForRuntimeAccess (
EFI_PHYSICAL_ADDRESS BaseAddress,
UINTN Length
)
{
EFI_STATUS Status;
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
//
// Mark flash region as runtime memory
@@ -847,18 +848,31 @@ MarkMemoryRangeForRuntimeAccess (
);
Status = gDS->AddMemorySpace (
- EfiGcdMemoryTypeSystemMemory,
+ EfiGcdMemoryTypeMemoryMappedIo,
BaseAddress,
Length,
EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
);
ASSERT_EFI_ERROR (Status);
- Status = gBS->AllocatePages (
- AllocateAddress,
- EfiRuntimeServicesData,
- EFI_SIZE_TO_PAGES (Length),
- &BaseAddress
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypeMemoryMappedIo,
+ 0,
+ Length,
+ &BaseAddress,
+ gImageHandle,
+ NULL
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->SetMemorySpaceAttributes (
+ BaseAddress,
+ Length,
+ GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
);
ASSERT_EFI_ERROR (Status);
@@ -1091,7 +1105,7 @@ FvbInitialize (
//
InstallProtocolInterfaces (FvbDevice);
- MarkMemoryRangeForRuntimeAccess (BaseAddress, Length);
+ MarkIoMemoryRangeForRuntimeAccess (BaseAddress, Length);
//
// Set several PCD values to point to flash
--
2.18.0
From 49601d19834d3bb3b65be5fe6f7ae2b966e3f348 Mon Sep 17 00:00:00 2001
From: Brijesh Singh <brijesh.singh@amd.com>
Date: Fri, 6 Jul 2018 10:00:41 -0500
Subject: [PATCH 3/4] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose
MMIO in SMM build
In the SMM build, only an SMM driver is using the address range hence we
do not need to expose the flash MMIO range in EFI runtime mapping.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
---
.../FwBlockService.c | 50 -------------------
.../FwBlockService.h | 7 +++
.../FwBlockServiceDxe.c | 50 +++++++++++++++++++
.../FwBlockServiceSmm.c | 13 +++++
4 files changed, 70 insertions(+), 50 deletions(-)
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index b3f428bb4284..eec8b1b1ae9d 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -829,56 +829,6 @@ ValidateFvHeader (
return EFI_SUCCESS;
}
-STATIC
-EFI_STATUS
-MarkIoMemoryRangeForRuntimeAccess (
- EFI_PHYSICAL_ADDRESS BaseAddress,
- UINTN Length
- )
-{
- EFI_STATUS Status;
- EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
-
- //
- // Mark flash region as runtime memory
- //
- Status = gDS->RemoveMemorySpace (
- BaseAddress,
- Length
- );
-
- Status = gDS->AddMemorySpace (
- EfiGcdMemoryTypeMemoryMappedIo,
- BaseAddress,
- Length,
- EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
- );
- ASSERT_EFI_ERROR (Status);
-
- Status = gDS->AllocateMemorySpace (
- EfiGcdAllocateAddress,
- EfiGcdMemoryTypeMemoryMappedIo,
- 0,
- Length,
- &BaseAddress,
- gImageHandle,
- NULL
- );
- ASSERT_EFI_ERROR (Status);
-
- Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor);
- ASSERT_EFI_ERROR (Status);
-
- Status = gDS->SetMemorySpaceAttributes (
- BaseAddress,
- Length,
- GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
- );
- ASSERT_EFI_ERROR (Status);
-
- return Status;
-}
-
STATIC
EFI_STATUS
InitializeVariableFvHeader (
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.h b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.h
index 1f9287b08769..178f578d49f0 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.h
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.h
@@ -189,4 +189,11 @@ VOID
InstallVirtualAddressChangeHandler (
VOID
);
+
+EFI_STATUS
+MarkIoMemoryRangeForRuntimeAccess (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINTN Length
+ );
+
#endif
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
index 63b308658e36..37deece363e6 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -17,6 +17,7 @@
#include <Guid/EventGroup.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
+#include <Library/DxeServicesTableLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeLib.h>
@@ -155,3 +156,52 @@ InstallVirtualAddressChangeHandler (
);
ASSERT_EFI_ERROR (Status);
}
+
+EFI_STATUS
+MarkIoMemoryRangeForRuntimeAccess (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINTN Length
+ )
+{
+ EFI_STATUS Status;
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
+
+ //
+ // Mark flash region as runtime memory
+ //
+ Status = gDS->RemoveMemorySpace (
+ BaseAddress,
+ Length
+ );
+
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeMemoryMappedIo,
+ BaseAddress,
+ Length,
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypeMemoryMappedIo,
+ 0,
+ Length,
+ &BaseAddress,
+ gImageHandle,
+ NULL
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->SetMemorySpaceAttributes (
+ BaseAddress,
+ Length,
+ GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ return Status;
+}
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c
index e0617f2503a2..af08fa69d489 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c
@@ -67,3 +67,16 @@ InstallVirtualAddressChangeHandler (
// Nothing.
//
}
+
+EFI_STATUS
+MarkIoMemoryRangeForRuntimeAccess (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINTN Length
+ )
+{
+ //
+ // Nothing
+ //
+
+ return EFI_SUCCESS;
+}
--
2.18.0
From f36bbd768da8b0b490a96ee660800f0c22f42472 Mon Sep 17 00:00:00 2001
From: Brijesh Singh <brijesh.singh@amd.com>
Date: Fri, 6 Jul 2018 10:00:42 -0500
Subject: [PATCH 4/4] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Restore C-bit
when SEV is active
AmdSevDxe maps the flash memory range with C=0, but
SetMemorySpaceAttributes() unconditionally resets the C-bit to '1'. Lets
restore the mapping back to C=0.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
---
.../FvbServicesRuntimeDxe.inf | 1 +
.../FwBlockServiceDxe.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
index c0dda75bf75f..ab4e01bbebde 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
@@ -51,6 +51,7 @@ [LibraryClasses]
DebugLib
DevicePathLib
DxeServicesTableLib
+ MemEncryptSevLib
MemoryAllocationLib
PcdLib
UefiBootServicesTableLib
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
index 37deece363e6..1fbe1342a57c 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -18,6 +18,7 @@
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/DxeServicesTableLib.h>
+#include <Library/MemEncryptSevLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeLib.h>
@@ -203,5 +204,21 @@ MarkIoMemoryRangeForRuntimeAccess (
);
ASSERT_EFI_ERROR (Status);
+ //
+ // When SEV is active, AmdSevDxe mapped the BaseAddress with C=0 but
+ // SetMemorySpaceAttributes() remaps the range with C=1. Let's restore
+ // the mapping so that both guest and hyervisor can access the flash
+ // memory range.
+ //
+ if (MemEncryptSevIsEnabled ()) {
+ Status = MemEncryptSevClearPageEncMask (
+ 0,
+ BaseAddress,
+ EFI_SIZE_TO_PAGES (Length),
+ FALSE
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+
return Status;
}
--
2.18.0
