On 3/3/20 6:33 AM, Laszlo Ersek wrote: > Hi Tom, > > On 03/03/20 00:07, Lendacky, Thomas wrote: >> BZ: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec51d6f2d119454bdbc508d7bf6f127a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637188356100831031&sdata=h7GUpHnIH8kilQY9wvXwvIujG6YU5PgYl5wYK2rP2Eg%3D&reserved=0 >> >> 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.
Hmmm... I thought I had when I consolidated the library references, but obviously I didn't. I'll fix that up. Thanks, Tom > > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2F53e0bc61-5105-1597-7add-86e038015e15%40redhat.com&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec51d6f2d119454bdbc508d7bf6f127a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637188356100841024&sdata=QrbXlgrKDVhspRRKaXPUSaD5KJ5u7vZ9q6yQ70SKSWM%3D&reserved=0 > > 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 (#55338): https://edk2.groups.io/g/devel/message/55338 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] -=-=-=-=-=-=-=-=-=-=-=-
