Kvmtool allows guest VMs to be launched with or without a CFI flash device. The guest hardware configuration can be seen in the device tree that Kvmtool hands off to the guest firmware. Therefore, add support to dynamically detect if a CFI flash device is present. If CFI is present use the NorFlashDxe driver as the backend for variable services; otherwise use emulated runtime variables. The last 2 patches in this series fix a crash due to stack overflow which is observed when running the UEFI shell command 'dmpstore'. The changes can be seen at: https://github.com/samimujawar/edk2/tree/2646_dynamic_cfi_detection_v1 Sami Mujawar (6): ArmPkg: Configure PcdEmuVariableNvModeEnable as a dynamic PCD ArmVirtPkg: Define variables for emulating runtime variables ArmVirtPkg: Fallback to variable emulation if no CFI is found ArmVirtPkg: Dispatch variable service if variable emulation is enabled ArmVirtPkg: Kvmtool: Increase primary core stack size ArmVirtPkg: ArmVirtQemuKernel: Increase primary core stack size ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +- ArmVirtPkg/ArmVirtKvmTool.dsc | 13 +++++-- ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 13 ++++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 4 ++- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 38 +++++++++++++++++--- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 3 +- 7 files changed, 63 insertions(+), 12 deletions(-) -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105016): https://edk2.groups.io/g/devel/message/105016 Mute This Topic: https://groups.io/mt/98987539/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable indicates if a variable driver will emulate the variable NV mode. This PCD is defined as [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]. Some firmware builds may define this PCD as a dynamic PCD and initialise the value at runtime. Therefore, move the PCD declaration from the [FixedPcd] section to the [Pcd] section in the platform boot manager library file PlatformBootManagerLib.inf. Without this change the build would not succeed. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -XXX,XX +XXX,XX @@ [FeaturePcd] [FixedPcd] gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable - gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits @@ -XXX,XX +XXX,XX @@ [FixedPcd] [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable [Guids] gBootDiscoveryPolicyMgrFormsetGuid -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105021): https://edk2.groups.io/g/devel/message/105021 Mute This Topic: https://groups.io/mt/98987546/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- Kvmtool allows guest VMs to be launched with or without a CFI flash device. When the kvmtool option '--flash <flash filename>' is used to launch a guest VM a CFI flash device maps the flash file that was specified at the command line. The NorFlash driver uses this flash as the variable storage backend. However, when the above option is not specified, a CFI flash device is not present. In such cases, the firmware can fallback to use emulated runtime variables (which uses the VMs DRAM as the storage backend). Therefore, define the PCD PcdEmuVariableNvModeEnable required to enable the emulated runtime variable support, but do not enable it by default. The firmware is expected to dynamically discover if the CFI flash is present and subsequently enable NorFlash or emulate the runtime variables. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtKvmTool.dsc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtKvmTool.dsc +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc @@ -XXX,XX +XXX,XX @@ # @file # Workspace file for KVMTool virtual platform. # -# Copyright (c) 2018 - 2022, ARM Limited. All rights reserved. +# Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -XXX,XX +XXX,XX @@ [PcdsDynamicDefault.common] gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x40000 + # Define PCD for emulating Runtime Variable storage when + # CFI flash is absent. + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|FALSE + ## RTC Register address in MMIO space. gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0 gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0 -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105019): https://edk2.groups.io/g/devel/message/105019 Mute This Topic: https://groups.io/mt/98987543/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The kvmtool option '--flash <flash filename>' is used to launch a guests VM with a CFI flash device that maps the flash file specified at the command line. However, kvmtool allows guest VMs to be launched without a CFI flash device. In such scenarios the firmware can utilize the emulated variable storage for UEFI variables. To support this the PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable must be set to TRUE. Therefore, update the NorFlashKvmtoolLib to fallback to variable emulation if a CFI device is not detected. Also improve the error logging. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 38 +++++++++++++++++--- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 3 +- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c @@ -XXX,XX +XXX,XX @@ /** @file An instance of the NorFlashPlatformLib for Kvmtool platform. - Copyright (c) 2020, ARM Ltd. All rights reserved.<BR> + Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -XXX,XX +XXX,XX @@ NorFlashPlatformLibConstructor ( CONST CHAR8 *Label; UINT32 LabelLen; - if (mNorFlashDeviceCount != 0) { + if ((mNorFlashDeviceCount != 0) || PcdGetBool (PcdEmuVariableNvModeEnable)) { return EFI_SUCCESS; } @@ -XXX,XX +XXX,XX @@ NorFlashPlatformLibConstructor ( } if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) { - return SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]); + Status = SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]); + if (EFI_ERROR (Status)) { + DEBUG (( + DEBUG_ERROR, + "ERROR: Failed to setup variable store, Status = %r\n", + Status + )); + ASSERT (0); + } + } else { + DEBUG (( + DEBUG_ERROR, + "ERROR: Invalid Flash device Base address\n" + )); + ASSERT (0); + Status = EFI_NOT_FOUND; + } + } else { + // No Flash device found fallback to Runtime Variable Emulation. + DEBUG (( + DEBUG_INFO, + "INFO: No Flash device found fallback to Runtime Variable Emulation.\n" + )); + Status = PcdSetBoolS (PcdEmuVariableNvModeEnable, TRUE); + if (EFI_ERROR (Status)) { + DEBUG (( + DEBUG_ERROR, + "ERROR: Failed to set PcdEmuVariableNvModeEnable, Status = %r\n", + Status + )); + ASSERT (0); } } - return EFI_NOT_FOUND; + return Status; } diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf @@ -XXX,XX +XXX,XX @@ ## @file # Nor Flash library for Kvmtool. # -# Copyright (c) 2020, ARM Ltd. All rights reserved.<BR> +# Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -XXX,XX +XXX,XX @@ [Pcd] gArmTokenSpaceGuid.PcdFvBaseAddress gArmTokenSpaceGuid.PcdFvSize + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105018): https://edk2.groups.io/g/devel/message/105018 Mute This Topic: https://groups.io/mt/98987541/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The VariableRuntimeDxe links with NvVarStoreFormattedLib which is required to establish the dependency on OvmfPkg\VirtNorFlashDxe. The VirtNorFlashDxe installs the gEdkiiNvVarStoreFormattedGuid to indicate it has finished initialising the flash variable storage and that the variable service can be dispatched. However, the kvmtool guest firmware dynamically detects if CFI flash is absent and sets PcdEmuVariableNvModeEnable to TRUE indicating emulated runtime variable must be used. Therefore, in this scenario install the gEdkiiNvVarStoreFormattedGuid so that the variable service can be dispatched. Also link the NorFlashKvmtoolLib as a NULL library so that it can discover if the CFI flash is absent and setup the PCD PcdEmuVariableNvModeEnable. This is required in case the NorFlashDxe is not yet dispatched. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtKvmTool.dsc | 5 ++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 13 ++++++++++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 4 +++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtKvmTool.dsc +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc @@ -XXX,XX +XXX,XX @@ [Components.common] # # Platform Driver # - ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf + ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf { + <LibraryClasses> + NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf + } OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c @@ -XXX,XX +XXX,XX @@ - It decides if the firmware should expose ACPI or Device Tree-based hardware description to the operating system. - Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. + Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent @@ -XXX,XX +XXX,XX @@ KvmtoolPlatformDxeEntryPoint ( { EFI_STATUS Status; + if (PcdGetBool (PcdEmuVariableNvModeEnable)) { + // The driver implementing the variable service can now be dispatched. + Status = gBS->InstallProtocolInterface ( + &gImageHandle, + &gEdkiiNvVarStoreFormattedGuid, + EFI_NATIVE_INTERFACE, + NULL + ); + ASSERT_EFI_ERROR (Status); + } + Status = PlatformHasAcpiDt (ImageHandle); ASSERT_EFI_ERROR (Status); diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf @@ -XXX,XX +XXX,XX @@ # - It decides if the firmware should expose ACPI or Device Tree-based # hardware description to the operating system. # -# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. +# Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -XXX,XX +XXX,XX @@ [LibraryClasses] UefiDriverEntryPoint [Guids] + gEdkiiNvVarStoreFormattedGuid ## SOMETIMES_PRODUCES ## PROTOCOL gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL [Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gUefiOvmfPkgTokenSpaceGuid.PcdForceNoAcpi [Depex] -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105017): https://edk2.groups.io/g/devel/message/105017 Mute This Topic: https://groups.io/mt/98987540/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard" enabled stack overflow detection for ArmVirtPkg. Following this patch, running UEFI shell command 'dmpstore' resulted in a crash indicating a stack overflow. Invoking 'dmpstore' results in recursive calls to CascadeProcessVariables () which apparently consumes the available stack space and overflows. Therefore, increase the primary core stack size. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtKvmTool.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtKvmTool.dsc +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc @@ -XXX,XX +XXX,XX @@ [PcdsFixedAtBuild.common] gArmTokenSpaceGuid.PcdVFPEnabled|1 !endif - gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000 + gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105015): https://edk2.groups.io/g/devel/message/105015 Mute This Topic: https://groups.io/mt/98987538/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard" enabled stack overflow detection for ArmVirtPkg. Following this patch, running UEFI shell command 'dmpstore' resulted in a crash indicating a stack overflow. Invoking 'dmpstore' results in recursive calls to CascadeProcessVariables () which apparently consumes the available stack space and overflows. Therefore, increase the primary core stack size. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -XXX,XX +XXX,XX @@ [PcdsFixedAtBuild.common] gArmTokenSpaceGuid.PcdVFPEnabled|1 !endif - gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000 + gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 !if $(NETWORK_TLS_ENABLE) == TRUE -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105020): https://edk2.groups.io/g/devel/message/105020 Mute This Topic: https://groups.io/mt/98987544/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- Kvmtool allows guest VMs to be launched with or without a CFI flash device. The guest hardware configuration can be seen in the device tree that Kvmtool hands off to the guest firmware. Therefore, add support to dynamically detect if a CFI flash device is present. If CFI is present use the NorFlashDxe driver as the backend for variable services; otherwise use emulated runtime variables. The last patch in this series fix a crash due to stack overflow which is observed when running the UEFI shell command 'dmpstore'. The first 4 patches in this series have not been modified and are resent with the v2 series. The changes can be seen at: https://github.com/samimujawar/edk2/tree/2646_dynamic_cfi_detection_v2 Sami Mujawar (5): ArmPkg: Configure PcdEmuVariableNvModeEnable as a dynamic PCD ArmVirtPkg: Define variables for emulating runtime variables ArmVirtPkg: Fallback to variable emulation if no CFI is found ArmVirtPkg: Dispatch variable service if variable emulation is enabled ArmVirtPkg/PrePi: Allocate separate stack for Dxe phase ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +- ArmVirtPkg/ArmVirtKvmTool.dsc | 11 ++++-- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 13 ++++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 4 ++- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 38 +++++++++++++++++--- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 3 +- ArmVirtPkg/PrePi/PrePi.c | 4 +-- 7 files changed, 63 insertions(+), 12 deletions(-) -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105076): https://edk2.groups.io/g/devel/message/105076 Mute This Topic: https://groups.io/mt/99013766/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable indicates if a variable driver will emulate the variable NV mode. This PCD is defined as [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]. Some firmware builds may define this PCD as a dynamic PCD and initialise the value at runtime. Therefore, move the PCD declaration from the [FixedPcd] section to the [Pcd] section in the platform boot manager library file PlatformBootManagerLib.inf. Without this change the build would not succeed. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -XXX,XX +XXX,XX @@ [FeaturePcd] [FixedPcd] gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable - gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits @@ -XXX,XX +XXX,XX @@ [FixedPcd] [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable [Guids] gBootDiscoveryPolicyMgrFormsetGuid -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105078): https://edk2.groups.io/g/devel/message/105078 Mute This Topic: https://groups.io/mt/99013771/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- Kvmtool allows guest VMs to be launched with or without a CFI flash device. When the kvmtool option '--flash <flash filename>' is used to launch a guest VM a CFI flash device maps the flash file that was specified at the command line. The NorFlash driver uses this flash as the variable storage backend. However, when the above option is not specified, a CFI flash device is not present. In such cases, the firmware can fallback to use emulated runtime variables (which uses the VMs DRAM as the storage backend). Therefore, define the PCD PcdEmuVariableNvModeEnable required to enable the emulated runtime variable support, but do not enable it by default. The firmware is expected to dynamically discover if the CFI flash is present and subsequently enable NorFlash or emulate the runtime variables. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtKvmTool.dsc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtKvmTool.dsc +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc @@ -XXX,XX +XXX,XX @@ # @file # Workspace file for KVMTool virtual platform. # -# Copyright (c) 2018 - 2022, ARM Limited. All rights reserved. +# Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -XXX,XX +XXX,XX @@ [PcdsDynamicDefault.common] gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x40000 + # Define PCD for emulating Runtime Variable storage when + # CFI flash is absent. + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|FALSE + ## RTC Register address in MMIO space. gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0 gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0 -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105080): https://edk2.groups.io/g/devel/message/105080 Mute This Topic: https://groups.io/mt/99013776/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The kvmtool option '--flash <flash filename>' is used to launch a guests VM with a CFI flash device that maps the flash file specified at the command line. However, kvmtool allows guest VMs to be launched without a CFI flash device. In such scenarios the firmware can utilize the emulated variable storage for UEFI variables. To support this the PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable must be set to TRUE. Therefore, update the NorFlashKvmtoolLib to fallback to variable emulation if a CFI device is not detected. Also improve the error logging. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 38 +++++++++++++++++--- ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 3 +- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c @@ -XXX,XX +XXX,XX @@ /** @file An instance of the NorFlashPlatformLib for Kvmtool platform. - Copyright (c) 2020, ARM Ltd. All rights reserved.<BR> + Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -XXX,XX +XXX,XX @@ NorFlashPlatformLibConstructor ( CONST CHAR8 *Label; UINT32 LabelLen; - if (mNorFlashDeviceCount != 0) { + if ((mNorFlashDeviceCount != 0) || PcdGetBool (PcdEmuVariableNvModeEnable)) { return EFI_SUCCESS; } @@ -XXX,XX +XXX,XX @@ NorFlashPlatformLibConstructor ( } if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) { - return SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]); + Status = SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]); + if (EFI_ERROR (Status)) { + DEBUG (( + DEBUG_ERROR, + "ERROR: Failed to setup variable store, Status = %r\n", + Status + )); + ASSERT (0); + } + } else { + DEBUG (( + DEBUG_ERROR, + "ERROR: Invalid Flash device Base address\n" + )); + ASSERT (0); + Status = EFI_NOT_FOUND; + } + } else { + // No Flash device found fallback to Runtime Variable Emulation. + DEBUG (( + DEBUG_INFO, + "INFO: No Flash device found fallback to Runtime Variable Emulation.\n" + )); + Status = PcdSetBoolS (PcdEmuVariableNvModeEnable, TRUE); + if (EFI_ERROR (Status)) { + DEBUG (( + DEBUG_ERROR, + "ERROR: Failed to set PcdEmuVariableNvModeEnable, Status = %r\n", + Status + )); + ASSERT (0); } } - return EFI_NOT_FOUND; + return Status; } diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf @@ -XXX,XX +XXX,XX @@ ## @file # Nor Flash library for Kvmtool. # -# Copyright (c) 2020, ARM Ltd. All rights reserved.<BR> +# Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -XXX,XX +XXX,XX @@ [Pcd] gArmTokenSpaceGuid.PcdFvBaseAddress gArmTokenSpaceGuid.PcdFvSize + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105075): https://edk2.groups.io/g/devel/message/105075 Mute This Topic: https://groups.io/mt/99013765/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The VariableRuntimeDxe links with NvVarStoreFormattedLib which is required to establish the dependency on OvmfPkg\VirtNorFlashDxe. The VirtNorFlashDxe installs the gEdkiiNvVarStoreFormattedGuid to indicate it has finished initialising the flash variable storage and that the variable service can be dispatched. However, the kvmtool guest firmware dynamically detects if CFI flash is absent and sets PcdEmuVariableNvModeEnable to TRUE indicating emulated runtime variable must be used. Therefore, in this scenario install the gEdkiiNvVarStoreFormattedGuid so that the variable service can be dispatched. Also link the NorFlashKvmtoolLib as a NULL library so that it can discover if the CFI flash is absent and setup the PCD PcdEmuVariableNvModeEnable. This is required in case the NorFlashDxe is not yet dispatched. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/ArmVirtKvmTool.dsc | 5 ++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 13 ++++++++++++- ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 4 +++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/ArmVirtKvmTool.dsc +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc @@ -XXX,XX +XXX,XX @@ [Components.common] # # Platform Driver # - ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf + ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf { + <LibraryClasses> + NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf + } OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c @@ -XXX,XX +XXX,XX @@ - It decides if the firmware should expose ACPI or Device Tree-based hardware description to the operating system. - Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. + Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent @@ -XXX,XX +XXX,XX @@ KvmtoolPlatformDxeEntryPoint ( { EFI_STATUS Status; + if (PcdGetBool (PcdEmuVariableNvModeEnable)) { + // The driver implementing the variable service can now be dispatched. + Status = gBS->InstallProtocolInterface ( + &gImageHandle, + &gEdkiiNvVarStoreFormattedGuid, + EFI_NATIVE_INTERFACE, + NULL + ); + ASSERT_EFI_ERROR (Status); + } + Status = PlatformHasAcpiDt (ImageHandle); ASSERT_EFI_ERROR (Status); diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf index XXXXXXX..XXXXXXX 100644 --- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf @@ -XXX,XX +XXX,XX @@ # - It decides if the firmware should expose ACPI or Device Tree-based # hardware description to the operating system. # -# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. +# Copyright (c) 2018 - 2023, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -XXX,XX +XXX,XX @@ [LibraryClasses] UefiDriverEntryPoint [Guids] + gEdkiiNvVarStoreFormattedGuid ## SOMETIMES_PRODUCES ## PROTOCOL gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL [Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable gUefiOvmfPkgTokenSpaceGuid.PcdForceNoAcpi [Depex] -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105079): https://edk2.groups.io/g/devel/message/105079 Mute This Topic: https://groups.io/mt/99013774/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard" enabled stack overflow detection for ArmVirtPkg. Following this patch, running UEFI shell command 'dmpstore' resulted in a crash indicating a stack overflow. Invoking 'dmpstore' results in recursive calls to CascadeProcessVariables () which apparently consumes the available stack space and overflows. Normally, SEC and PEI run off the initial stack, and the DxeIpl PEIM is in charge of launching the DxeCore with a full-sized stack and remapping it non-executable as well. PrePi platforms take some shortcuts and the DXE and BDS run off the initial stack which is relatively small. It is therefore desirable to allocate 128 KiB worth of boot services data memory as the stack for the Dxe phase. The PrePiMain () in ArmVirtPkg/PrePi/PrePi.c invokes the LoadDxeCoreFromFv () to load the Dxe core and transfers control. The second parameter to LoadDxeCoreFromFv () is the stack size, which is currently set to 0. LoadDxeCoreFromFv () is implemented in PrePiLib and if the stack size is 0, it continues to use the initial stack. However, if a stack size is specified in the call to LoadDxeCoreFromFv (), memory is allocated for a new stack and the stack is switched to use the newly allocated stack for the Dxe phase. Therefore, specify 128 KiB as the stack size in the call to LoadDxeCoreFromFv () so that a separate stack is allocated and used for the Dxe phase. Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> --- ArmVirtPkg/PrePi/PrePi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c index XXXXXXX..XXXXXXX 100755 --- a/ArmVirtPkg/PrePi/PrePi.c +++ b/ArmVirtPkg/PrePi/PrePi.c @@ -XXX,XX +XXX,XX @@ /** @file * -* Copyright (c) 2011-2014, ARM Limited. All rights reserved. +* Copyright (c) 2011-2023, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent * @@ -XXX,XX +XXX,XX @@ PrePiMain ( ASSERT_EFI_ERROR (Status); // Load the DXE Core and transfer control to it - Status = LoadDxeCoreFromFv (NULL, 0); + Status = LoadDxeCoreFromFv (NULL, SIZE_128KB); ASSERT_EFI_ERROR (Status); } -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105077): https://edk2.groups.io/g/devel/message/105077 Mute This Topic: https://groups.io/mt/99013767/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-