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.
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 (#119935): https://edk2.groups.io/g/devel/message/119935
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]
-=-=-=-=-=-=-=-=-=-=-=-
