public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, thomas.lendacky@amd.com
Cc: Jordan Justen <jordan.l.justen@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Liming Gao <liming.gao@intel.com>,
	Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [edk2-devel] [PATCH v5 36/42] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Bypass flash detection with SEV-ES is enabled
Date: Tue, 3 Mar 2020 13:33:16 +0100	[thread overview]
Message-ID: <a261853e-c6c7-c17d-2a01-b37190ff7f99@redhat.com> (raw)
In-Reply-To: <05bb0fa97078cbaeac5d6388c25a7b203fb62286.1583190432.git.thomas.lendacky@amd.com>

Hi Tom,

On 03/03/20 00:07, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> The flash detection routine will attempt to determine how the flash
> device behaves (e.g. ROM, RAM, Flash). But when SEV-ES is enabled and
> the flash device behaves as a ROM device (meaning it is marked read-only
> by the hypervisor), this check may result in an infinite nested page fault
> because of the attempted write. Since the instruction cannot be emulated
> when SEV-ES is enabled, the RIP is never advanced, resulting in repeated
> nested page faults.
> 
> When SEV-ES is enabled, exit the flash detection early and assume that
> the FD behaves as Flash. This will result in QemuFlashWrite() being called
> to store EFI variables, which will also result in an infinite nested page
> fault when the write is performed. In this case, update QemuFlashWrite()
> to use the VmgMmioWrite function from the VmgExitLib library to have the
> hypervisor perform the write without having to emulate the instruction.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc                       |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  1 +

I asked for these lib class resolutions to be dropped, under v4.

http://mid.mail-archive.com/53e0bc61-5105-1597-7add-86e038015e15@redhat.com

Laszlo

>  .../FvbServicesRuntimeDxe.inf                 |  2 ++
>  .../QemuFlash.h                               |  6 +++++
>  .../QemuFlash.c                               | 23 ++++++++++++++++---
>  .../QemuFlashDxe.c                            | 15 ++++++++++++
>  .../QemuFlashSmm.c                            |  9 ++++++++
>  8 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 51d7acafdda3..2531b7edccf5 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -318,6 +318,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 764fa8c287a0..629725ef2b44 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -323,6 +323,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index b6e29e09db97..74076cbe7692 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -323,6 +323,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> index 8125fd0735a1..3ce19d1bfa8e 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> @@ -38,6 +38,7 @@ [Sources]
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
>    OvmfPkg/OvmfPkg.dec
>  
>  [LibraryClasses]
> @@ -52,6 +53,7 @@ [LibraryClasses]
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
>    UefiRuntimeLib
> +  VmgExitLib
>  
>  [Guids]
>    gEfiEventVirtualAddressChangeGuid   # ALWAYS_CONSUMED
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
> index f1afabcbe6ae..19ac1f733279 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
> @@ -89,5 +89,11 @@ QemuFlashBeforeProbe (
>    IN  UINTN                   FdBlockCount
>    );
>  
> +VOID
> +QemuFlashPtrWrite (
> +  IN        volatile UINT8    *Ptr,
> +  IN        UINT8             Value
> +  );
> +
>  #endif
>  
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> index c81c58972bf2..ccf5ad7f7afb 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
> @@ -9,6 +9,7 @@
>  
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/MemEncryptSevLib.h>
>  #include <Library/PcdLib.h>
>  
>  #include "QemuFlash.h"
> @@ -80,6 +81,21 @@ QemuFlashDetected (
>  
>    DEBUG ((EFI_D_INFO, "QEMU Flash: Attempting flash detection at %p\n", Ptr));
>  
> +  if (MemEncryptSevEsIsEnabled()) {
> +    //
> +    // When SEV-ES is enabled, the check below can result in an infinite
> +    // loop with respect to a nested page fault. When the memslot is mapped
> +    // read-only, the nested page table entry is read-only. The check below
> +    // will cause a nested page fault that cannot be emulated, causing
> +    // the instruction to retried over and over. For SEV-ES, acknowledge that
> +    // the FD appears as ROM and not as FLASH, but report FLASH anyway because
> +    // FLASH behavior can be simulated using VMGEXIT.
> +    //
> +    DEBUG ((DEBUG_INFO,
> +      "QEMU Flash: SEV-ES enabled, assuming FD behaves as FLASH\n"));
> +    return TRUE;
> +  }
> +
>    OriginalUint8 = *Ptr;
>    *Ptr = CLEAR_STATUS_CMD;
>    ProbeUint8 = *Ptr;
> @@ -181,8 +197,9 @@ QemuFlashWrite (
>    //
>    Ptr = QemuFlashPtr (Lba, Offset);
>    for (Loop = 0; Loop < *NumBytes; Loop++) {
> -    *Ptr = WRITE_BYTE_CMD;
> -    *Ptr = Buffer[Loop];
> +    QemuFlashPtrWrite (Ptr, WRITE_BYTE_CMD);
> +    QemuFlashPtrWrite (Ptr, Buffer[Loop]);
> +
>      Ptr++;
>    }
>  
> @@ -190,7 +207,7 @@ QemuFlashWrite (
>    // Restore flash to read mode
>    //
>    if (*NumBytes > 0) {
> -    *(Ptr - 1) = READ_ARRAY_CMD;
> +    QemuFlashPtrWrite (Ptr - 1, READ_ARRAY_CMD);
>    }
>  
>    return EFI_SUCCESS;
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
> index 5aabe9d7b59c..939463a8e17c 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
> @@ -10,6 +10,8 @@
>  **/
>  
>  #include <Library/UefiRuntimeLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/VmgExitLib.h>
>  
>  #include "QemuFlash.h"
>  
> @@ -32,3 +34,16 @@ QemuFlashBeforeProbe (
>    // Do nothing
>    //
>  }
> +
> +VOID
> +QemuFlashPtrWrite (
> +  IN        volatile UINT8    *Ptr,
> +  IN        UINT8             Value
> +  )
> +{
> +  if (MemEncryptSevEsIsEnabled()) {
> +    VmgMmioWrite ((UINT8 *) Ptr, &Value, 1);
> +  } else {
> +    *Ptr = Value;
> +  }
> +}
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> index 7eb426e03855..eff40ae28032 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> @@ -46,3 +46,12 @@ QemuFlashBeforeProbe (
>               );
>    ASSERT_EFI_ERROR (Status);
>  }
> +
> +VOID
> +QemuFlashPtrWrite (
> +  IN        volatile UINT8    *Ptr,
> +  IN        UINT8             Value
> +  )
> +{
> +  *Ptr = Value;
> +}
> 


  reply	other threads:[~2020-03-03 12:33 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 23:06 [PATCH v5 00/42] SEV-ES guest support Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 01/42] MdePkg: Create PCDs to be used in support of SEV-ES Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 02/42] MdePkg: Add the MSR definition for the GHCB register Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 03/42] MdePkg: Add a structure definition for the GHCB Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 04/42] MdeModulePkg/DxeIplPeim: Support GHCB pages when creating page tables Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 05/42] MdePkg/BaseLib: Add support for the XGETBV instruction Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 06/42] MdePkg/BaseLib: Add support for the VMGEXIT instruction Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 07/42] UefiCpuPkg: Implement library support for VMGEXIT Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 08/42] OvmfPkg: Prepare OvmfPkg to use the VmgExitLib library Lendacky, Thomas
