[PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO
@ 2018-07-05 14:05 Brijesh Singh
  2018-07-05 14:05 ` [PATCH v2 2/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose MMIO in SMM build Brijesh Singh
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Brijesh Singh @ 2018-07-05 14:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Tom Lendacky, Brijesh Singh, Justen Jordan L, Laszlo Ersek

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: 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>
---
 .../FwBlockService.c                               | 28 ++++++++++++++++------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index 558b395dff4a..28499991a43c 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 (
+  Status = gDS->AllocateMemorySpace (
                   AllocateAddress,
-                  EfiRuntimeServicesData,
-                  EFI_SIZE_TO_PAGES (Length),
-                  &BaseAddress
+                  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.7.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose MMIO in SMM build
  2018-07-05 14:05 [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Brijesh Singh
@ 2018-07-05 14:05 ` Brijesh Singh
  2018-07-05 14:05 ` [PATCH v2 3/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Restore C-bit when SEV is active Brijesh Singh
  2018-07-05 14:41 ` [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Laszlo Ersek
  2 siblings, 0 replies; 5+ messages in thread
From: Brijesh Singh @ 2018-07-05 14:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Tom Lendacky, Brijesh Singh, Justen Jordan L, Laszlo Ersek

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: 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>
---
 .../FwBlockService.c                               | 50 ---------------------
 .../FwBlockService.h                               |  7 +++
 .../FwBlockServiceDxe.c                            | 51 ++++++++++++++++++++++
 .../FwBlockServiceSmm.c                            | 13 ++++++
 4 files changed, 71 insertions(+), 50 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index 28499991a43c..eec8b1b1ae9d 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -831,56 +831,6 @@ ValidateFvHeader (
 
 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 (
-                  AllocateAddress,
-                  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 (
   VOID
   )
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..646427bf4e2c 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -22,6 +22,8 @@
 #include <Library/UefiRuntimeLib.h>
 #include <Protocol/DevicePath.h>
 #include <Protocol/FirmwareVolumeBlock.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
 
 #include "FwBlockService.h"
 #include "QemuFlash.h"
@@ -155,3 +157,52 @@ InstallVirtualAddressChangeHandler (
                   );
   ASSERT_EFI_ERROR (Status);
 }
+
+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 (
+                  AllocateAddress,
+                  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..cdb073348158 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceSmm.c
@@ -67,3 +67,16 @@ InstallVirtualAddressChangeHandler (
   // Nothing.
   //
 }
+
+EFI_STATUS
+MarkIoMemoryRangeForRuntimeAccess (
+  EFI_PHYSICAL_ADDRESS                BaseAddress,
+  UINTN                               Length
+  )
+{
+  //
+  // Nothing
+  //
+
+  return EFI_SUCCESS;
+}
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Restore C-bit when SEV is active
  2018-07-05 14:05 [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Brijesh Singh
  2018-07-05 14:05 ` [PATCH v2 2/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose MMIO in SMM build Brijesh Singh
@ 2018-07-05 14:05 ` Brijesh Singh
  2018-07-05 14:41 ` [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Laszlo Ersek
  2 siblings, 0 replies; 5+ messages in thread
From: Brijesh Singh @ 2018-07-05 14:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Tom Lendacky, Brijesh Singh, Justen Jordan L, Laszlo Ersek

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: 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>
---
 .../FvbServicesRuntimeDxe.inf                           |  1 +
 .../QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c  | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
index d7b4ec06c4e6..6bb5c2093790 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
@@ -54,6 +54,7 @@ [LibraryClasses]
   DevicePathLib
   DxeServicesTableLib
   MemoryAllocationLib
+  MemEncryptSevLib
   PcdLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
index 646427bf4e2c..3add4bbad74c 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -24,6 +24,7 @@
 #include <Protocol/FirmwareVolumeBlock.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/MemEncryptSevLib.h>
 
 #include "FwBlockService.h"
 #include "QemuFlash.h"
@@ -204,5 +205,21 @@ MarkIoMemoryRangeForRuntimeAccess (
                   );
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // When SEV is active, AmdSevDxe should have mapped the BaseAddress with
+  // C=0 but SetMemorySpaceAttribute() remap the range with C=1. Lets 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.7.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO
  2018-07-05 14:05 [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Brijesh Singh
  2018-07-05 14:05 ` [PATCH v2 2/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose MMIO in SMM build Brijesh Singh
  2018-07-05 14:05 ` [PATCH v2 3/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Restore C-bit when SEV is active Brijesh Singh
@ 2018-07-05 14:41 ` Laszlo Ersek
  2018-07-05 16:48   ` Brijesh Singh
  2 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2018-07-05 14:41 UTC (permalink / raw)
  To: Brijesh Singh, edk2-devel; +Cc: Tom Lendacky, Justen Jordan L

Hi Brijesh,

On 07/05/18 16:05, Brijesh Singh wrote:
> [...]

I'll need some time before I can look at this -- meanwhile, can you
please repost the series with a dedicated cover letter email? If you
wish you can include a short summary description and v2/v3 etc updates
there; however, the main point is that the patches be nicely collected
in a threaded list view. Patches #2 and #3 shouldn't be children of
Patch #1. And, people should be able to comment on the series as a whole
-- for that, cover letters which they can respond to are the best.

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05

  git config format.coverletter true

or

  git config format.coverletter auto


When you do this, please collect all the CC's across the patch emails,
sort them uniquely, and add them manually to the cover letter too.

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23

(Note that the references above are not official edk2 material; I'm just
linking them for better explaining my request.)

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO
  2018-07-05 14:41 ` [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Laszlo Ersek
@ 2018-07-05 16:48   ` Brijesh Singh
  0 siblings, 0 replies; 5+ messages in thread
From: Brijesh Singh @ 2018-07-05 16:48 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel; +Cc: brijesh.singh, Tom Lendacky, Justen Jordan L



On 7/5/18 9:41 AM, Laszlo Ersek wrote:
> Hi Brijesh,
>
> On 07/05/18 16:05, Brijesh Singh wrote:
>> [...]
> I'll need some time before I can look at this -- meanwhile, can you
> please repost the series with a dedicated cover letter email? If you
> wish you can include a short summary description and v2/v3 etc updates
> there; however, the main point is that the patches be nicely collected
> in a threaded list view. Patches #2 and #3 shouldn't be children of
> Patch #1. And, people should be able to comment on the series as a whole
> -- for that, cover letters which they can respond to are the best.
>
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05
>
>   git config format.coverletter true
>
> or
>
>   git config format.coverletter auto
>
>
> When you do this, please collect all the CC's across the patch emails,
> sort them uniquely, and add them manually to the cover letter too.
>
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
>
> (Note that the references above are not official edk2 material; I'm just
> linking them for better explaining my request.)

Sure, I will resend series with cover-letter. thanks




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-07-05 16:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-05 14:05 [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Brijesh Singh
2018-07-05 14:05 ` [PATCH v2 2/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not expose MMIO in SMM build Brijesh Singh
2018-07-05 14:05 ` [PATCH v2 3/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Restore C-bit when SEV is active Brijesh Singh
2018-07-05 14:41 ` [PATCH v2 1/3] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: mark Flash memory range as MMIO Laszlo Ersek
2018-07-05 16:48   ` Brijesh Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox