On 05/14/20 14:12, Laszlo Ersek wrote: > On 05/14/20 11:29, Ard Biesheuvel wrote: >> On 5/14/20 10:40 AM, Sami Mujawar wrote: >>> Kvmtool is a virtual machine manager that enables >>> hosting KVM guests. It essentially provides an >>> emulated platform for guest operating systems. >>> >>> Kvmtool hands of a device tree containing the >>> current hardware configuration to the firmware. >>> >>> A standards-based operating system would use >>> ACPI to consume the platform hardware >>> information, while some operating systems may >>> prefer to use Device Tree. >>> >>> The KvmtoolPlatformDxe performs the platform >>> actions like determining if the firmware should >>> expose ACPI or the Device Tree based hardware >>> description to the operating system. >>> >>> Signed-off-by: Sami Mujawar <sami.muja...@arm.com> >>> --- >>> >>> Notes: >>> v2: >>> - Updated according to review comments. [Sami] >>> v1: >>> - Add kvmtool platform driver to support loading platform [Sami] >>> specific information. >>> - Keep code to initialise the variable storage PCDs in the >>> [Laszlo] >>> platform-specific FVB driver. >>> - Document code derived from >>> [Laszlo] >>> "ArmVirtPkg/PlatformHasAcpiDtDxe" >>> Ref: https://edk2.groups.io/g/devel/topic/30915278#30757 >>> >>> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 93 >>> ++++++++++++++++++++ >>> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 47 ++++++++++ >>> 2 files changed, 140 insertions(+) >>> >>> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c >>> b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c >>> new file mode 100644 >>> index >>> 0000000000000000000000000000000000000000..e7568f66f5ebeb0423fc1c10345cd8dad0800d94 >>> >>> --- /dev/null >>> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c >>> @@ -0,0 +1,93 @@ >>> +/** @file >>> + >>> + The KvmtoolPlatformDxe performs the platform specific >>> initialization like: >>> + - 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. >>> + >>> + SPDX-License-Identifier: BSD-2-Clause-Patent >>> + >>> +**/ >>> + >>> +#include <Guid/VariableFormat.h> >>> +#include <Library/BaseLib.h> >>> +#include <Library/BaseMemoryLib.h> >>> +#include <Library/DebugLib.h> >>> +#include <Library/DxeServicesTableLib.h> >>> +#include <Library/UefiBootServicesTableLib.h> >>> +#include <Library/UefiDriverEntryPoint.h> >>> +#include <Protocol/FdtClient.h> >>> + >>> +/** Decide if the firmware should expose ACPI tables or Device Tree and >>> + install the appropriate protocol interface. >>> + >>> + Note: This function is derived from "ArmVirtPkg/PlatformHasAcpiDtDxe", >>> + by dropping the word size check, and the fw_cfg check. >>> + >>> + @param [in] ImageHandle Handle for this image. >>> + >>> + @retval EFI_SUCCESS Success. >>> + @retval EFI_OUT_OF_RESOURCES There was not enough memory to >>> install the >>> + protocols. >>> + @retval EFI_INVALID_PARAMETER A parameter is invalid. >>> + >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +PlatformHasAcpiDt ( >>> + IN EFI_HANDLE ImageHandle >>> + ) >>> +{ >>> + if (!PcdGetBool (PcdForceNoAcpi)) { >>> + // Expose ACPI tables >>> + return gBS->InstallProtocolInterface ( >>> + &ImageHandle, >>> + &gEdkiiPlatformHasAcpiGuid, >>> + EFI_NATIVE_INTERFACE, >>> + NULL >>> + ); >>> + } >>> + >>> + // Expose the Device Tree. >>> + return gBS->InstallProtocolInterface ( >>> + &ImageHandle, >>> + &gEdkiiPlatformHasDeviceTreeGuid, >>> + EFI_NATIVE_INTERFACE, >>> + NULL >>> + ); >>> +} >>> + >>> +/** Entry point for Kvmtool Platform Dxe >>> + >>> + @param [in] ImageHandle Handle for this image. >>> + @param [in] SystemTable Pointer to the EFI system table. >>> + >>> + @retval EFI_SUCCESS Success. >>> + @retval EFI_OUT_OF_RESOURCES There was not enough memory to >>> install the >>> + protocols. >>> + @retval EFI_INVALID_PARAMETER A parameter is invalid. >>> + >>> +**/ >>> +EFI_STATUS >>> +EFIAPI >>> +KvmtoolPlatformDxeEntryPoint ( >>> + IN EFI_HANDLE ImageHandle, >>> + IN EFI_SYSTEM_TABLE *SystemTable >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + >>> + Status = PlatformHasAcpiDt (ImageHandle); >>> + if (EFI_ERROR (Status)) { >>> + goto Failed; >>> + } >>> + >>> + return Status; >>> + >>> +Failed: >>> + ASSERT_EFI_ERROR (Status); >>> + CpuDeadLoop (); >>> + >>> + return Status; >>> +} >> >> Please don't use CpuDeadLoop()s in your drivers. >> >> Installing a protocol on an image handle like this should not ever fail, >> and if it does, it is unlikely to be an issue in the driver itself. So >> just use ASSERT_EFI_ERROR() here, and return EFI_SUCCESS. > > I think Sami just followed the original code in > "ArmVirtPkg/PlatformHasAcpiDtDxe". > > I'm fine either way: > > Acked-by: Laszlo Ersek <ler...@redhat.com> > > Different question: > > Should we ask Sami to become a designated reviewer (in Maintainers.txt) > for the kvmtool-specific modules under ArmVirtPkg? Personally I'm > unlikely to use kvmtool.
Apologies -- I now see that I had requested that under v1 (in October 2018... a long time ago), and I've also found the last patch now. (I guess I thought of documenting the new modules in Maintainers.txt in the same patches that added the new modules, but end-of-series is fine too.) Thanks! Laszlo > > Thanks > Laszlo > >> >> >>> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf >>> b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf >>> new file mode 100644 >>> index >>> 0000000000000000000000000000000000000000..08a0fe5ce14469133479046385bdd48c22698639 >>> >>> --- /dev/null >>> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf >>> @@ -0,0 +1,47 @@ >>> +#/** @file >>> +# >>> +# The KvmtoolPlatformDxe performs the platform specific >>> initialization like: >>> +# - 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. >>> +# >>> +# SPDX-License-Identifier: BSD-2-Clause-Patent >>> +# >>> +#**/ >>> + >>> +[Defines] >>> + INF_VERSION = 0x0001001B >>> + BASE_NAME = KvmtoolPlatformDxe >>> + FILE_GUID = 7479CCCD-D721-442A-8C73-A72DBB886669 >>> + MODULE_TYPE = DXE_DRIVER >>> + VERSION_STRING = 1.0 >>> + ENTRY_POINT = KvmtoolPlatformDxeEntryPoint >>> + >>> +[Sources] >>> + KvmtoolPlatformDxe.c >>> + >>> +[Packages] >>> + ArmVirtPkg/ArmVirtPkg.dec >>> + EmbeddedPkg/EmbeddedPkg.dec >>> + MdePkg/MdePkg.dec >>> + MdeModulePkg/MdeModulePkg.dec >>> + >>> +[LibraryClasses] >>> + BaseLib >>> + BaseMemoryLib >>> + DebugLib >>> + DxeServicesTableLib >>> + MemoryAllocationLib >>> + UefiBootServicesTableLib >>> + UefiDriverEntryPoint >>> + >>> +[Guids] >>> + gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL >>> + gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL >>> + >>> +[Pcd] >>> + gArmVirtTokenSpaceGuid.PcdForceNoAcpi >>> + >>> +[Depex] >>> + TRUE >>> >> >> >> >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59551): https://edk2.groups.io/g/devel/message/59551 Mute This Topic: https://groups.io/mt/74200911/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
