[PATCH v3 1/1] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Clear C-bit when SEV is active

[PATCH v3 1/1] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Clear C-bit when SEV is active

[Brijesh Singh]

Commit:24e4ad7 (OvmfPkg: Add AmdSevDxe driver) added a driver which runs
early in DXE phase and clears the C-bit from NonExistent entry -- which
is later split and accommodate the flash MMIO. When SMM is enabled, we
build two sets of page tables; first page table is used when executing
code in non SMM mode (SMM-less-pgtable) and second page table is used
when we are executing code in SMM mode (SMM-pgtable).

During boot time, AmdSevDxe driver clears the C-bit from the
SMM-less-pgtable. But when SMM is enabled, Qemu Flash services are used
from SMM mode.

In this patch we explicitly clear the C-bit from Qemu flash MMIO range
before we probe the flash. When OVMF is built with SMM_REQUIRE then
call to initialize the flash services happen after the SMM-pgtable is
created and processor has served the first SMI. At this time we will
have access to the SMM-pgtable.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---

Changes since v2:
 - rename BeforeFlashProbe() -> QemuFlashBeforeProbe()
 - add new file to define Smm specific QemuFlashBeforeProbe()
 - update commit message and comment in the code

Patch is also available at
url: github.com/codomania/edk2.git
branch: smm-v3


 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf |  2 +
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h        |  7 +++
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c        |  8 +++
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c     | 12 +++++
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c     | 54 ++++++++++++++++++++
 5 files changed, 83 insertions(+)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
index ba2d3679a46d..b5c79762c23f 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
@@ -40,6 +40,7 @@ [Sources]
   FwBlockService.c
   FwBlockServiceSmm.c
   QemuFlash.c
+  QemuFlashSmm.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -53,6 +54,7 @@ [LibraryClasses]
   DevicePathLib
   DxeServicesTableLib
   MemoryAllocationLib
+  MemEncryptSevLib
   PcdLib
   SmmServicesTableLib
   UefiBootServicesTableLib
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
index 8d83dca7a52c..d34a8a19a039 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
@@ -88,5 +88,12 @@ QemuFlashConvertPointers (
   VOID
   );
 
