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.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Tom Lendacky <thomas.lenda...@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. 53e0bc61-5105-1597-7add-86e038015e15@redhat.com">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; > +} > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55319): https://edk2.groups.io/g/devel/message/55319 Mute This Topic: https://groups.io/mt/71687839/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
