On 7/15/24 09:15, Tom Lendacky wrote:
> On 7/14/24 07:24, [email protected] wrote:
>> From: hanliyang <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
