Hi Tom,
Thank you for your response. In fact, I'm unable to proceed with the development of the fix patch locally as I don't have a SEV-SNP hardware for experimentation. However, it has proven to be crucial for effectively testing and completing the patch. Given your expertise and potentially available hardware, would your team be able to take over the fixing of this issue? (bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4807 ) Thank you very much for your time and consideration. Best regards, hanliyang At 2024-07-15 22:32:03, "Lendacky, Thomas via groups.io" <thomas.lendacky=amd....@groups.io> wrote: >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 (#119945): https://edk2.groups.io/g/devel/message/119945 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] -=-=-=-=-=-=-=-=-=-=-=-
