* [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages
@ 2024-07-14 12:22 wojiaohanliyang
2024-07-14 12:22 ` [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it wojiaohanliyang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: wojiaohanliyang @ 2024-07-14 12:22 UTC (permalink / raw)
To: devel
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807
This patch series provides fixes for boot VM when use secure boot
supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages.
- Detect FlashNvVarStore before calls PlaformValidateNvVarStore in PlatformInitLib
and skip the validation process if the FlashNvVarStore doesn't exist. Since the
PlatformValidateNvVarStore will return false if the FlashNvVarStore doesn't
exist, this will trigger ASSERT (FALSE) and prevent the guest from moving
forward.
- Init the whole range of EmuVariableNvStore before copy content from the
FlashNvVarStore to EmuVariableNvStore. If the Ftw (Fault Tolerant Write) part
of the EmuVariableNvStore isn't initialized, the FaultToleranteWriteDxe will
use scrambled address to access memory and leads to crash if the VM is a
SEV guest.
- Fix the mapping for FlashNvVarStore. If launch a SEV VM with only OVMF.fd,
the address range for FlashNvVarStore should be mapped as encrypted before
access FlashNvVarStore in PlatformValidateNvVarStore. If launch a SEV VM with
both OVMF_CODE.fd and OVMF_VARS.fd, the address range for FlashNvVarStore
should be mapped as decrypted before access FlashNvVarStore in
PlatformValidateNvVarStore.
---
These patches based on commit:
d4dbe5e101dc ("SecurityPkg/Tcg2Acpi: Revise debug print")
Han Liyang (3):
OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it
OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data
OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it
OvmfPkg/Library/PlatformInitLib/Platform.c | 67 +++++++++++
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 105 ++++++++++++++++++
OvmfPkg/PlatformPei/Platform.c | 6 +
OvmfPkg/PlatformPei/Platform.h | 6 +
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
6 files changed, 186 insertions(+)
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119924): https://edk2.groups.io/g/devel/message/119924
Mute This Topic: https://groups.io/mt/107212891/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread* [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it 2024-07-14 12:22 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang @ 2024-07-14 12:22 ` wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it wojiaohanliyang 2 siblings, 0 replies; 5+ messages in thread From: wojiaohanliyang @ 2024-07-14 12:22 UTC (permalink / raw) To: devel; +Cc: hanliyang From: hanliyang <wojiaohanliyang@163.com> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807 The commit 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore") rename the function from TdxValidateCfv to PlatformValidateNvVarStore. PlatformValidateNvVarStore is placed in the PlatformInitLib and is used in the case that OVMF is launched with -bios parameter and to validate the integrity of FlashNvVarStore. But if we launch a VM without FlashNvVarStore, the PlatformValidateNvVarStore will fail to validate the integrity and will trigger ASSERT (FALSE) in PlatformInitEmuVariableNvStore. In order to prevent calls to PlatformValidateNvVarStore in the case lack of FlashNvVarStore, we should detect FlashNvVarStore before calls to PlatformValidateNvVarStore. If fail to detect FlashNvVarStore, we should return don't initialize the EmuVariableNvStore, otherwise calls to PlatformValidateNvVarStore and initialize the EmuVariableNvStore when succeed to validate the integrity of NvVarStore. Fixes: 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore") Signed-off-by: hanliyang <wojiaohanliyang@163.com> --- OvmfPkg/Library/PlatformInitLib/Platform.c | 47 +++++++++++++++++++ .../PlatformInitLib/PlatformInitLib.inf | 1 + 2 files changed, 48 insertions(+) diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c index f48bf16ae3..0a720a4c2c 100644 --- a/OvmfPkg/Library/PlatformInitLib/Platform.c +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c @@ -895,6 +895,16 @@ PlatformReserveEmuVariableNvStore ( return VariableStore; } +#define WRITE_BYTE_CMD 0x10 +#define BLOCK_ERASE_CMD 0x20 +#define CLEAR_STATUS_CMD 0x50 +#define READ_STATUS_CMD 0x70 +#define READ_DEVID_CMD 0x90 +#define BLOCK_ERASE_CONFIRM_CMD 0xd0 +#define READ_ARRAY_CMD 0xff + +#define CLEARED_ARRAY_STATUS 0x00 + /** When OVMF is lauched with -bios parameter, UEFI variables will be partially emulated, and non-volatile variables may lose their contents @@ -928,6 +938,43 @@ PlatformInitEmuVariableNvStore ( Size = (UINT32)PcdGet32 (PcdFlashNvStorageVariableSize); ASSERT (Size < EmuVariableNvStoreSize); + // + // If launch a VM without OvmfFlashNvStorage device, then we'll fail + // to check the integrity of NvVarStore and trigger ASSERT (FALSE). + // So, we should detect the OvmfFlashNvStorage before calls to + // PlatformValidateNvVarStore(). If fail to detect OvmfFlashNvStorage, + // we should return and don't initialize the EmuVariableNvStore, + // otherwise calls to PlatformValidateNvVarStore() and initialize the + // EmuVariableNvStore when succeed to check the integrity of + // NvVarStore. + // + // This method to detect the OvmfFlashNvStorage here references + // OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c. + // + volatile UINT8 *Ptr; + + UINTN BlockSize; + UINTN Offset; + UINT8 ProbeUint8; + + BlockSize = PcdGet32 (PcdOvmfFirmwareBlockSize); + + for (Offset = 0; Offset < BlockSize; Offset++) { + Ptr = Base + Offset; + ProbeUint8 = *Ptr; + if ((ProbeUint8 != CLEAR_STATUS_CMD) && + (ProbeUint8 != READ_STATUS_CMD) && + (ProbeUint8 != CLEARED_ARRAY_STATUS)) + { + break; + } + } + + if (Offset >= BlockSize) { + DEBUG ((DEBUG_INFO, "OvmfFlashNvStorage: Failed to find probe location\n")); + return EFI_INVALID_PARAMETER; + } + if (!PlatformValidateNvVarStore (Base, PcdGet32 (PcdCfvRawDataSize))) { ASSERT (FALSE); return EFI_INVALID_PARAMETER; diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf index 21e6efa5e0..b7d5e63dcd 100644 --- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf +++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf @@ -104,6 +104,7 @@ gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize [FeaturePcd] gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119922): https://edk2.groups.io/g/devel/message/119922 Mute This Topic: https://groups.io/mt/107212920/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data 2024-07-14 12:22 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it wojiaohanliyang @ 2024-07-14 12:22 ` wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it wojiaohanliyang 2 siblings, 0 replies; 5+ messages in thread From: wojiaohanliyang @ 2024-07-14 12:22 UTC (permalink / raw) To: devel; +Cc: hanliyang From: hanliyang <wojiaohanliyang@163.com> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807 In the case launch with just OVMF.fd, if we just init part of the EmuVariableNvStore, then EmuVariableFvbRuntimeDxe will skip the initialize process of the EmuVariableNvStore and the Ftw (Fault Tolerant Write) part of the EmuVariableNvStore will not be initialized before the Ftw part is accessed. When we launch a SEV guest, the FaultTolerantWriteDxe will get scrambled data when read Ftw part of the EmuVariableNvStore, the FaultToleranteWriteDxe access address specified by the scrambled data will cause invalid address access and crash. The crash message is shown as below. Loading driver at 0x000BDB92000 EntryPoint=0x000BDB95EF4 FaultTolerantWriteDxe.efi InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF BDE01D98 ProtectUefiImageCommon - 0xBDE01040 - 0x00000000BDB92000 - 0x0000000000005B00 Ftw: FtwWorkSpaceLba - 0x40, WorkBlockSize - 0x1000, FtwWorkSpaceBase - 0x0 Ftw: FtwSpareLba - 0x42, SpareBlockSize - 0x1000 Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x40 Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0x0 Ftw: Remaining work space size - FE0 !!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000 !!!! ExceptionData - 0000000000000000 RIP - 00000000BDB92459, CS - 0000000000000038, RFLAGS - 0000000000010286 RAX - 587E3201A019FB0C, RCX - 587E3200E238F994, RDX - 0000000000000001 RBX - 00000000BDE10018, RSP - 00000000BFB79AD8, RBP - 0000000000000FE0 RSI - 00000000BDE100A8, RDI - 00000000BDE10128 R8 - D4642A9DFB7C79BE, R9 - 00000000000003F8, R10 - 00000000BDB96602 R11 - 0000000000000002, R12 - 00000000BDE100A0, R13 - 0000000000000000 R14 - 0000000000000001, R15 - 00000000BFBA76C0 DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 GS - 0000000000000030, SS - 0000000000000030 CR0 - 0000000080010033, CR2 - 0000000000000000, CR3 - 00000000BF801000 CR4 - 0000000000000668, CR8 - 0000000000000000 DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 00000000BF5DC000 0000000000000047, LDTR - 0000000000000000 IDTR - 00000000BEF0C018 0000000000000FFF, TR - 0000000000000000 FXSAVE_STATE - 00000000BFB79730 !!!! Find image based on IP(0xBDB92459) /dev/shm/edk2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe/DEBUG/FaultTolerantWriteDxe.dll (ImageBase=00000000BDB92000, EntryPoint=00000000BDB95EF4) !!!! Fixes: 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore") Signed-off-by: hanliyang <wojiaohanliyang@163.com> --- OvmfPkg/Library/PlatformInitLib/Platform.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c index 0a720a4c2c..5dbc5506f4 100644 --- a/OvmfPkg/Library/PlatformInitLib/Platform.c +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c @@ -905,6 +905,8 @@ PlatformReserveEmuVariableNvStore ( #define CLEARED_ARRAY_STATUS 0x00 +#define ERASED_UINT8 0xff + /** When OVMF is lauched with -bios parameter, UEFI variables will be partially emulated, and non-volatile variables may lose their contents @@ -982,6 +984,24 @@ PlatformInitEmuVariableNvStore ( DEBUG ((DEBUG_INFO, "Init EmuVariableNvStore with the content in FlashNvStorage\n")); + // + // Init the whole EmuVariableNvStore before copy the content from + // FlashNvStorage to the EmuVariableNvStore. + // + // In the case launch with just OVMF.fd, if we just init part of the + // EmuVariableNvStore, then EmuVariableFvbRuntimeDxe will skip the + // initialize process of the EmuVariableNvStore and the Ftw (Fault + // Tolerant Write) part of the EmuVariableNvStore will not be + // initialized before the Ftw part is accessed. When we launch a SEV + // guest, the FaultTolerantWriteDxe will get scrambled data when read + // Ftw part of the EmuVariableNvStore, the FaultToleranteWriteDxe + // access address specified by the scrambled data will cause invalid + // address access and crash. + // + // The method to init EmuVariableNvStore here references + // OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c. + // + SetMem (EmuVariableNvStore, EmuVariableNvStoreSize, ERASED_UINT8); CopyMem (EmuVariableNvStore, Base, Size); return EFI_SUCCESS; -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119923): https://edk2.groups.io/g/devel/message/119923 Mute This Topic: https://groups.io/mt/107212921/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it 2024-07-14 12:22 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data wojiaohanliyang @ 2024-07-14 12:22 ` wojiaohanliyang 2 siblings, 0 replies; 5+ messages in thread From: wojiaohanliyang @ 2024-07-14 12:22 UTC (permalink / raw) To: devel; +Cc: hanliyang From: hanliyang <wojiaohanliyang@163.com> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807 In the case of launch a SEV-ES VM with just OVMF_CODE.fd, the validation process in PlatformValidateNvVarStore will trigger MMIO NPF, and the #VC handler will detect that mmio access is invalid because the mmio address range of FlashNvVarStore is mapped as encrypted. In the case of launch a SEV VM with both OVMF_CODE.fd and OVMF_VARS.fd, PlatformValidateNvVarStore will fail to validate FlashNvVarStore because the mapping of FlashNvVarStore address range is encrypted in the guest but the corresponding data in system physical memory was not encrypted by guest key. We should map FlashNvVarStore address range as unencrypted for the above cases. Fixes: 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore") Signed-off-by: hanliyang <wojiaohanliyang@163.com> --- OvmfPkg/PlatformPei/AmdSev.c | 105 ++++++++++++++++++++++++++++ OvmfPkg/PlatformPei/Platform.c | 6 ++ OvmfPkg/PlatformPei/Platform.h | 6 ++ OvmfPkg/PlatformPei/PlatformPei.inf | 1 + 4 files changed, 118 insertions(+) diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c index 8562787035..89f4c02b6a 100644 --- a/OvmfPkg/PlatformPei/AmdSev.c +++ b/OvmfPkg/PlatformPei/AmdSev.c @@ -575,3 +575,108 @@ SevInitializeRam ( ); } } + +/** + Prepared for FlashNvVarStore access. + + **/ +VOID +SevFlashNvVarStoreUpdateMapping ( + IN UINTN NvVarStoreBase, + IN UINTN NvVarStoreSize + ) +{ + volatile UINT8 *Ptr; + + RETURN_STATUS DecEncStatus; + UINT8 Offset; + + DEBUG ((DEBUG_INFO, "%a\n", __func__)); + + if (!MemEncryptSevIsEnabled ()) { + return; + } + + // + // In the case of launch a SEV-ES VM with just OVMF_CODE.fd, the + // validation process in PlatformValidateNvVarStore will trigger MMIO + // NPF, and the #VC handler will detect that mmio access is invalid + // because the mmio address range of FlashNvVarStore is mapped as + // encrypted. So, first we should update the mapping of address range + // of FlashNvVarStore as decrypted. + // + DEBUG ( + ( + DEBUG_INFO, + "%a: mapping FlashNvVarStore address range unencrypted [0x%p - 0x%p]\n", + __func__, + (UINT8 *)NvVarStoreBase, + (UINT8 *)NvVarStoreBase + NvVarStoreSize - 1 + ) + ); + + DecEncStatus = MemEncryptSevClearMmioPageEncMask ( + 0, + NvVarStoreBase, + EFI_SIZE_TO_PAGES (NvVarStoreSize) + ); + if (RETURN_ERROR (DecEncStatus)) { + DEBUG ( + ( + DEBUG_ERROR, + "%a: failed to map FlashNvStorage address range unencrypted\n", + __func__ + ) + ); + ASSERT_RETURN_ERROR (DecEncStatus); + } + + // + // Here, the first 16 bytes of FlashNvVarStore will be all zeros in + // the following cases: + // a. Launch VM with just OVMF_CODE.fd + // b. Launch VM with OVMF_CODE.fd and OVMF_VARS.fd + // In these cases, the access of FlashNvVarStore will be as expected. + // + // But if launch VM with just OVMF.fd, the first 16 bytes of + // FlashNvVarStore will be scrambled data because the data of + // FlashNvVarStore are encrypted by SEV API. In this case, we need + // mapping FlashNvVarStore address range as encrypted again, otherwise + // the validation of FlashNvVarStore will fail and trigger + // ASSERT (FALSE). + // + for (Offset = 0; Offset < 16; Offset++) { + Ptr = (UINT8 *)NvVarStoreBase + Offset; + if (*Ptr) { + break; + } + } + + if (Offset == 16) { + return; + } + + DEBUG ( + ( + DEBUG_INFO, + "%a: mapping FlashNvStorage address range encrypted\n", + __func__ + ) + ); + + DecEncStatus = MemEncryptSevSetPageEncMask ( + 0, + NvVarStoreBase, + EFI_SIZE_TO_PAGES (NvVarStoreSize) + ); + if (RETURN_ERROR (DecEncStatus)) { + DEBUG ( + ( + DEBUG_ERROR, + "%a: failed to map FlashNvStorage address range encrypted\n", + __func__ + ) + ); + ASSERT_RETURN_ERROR (DecEncStatus); + } +} diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 0114529778..bcb18dacac 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -224,6 +224,12 @@ ReserveEmuVariableNvStore ( PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); if (FeaturePcdGet (PcdSecureBootSupported)) { + // update mapping of FlashNvVarStore address range + SevFlashNvVarStoreUpdateMapping ( + PcdGet32 (PcdOvmfFlashNvStorageVariableBase), + 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize) + ); + // restore emulated VarStore from pristine ROM copy PlatformInitEmuVariableNvStore ((VOID *)(UINTN)VariableStore); } diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h index 0a59547cfc..b8ad8df1dc 100644 --- a/OvmfPkg/PlatformPei/Platform.h +++ b/OvmfPkg/PlatformPei/Platform.h @@ -111,4 +111,10 @@ SevInitializeRam ( VOID ); +VOID +SevFlashNvVarStoreUpdateMapping ( + IN UINTN NvVarStoreBase, + IN UINTN NvVarStoreSize + ); + #endif // _PLATFORM_PEI_H_INCLUDED_ diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index 0bb1a46291..a3dd3db72d 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -139,6 +139,7 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase [FeaturePcd] gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119925): https://edk2.groups.io/g/devel/message/119925 Mute This Topic: https://groups.io/mt/107212922/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages
@ 2024-07-14 12:24 wojiaohanliyang
2024-07-14 12:24 ` [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data wojiaohanliyang
0 siblings, 1 reply; 5+ messages in thread
From: wojiaohanliyang @ 2024-07-14 12:24 UTC (permalink / raw)
To: devel; +Cc: erdemaktas, jejb, jiewen.yao, min.m.xu, thomas.lendacky, kraxel
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807
This patch series provides fixes for boot VM when use secure boot
supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages.
- Detect FlashNvVarStore before calls PlaformValidateNvVarStore in PlatformInitLib
and skip the validation process if the FlashNvVarStore doesn't exist. Since the
PlatformValidateNvVarStore will return false if the FlashNvVarStore doesn't
exist, this will trigger ASSERT (FALSE) and prevent the guest from moving
forward.
- Init the whole range of EmuVariableNvStore before copy content from the
FlashNvVarStore to EmuVariableNvStore. If the Ftw (Fault Tolerant Write) part
of the EmuVariableNvStore isn't initialized, the FaultToleranteWriteDxe will
use scrambled address to access memory and leads to crash if the VM is a
SEV guest.
- Fix the mapping for FlashNvVarStore. If launch a SEV VM with only OVMF.fd,
the address range for FlashNvVarStore should be mapped as encrypted before
access FlashNvVarStore in PlatformValidateNvVarStore. If launch a SEV VM with
both OVMF_CODE.fd and OVMF_VARS.fd, the address range for FlashNvVarStore
should be mapped as decrypted before access FlashNvVarStore in
PlatformValidateNvVarStore.
---
These patches based on commit:
d4dbe5e101dc ("SecurityPkg/Tcg2Acpi: Revise debug print")
Han Liyang (3):
OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it
OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data
OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it
OvmfPkg/Library/PlatformInitLib/Platform.c | 67 +++++++++++
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 105 ++++++++++++++++++
OvmfPkg/PlatformPei/Platform.c | 6 +
OvmfPkg/PlatformPei/Platform.h | 6 +
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
6 files changed, 186 insertions(+)
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119926): https://edk2.groups.io/g/devel/message/119926
Mute This Topic: https://groups.io/mt/107212891/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread* [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data 2024-07-14 12:24 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang @ 2024-07-14 12:24 ` wojiaohanliyang 0 siblings, 0 replies; 5+ messages in thread From: wojiaohanliyang @ 2024-07-14 12:24 UTC (permalink / raw) To: devel Cc: erdemaktas, jejb, jiewen.yao, min.m.xu, thomas.lendacky, kraxel, hanliyang From: hanliyang <wojiaohanliyang@163.com> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4807 In the case launch with just OVMF.fd, if we just init part of the EmuVariableNvStore, then EmuVariableFvbRuntimeDxe will skip the initialize process of the EmuVariableNvStore and the Ftw (Fault Tolerant Write) part of the EmuVariableNvStore will not be initialized before the Ftw part is accessed. When we launch a SEV guest, the FaultTolerantWriteDxe will get scrambled data when read Ftw part of the EmuVariableNvStore, the FaultToleranteWriteDxe access address specified by the scrambled data will cause invalid address access and crash. The crash message is shown as below. Loading driver at 0x000BDB92000 EntryPoint=0x000BDB95EF4 FaultTolerantWriteDxe.efi InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF BDE01D98 ProtectUefiImageCommon - 0xBDE01040 - 0x00000000BDB92000 - 0x0000000000005B00 Ftw: FtwWorkSpaceLba - 0x40, WorkBlockSize - 0x1000, FtwWorkSpaceBase - 0x0 Ftw: FtwSpareLba - 0x42, SpareBlockSize - 0x1000 Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x40 Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0x0 Ftw: Remaining work space size - FE0 !!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000 !!!! ExceptionData - 0000000000000000 RIP - 00000000BDB92459, CS - 0000000000000038, RFLAGS - 0000000000010286 RAX - 587E3201A019FB0C, RCX - 587E3200E238F994, RDX - 0000000000000001 RBX - 00000000BDE10018, RSP - 00000000BFB79AD8, RBP - 0000000000000FE0 RSI - 00000000BDE100A8, RDI - 00000000BDE10128 R8 - D4642A9DFB7C79BE, R9 - 00000000000003F8, R10 - 00000000BDB96602 R11 - 0000000000000002, R12 - 00000000BDE100A0, R13 - 0000000000000000 R14 - 0000000000000001, R15 - 00000000BFBA76C0 DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 GS - 0000000000000030, SS - 0000000000000030 CR0 - 0000000080010033, CR2 - 0000000000000000, CR3 - 00000000BF801000 CR4 - 0000000000000668, CR8 - 0000000000000000 DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 00000000BF5DC000 0000000000000047, LDTR - 0000000000000000 IDTR - 00000000BEF0C018 0000000000000FFF, TR - 0000000000000000 FXSAVE_STATE - 00000000BFB79730 !!!! Find image based on IP(0xBDB92459) /dev/shm/edk2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe/DEBUG/FaultTolerantWriteDxe.dll (ImageBase=00000000BDB92000, EntryPoint=00000000BDB95EF4) !!!! Fixes: 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore") Signed-off-by: hanliyang <wojiaohanliyang@163.com> --- OvmfPkg/Library/PlatformInitLib/Platform.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c index 0a720a4c2c..5dbc5506f4 100644 --- a/OvmfPkg/Library/PlatformInitLib/Platform.c +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c @@ -905,6 +905,8 @@ PlatformReserveEmuVariableNvStore ( #define CLEARED_ARRAY_STATUS 0x00 +#define ERASED_UINT8 0xff + /** When OVMF is lauched with -bios parameter, UEFI variables will be partially emulated, and non-volatile variables may lose their contents @@ -982,6 +984,24 @@ PlatformInitEmuVariableNvStore ( DEBUG ((DEBUG_INFO, "Init EmuVariableNvStore with the content in FlashNvStorage\n")); + // + // Init the whole EmuVariableNvStore before copy the content from + // FlashNvStorage to the EmuVariableNvStore. + // + // In the case launch with just OVMF.fd, if we just init part of the + // EmuVariableNvStore, then EmuVariableFvbRuntimeDxe will skip the + // initialize process of the EmuVariableNvStore and the Ftw (Fault + // Tolerant Write) part of the EmuVariableNvStore will not be + // initialized before the Ftw part is accessed. When we launch a SEV + // guest, the FaultTolerantWriteDxe will get scrambled data when read + // Ftw part of the EmuVariableNvStore, the FaultToleranteWriteDxe + // access address specified by the scrambled data will cause invalid + // address access and crash. + // + // The method to init EmuVariableNvStore here references + // OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c. + // + SetMem (EmuVariableNvStore, EmuVariableNvStoreSize, ERASED_UINT8); CopyMem (EmuVariableNvStore, Base, Size); return EFI_SUCCESS; -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119928): https://edk2.groups.io/g/devel/message/119928 Mute This Topic: https://groups.io/mt/107212943/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-14 12:25 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-14 12:22 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformInitLib: Detect FlashNvVarStore before validate it wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data wojiaohanliyang 2024-07-14 12:22 ` [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Update mapping of FlashNvVarStore before validate it wojiaohanliyang -- strict thread matches above, loose matches on Subject: below -- 2024-07-14 12:24 [edk2-devel] [PATCH 0/3] Fix boot failure when use secure boot supported (-D SECURE_BOOT_ENABLE=TRUE) OVMF packages wojiaohanliyang 2024-07-14 12:24 ` [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformInitLib: Init the EmuVariableNvStore before copy data wojiaohanliyang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox