On 5/14/20 2:12 PM, 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.
Not sure if you saw patch 11/11, but I agree that in general, but for
ArmVirtPkg in particular as well, having package level maintainers is
sufficient, and there is no need for maintainer roles beyond that.
But let's discuss this in reply to 11/11
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59549): https://edk2.groups.io/g/devel/message/59549
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]
-=-=-=-=-=-=-=-=-=-=-=-
