On 7/15/24 09:15, Tom Lendacky wrote: > On 7/14/24 07:24, wojiaohanliy...@163.com wrote: >> From: hanliyang <wojiaohanliy...@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. >> > > While Secure Boot isn't supported at the moment for SEV-ES / SEV-SNP, > this will cause a boot failure for those types of VMs should it be > enabled. > > SEV-ES results in: > > Invalid MMIO opcode (AF) > ASSERT [SecMain] > /root/kernels/ovmf-build-X64/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c(498): > ((BOOLEAN)(0==1)) > > while SEV-SNP just terminates with an error in Qemu.
My bad, that was before this patch was applied. After applying this patch, I receive: SEV-ES: MMIO using encrypted memory: FFC00000 !!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000 !!!! SEV-SNP just terminates as before. So maybe you need patch #3 to be the first patch in order to maintain git bisection. But after applying all 3 patches, SEV-SNP self terminates for an unsupported #VC error code - 0x404. 0x404 is from accessing a page as encrypted, but it has not been validated. Thanks, Tom > > I haven't looked into what the cause is at this time. > > Thanks, > Tom > >> Fixes: 4f173db8b45b ("OvmfPkg/PlatformInitLib: Add functions for >> EmuVariableNvStore") >> Signed-off-by: hanliyang <wojiaohanliy...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119936): https://edk2.groups.io/g/devel/message/119936 Mute This Topic: https://groups.io/mt/107212942/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-