2020-03-03 12:10   ` [edk2-devel] " Laszlo Ersek
2020-03-02 23:06 ` [PATCH v5 09/42] UefiPayloadPkg: Prepare UefiPayloadPkg " Lendacky, Thomas
2020-03-03 23:52   ` [edk2-devel] " Guo Dong
2020-03-02 23:06 ` [PATCH v5 10/42] UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 11/42] UefiCpuPkg/CpuExceptionHandler: Add support for IOIO_PROT NAE events Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 12/42] UefiCpuPkg/CpuExceptionHandler: Support string IO " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 13/42] UefiCpuPkg/CpuExceptionHandler: Add support for CPUID " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 14/42] UefiCpuPkg/CpuExceptionHandler: Add support for MSR_PROT " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 15/42] UefiCpuPkg/CpuExceptionHandler: Add support for NPF NAE events (MMIO) Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 16/42] UefiCpuPkg/CpuExceptionHandler: Add support for WBINVD NAE events Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 17/42] UefiCpuPkg/CpuExceptionHandler: Add support for RDTSC " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 18/42] UefiCpuPkg/CpuExceptionHandler: Add support for RDPMC " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 19/42] UefiCpuPkg/CpuExceptionHandler: Add support for INVD " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 20/42] UefiCpuPkg/CpuExceptionHandler: Add support for VMMCALL " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 21/42] UefiCpuPkg/CpuExceptionHandler: Add support for RDTSCP " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 22/42] UefiCpuPkg/CpuExceptionHandler: Add support for MONITOR/MONITORX " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 23/42] UefiCpuPkg/CpuExceptionHandler: Add support for MWAIT/MWAITX " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 24/42] UefiCpuPkg/CpuExceptionHandler: Add support for DR7 Read/Write " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 25/42] OvmfPkg/MemEncryptSevLib: Add an SEV-ES guest indicator function Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 26/42] OvmfPkg: Add support to perform SEV-ES initialization Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 27/42] OvmfPkg: Create a GHCB page for use during Sec phase Lendacky, Thomas
2020-03-03  5:29   ` Lendacky, Thomas
2020-03-03  5:36   ` [PATCH v5.1 27/43] " Lendacky, Thomas
2020-03-02 23:06 ` [PATCH v5 28/42] OvmfPkg/PlatformPei: Reserve GHCB-related areas if S3 is supported Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 29/42] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 30/42] OvmfPkg/PlatformPei: Move early GDT into ram when SEV-ES is enabled Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 31/42] UefiCpuPkg: Create an SEV-ES workarea PCD Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 32/42] OvmfPkg: Reserve a page in memory for the SEV-ES usage Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 33/42] OvmfPkg/ResetVector: Add support for a 32-bit SEV check Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 34/42] OvmfPkg/Sec: Add #VC exception handling for Sec phase Lendacky, Thomas
2020-03-03 13:29   ` [edk2-devel] " Laszlo Ersek
2020-03-03 14:20     ` Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 35/42] OvmfPkg/Sec: Enable cache early to speed up booting Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 36/42] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Bypass flash detection with SEV-ES is enabled Lendacky, Thomas
2020-03-03 12:33   ` Laszlo Ersek [this message]
2020-03-03 14:26     ` [edk2-devel] " Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 37/42] UefiCpuPkg: Add a 16-bit protected mode code segment descriptor Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 38/42] UefiCpuPkg/MpInitLib: Add CPU MP data flag to indicate if SEV-ES is enabled Lendacky, Thomas
2020-03-02 23:07 ` [PATCH v5 39/42] UefiCpuPkg: Allow AP booting under SEV-ES Lendacky, Thomas
2020-03-03  5:43 ` [PATCH v5 40/42] OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector Lendacky, Thomas
2020-03-03  5:43 ` [PATCH v5 41/42] OvmfPkg: Move the GHCB allocations into reserved memory Lendacky, Thomas
2020-03-03  5:43 ` [PATCH v5 42/42] UefiCpuPkg/MpInitLib: Prepare SEV-ES guest APs for OS use Lendacky, Thomas

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=a261853e-c6c7-c17d-2a01-b37190ff7f99@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