+VOID
+QemuFlashBeforeProbe (
+  IN  EFI_PHYSICAL_ADDRESS    BaseAddress,
+  IN  UINTN                   FdBlockSize,
+  IN  UINTN                   FdBlockCount
+  );
+
 #endif
 
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
index 5677b5ee119c..3f057918298d 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
@@ -244,6 +244,14 @@ QemuFlashInitialize (
   ASSERT(PcdGet32 (PcdOvmfFirmwareFdSize) % mFdBlockSize == 0);
   mFdBlockCount = PcdGet32 (PcdOvmfFirmwareFdSize) / mFdBlockSize;
 
+  //
+  // execute module specific hooks before probing the flash
+  //
+  QemuFlashBeforeProbe (
+    (EFI_PHYSICAL_ADDRESS)(UINTN) mFlashBase,
+    mFdBlockSize, mFdBlockCount
+    );
+
   if (!QemuFlashDetected ()) {
     ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
     return EFI_WRITE_PROTECTED;
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
index 08ece2beaeca..332ceafd3f78 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
@@ -26,3 +26,15 @@ QemuFlashConvertPointers (
 {
   EfiConvertPointer (0x0, (VOID **) &mFlashBase);
 }
+
+VOID
+QemuFlashBeforeProbe (
+  IN  EFI_PHYSICAL_ADDRESS    BaseAddress,
+  IN  UINTN                   FdBlockSize,
+  IN  UINTN                   FdBlockCount
+  )
+{
+  //
+  // Do nothing
+  //
+}
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
new file mode 100644
index 000000000000..193fcec3690e
--- /dev/null
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
@@ -0,0 +1,54 @@
+/** @file
+  Define the module hooks used while probing the QEMU flash device.
+
+  Copyright (C) 2018, Advanced Micro Devices. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/MemEncryptSevLib.h>
+
+#include "QemuFlash.h"
+
+VOID
+QemuFlashBeforeProbe (
+  IN  EFI_PHYSICAL_ADDRESS    BaseAddress,
+  IN  UINTN                   FdBlockSize,
+  IN  UINTN                   FdBlockCount
+  )
+{
+  EFI_STATUS              Status;
+
+  ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
+
+  if (!MemEncryptSevIsEnabled()) {
+    return;
+  }
+
+  //
+  // When SEV is enabled, AmdSevDxe runs early in DXE phase and clears the C-bit
+  // from the NonExistent entry -- which is later split and accommodate the
+  // flash MMIO but the driver runs in non SMM context hence it cleared the
+  // flash ranges from non SMM page table. When SMM is enabled, the flash
+  // services are accessed from the SMM mode hence we explicitly clear the
+  // C-bit on flash ranges from SMM page table.
+  //
+
+  Status = MemEncryptSevClearPageEncMask (
+             0,
+             BaseAddress,
+             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount),
+             FALSE
+             );
+  ASSERT_EFI_ERROR (Status);
+}
-- 
2.14.3

Re: [PATCH v3 1/1] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Clear C-bit when SEV is active

[Laszlo Ersek]

On 03/09/18 17:06, Brijesh Singh wrote:
> Commit:24e4ad7 (OvmfPkg: Add AmdSevDxe driver) added a driver which runs
> early in DXE phase and clears the C-bit from NonExistent entry -- which
> is later split and accommodate the flash MMIO. When SMM is enabled, we
> build two sets of page tables; first page table is used when executing
> code in non SMM mode (SMM-less-pgtable) and second page table is used
> when we are executing code in SMM mode (SMM-pgtable).
>
> During boot time, AmdSevDxe driver clears the C-bit from the
> SMM-less-pgtable. But when SMM is enabled, Qemu Flash services are used
> from SMM mode.
>
> In this patch we explicitly clear the C-bit from Qemu flash MMIO range
> before we probe the flash. When OVMF is built with SMM_REQUIRE then
> call to initialize the flash services happen after the SMM-pgtable is
> created and processor has served the first SMI. At this time we will
> have access to the SMM-pgtable.
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>
> Changes since v2:
>  - rename BeforeFlashProbe() -> QemuFlashBeforeProbe()
>  - add new file to define Smm specific QemuFlashBeforeProbe()
>  - update commit message and comment in the code
>
> Patch is also available at
> url: github.com/codomania/edk2.git
> branch: smm-v3
>
>
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf |  2 +
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h        |  7 +++
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c        |  8 +++
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c     | 12 +++++
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c     | 54 ++++++++++++++++++++
>  5 files changed, 83 insertions(+)

[lersek@redhat.com: trivial coding style improvements]:

> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> index 3f057918298d..462d9c0322f4 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> @@ -249,7 +249,8 @@ QemuFlashInitialize (
>    //
>    QemuFlashBeforeProbe (
>      (EFI_PHYSICAL_ADDRESS)(UINTN) mFlashBase,
> -    mFdBlockSize, mFdBlockCount
> +    mFdBlockSize,
> +    mFdBlockCount
>      );
>
>    if (!QemuFlashDetected ()) {
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> index 193fcec3690e..8999ad8d0d2b 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> @@ -31,14 +31,14 @@ QemuFlashBeforeProbe (
>
>    ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
>
> -  if (!MemEncryptSevIsEnabled()) {
> +  if (!MemEncryptSevIsEnabled ()) {
>      return;
>    }
>
>    //
> -  // When SEV is enabled, AmdSevDxe runs early in DXE phase and clears the C-bit
> -  // from the NonExistent entry -- which is later split and accommodate the
> -  // flash MMIO but the driver runs in non SMM context hence it cleared the
> +  // When SEV is enabled, AmdSevDxe runs early in DXE phase and clears the
> +  // C-bit from the NonExistent entry -- which is later split and accommodate
> +  // the flash MMIO but the driver runs in non SMM context hence it cleared the
>    // flash ranges from non SMM page table. When SMM is enabled, the flash
>    // services are accessed from the SMM mode hence we explicitly clear the
>    // C-bit on flash ranges from SMM page table.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Commit e4a1d5a7c4e4.

Thanks Brijesh!
Laszlo