* [PATCH v1 0/5] Add ACPI support for Kvmtool
@ 2021-06-23 14:06 PierreGondois
2021-06-23 14:06 ` [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions PierreGondois
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Pierre Gondois <Pierre.Gondois@arm.com>
Kvmtool dynamically generates a device tree describing the platform
to boot on. Using the patch-sets listed below, the DynamicTables
framework generates ACPI tables describing a similar platform.
This patch-set:
- adds a ConfigurationManager allowing to generate ACPI tables
for Kvmtool
- adds the acpiview command line utility to the ArmVirtPkg
- update ArmVirtPkg.ci.yaml to add new words and use the
DynamicTablesPkg
This patch sets also set the default platform description format
to ACPI instead of the device tree (c.f.: PcdForceNoAcpi is set
to FALSE).
The changes can be seen at: https://github.com/PierreARM/edk2/tree/1456_Add_ACPI_support_for_Kvmtool_v1
The results of the CI can be seen at: https://github.com/tianocore/edk2/pull/1753
This patch-set is dependent over the following patch-sets:
[PATCH v1 00/10] Various DynamicTablesPkg modifications
https://edk2.groups.io/g/devel/message/76929
and:
[PATCH v1 00/13] Create a SSDT CPU topology generator
https://edk2.groups.io/g/devel/message/76941
and:
[PATCH v1 0/7] Create a SSDT PCIe generator
https://edk2.groups.io/g/devel/message/76958
and:
[PATCH v1 00/14] Implement a FdtHwInfoParserLib
https://edk2.groups.io/g/devel/message/76967
and:
[PATCH v1 0/5] Add DynamicPlatRepoLib
https://edk2.groups.io/g/devel/message/76984
Pierre Gondois (1):
ArmVirtPkg: Add cspell exceptions
Sami Mujawar (4):
ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware
ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
ArmVirtPkg: Enable ACPI support for Kvmtool
ArmVirtPkg: Enable Acpiview for ArmVirtPkg
ArmVirtPkg/ArmVirt.dsc.inc | 3 +-
ArmVirtPkg/ArmVirtKvmTool.dsc | 18 +-
ArmVirtPkg/ArmVirtKvmTool.fdf | 11 +
ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 +
.../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl | 19 +
.../KvmtoolCfgMgrDxe/ConfigurationManager.c | 948 ++++++++++++++++++
.../KvmtoolCfgMgrDxe/ConfigurationManager.h | 94 ++
.../ConfigurationManagerDxe.inf | 58 ++
8 files changed, 1151 insertions(+), 3 deletions(-)
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
--
2.17.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
@ 2021-06-23 14:06 ` PierreGondois
2021-06-24 11:11 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware PierreGondois
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Pierre Gondois <Pierre.Gondois@arm.com>
The cpsell tool checks for unknown words in the upstream CI.
Add some new words to the list of exceptions.
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 5f427e57233e..e3f30d69e89a 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -47,6 +47,7 @@
"MdePkg/MdePkg.dec",
"MdeModulePkg/MdeModulePkg.dec",
"ArmVirtPkg/ArmVirtPkg.dec",
+ "DynamicTablesPkg/DynamicTablesPkg.dec",
"NetworkPkg/NetworkPkg.dec",
"ArmPkg/ArmPkg.dec",
"OvmfPkg/OvmfPkg.dec",
@@ -97,6 +98,8 @@
"AuditOnly": False, # Fails right now with over 270 errors
"IgnoreFiles": [], # use gitignore syntax to ignore errors in matching files
"ExtendWords": [
+ "armltd",
+ "ssdts",
"setjump",
"plong",
"lparam",
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
2021-06-23 14:06 ` [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions PierreGondois
@ 2021-06-23 14:06 ` PierreGondois
2021-06-24 11:13 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager " PierreGondois
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Sami Mujawar <sami.mujawar@arm.com>
Most ACPI tables for Kvmtool firmware are dynamically
generated. The AML code is also generated at runtime
for most components in appropriate SSDTs.
Although there may not be much to describe in the DSDT,
the DSDT table is mandatory.
Therefore, add an empty stub for DSDT.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
new file mode 100644
index 000000000000..8467d1ede4ec
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
@@ -0,0 +1,19 @@
+/** @file
+ Differentiated System Description Table Fields (DSDT)
+
+ Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
+ Scope (_SB) {
+ // Most ACPI tables for Kvmtool firmware are
+ // dynamically generated. The AML code is also
+ // generated at runtime for most components in
+ // appropriate SSDTs.
+ // Although there may not be much to describe
+ // in the DSDT, the DSDT table is mandatory.
+ // Therefore, add an empty stub for DSDT.
+ } // Scope (_SB)
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
2021-06-23 14:06 ` [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions PierreGondois
2021-06-23 14:06 ` [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware PierreGondois
@ 2021-06-23 14:06 ` PierreGondois
2021-06-24 12:30 ` [edk2-devel] " Laszlo Ersek
2021-06-24 12:35 ` Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool PierreGondois
` (3 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Sami Mujawar <sami.mujawar@arm.com>
Add Configuration Manager to enable ACPI tables for Kvmtool
firmware. The Configuration Manager for Kvmtool uses the DT
Hardware Information Parser module (FdtHwInfoParser) to parse
the DT provided by Kvmtool. The FdtHwInfoParser parses the DT
and invokes the callback function HW_INFO_ADD_OBJECT to add
the Configuration Manager objects to the Platform Information
repository.
The information for some Configuration Manager objects may not
be available in the DT. Such objects are initialised locally
by the Configuration Manager.
Support for the following ACPI tables is provided:
- DBG2
- DSDT (Empty stub)
- FADT
- GTDT
- MADT
- SPCR
- SSDT (Cpu Hierarchy)
- SSDT (Pcie bus)
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
ArmVirtPkg/ArmVirtKvmTool.dsc | 3 +
.../KvmtoolCfgMgrDxe/ConfigurationManager.c | 948 ++++++++++++++++++
.../KvmtoolCfgMgrDxe/ConfigurationManager.h | 94 ++
.../ConfigurationManagerDxe.inf | 58 ++
4 files changed, 1103 insertions(+)
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 3bd1cc72a1eb..920880796ac2 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -71,6 +71,9 @@ [LibraryClasses.common]
PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+ HwInfoParserLib|DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
+ DynamicPlatRepoLib|DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+
[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
new file mode 100644
index 000000000000..07b8b403dd4a
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
@@ -0,0 +1,948 @@
+/** @file
+ Configuration Manager Dxe
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#include <IndustryStandard/DebugPort2Table.h>
+#include <IndustryStandard/IoRemappingTable.h>
+#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DynamicPlatRepoLib.h>
+#include <Library/HobLib.h>
+#include <Library/HwInfoParserLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/TableHelperLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+
+#include "ConfigurationManager.h"
+
+/** The platform configuration repository information.
+*/
+STATIC
+EDKII_PLATFORM_REPOSITORY_INFO KvmtoolPlatRepositoryInfo = {
+ /// Configuration Manager information
+ { CONFIGURATION_MANAGER_REVISION, CFG_MGR_OEM_ID },
+
+ // ACPI Table List
+ {
+ // FADT Table
+ {
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
+ NULL
+ },
+ // GTDT Table
+ {
+ EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdGtdt),
+ NULL
+ },
+ // MADT Table
+ {
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
+ NULL
+ },
+ // SPCR Table
+ {
+ EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSpcr),
+ NULL
+ },
+ // DSDT Table
+ {
+ EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ 0, // Unused
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDsdt),
+ (EFI_ACPI_DESCRIPTION_HEADER*)dsdt_aml_code
+ },
+ // SSDT Cpu Hierarchy Table
+ {
+ EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ 0, // Unused
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtCpuTopology),
+ NULL
+ },
+ // DBG2 Table
+ {
+ EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+ EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDbg2),
+ NULL
+ },
+ // PCI MCFG Table
+ {
+ EFI_ACPI_6_3_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMcfg),
+ NULL
+ },
+ // SSDT table describing the PCI root complex
+ {
+ EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ 0, // Unused
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtPciExpress),
+ NULL
+ },
+ // IORT Table
+ {
+ EFI_ACPI_6_3_IO_REMAPPING_TABLE_SIGNATURE,
+ EFI_ACPI_IO_REMAPPING_TABLE_REVISION,
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdIort),
+ NULL
+ },
+ },
+
+ // Power management profile information
+ { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Profile
+
+ // ITS group node
+ {
+ // Reference token for this Iort node
+ REFERENCE_TOKEN (ItsGroupInfo),
+ // The number of ITS identifiers in the ITS node.
+ 1,
+ // Reference token for the ITS identifier array
+ REFERENCE_TOKEN (ItsIdentifierArray)
+ },
+ // ITS identifier array
+ {
+ {
+ // The ITS Identifier
+ 0
+ }
+ },
+
+ // Root Complex node info
+ {
+ // Reference token for this Iort node
+ REFERENCE_TOKEN (RootComplexInfo),
+ // Number of ID mappings
+ 1,
+ // Reference token for the ID mapping array
+ REFERENCE_TOKEN (DeviceIdMapping[0]),
+
+ // Memory access properties : Cache coherent attributes
+ EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
+ // Memory access properties : Allocation hints
+ 0,
+ // Memory access properties : Memory access flags
+ 0,
+ // ATS attributes
+ EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED,
+ // PCI segment number
+ 0
+ },
+
+ // Array of Device ID mappings
+ {
+ /* RootComplex -> ITS Group
+ */
+ // Device ID mapping for Root complex node
+ {
+ // Input base
+ 0x0,
+ // Number of input IDs
+ 0x0000FFFF,
+ // Output Base
+ 0x0,
+ // Output reference
+ REFERENCE_TOKEN (ItsGroupInfo),
+ // Flags
+ 0
+ },
+ },
+};
+
+/** A helper function for returning the Configuration Manager Objects.
+
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Object Pointer to the Object(s).
+ @param [in] ObjectSize Total size of the Object(s).
+ @param [in] ObjectCount Number of Objects.
+ @param [in, out] CmObjectDesc Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObject (
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN VOID * Object,
+ IN CONST UINTN ObjectSize,
+ IN CONST UINTN ObjectCount,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObjectDesc
+ )
+{
+ CmObjectDesc->ObjectId = CmObjectId;
+ CmObjectDesc->Size = ObjectSize;
+ CmObjectDesc->Data = (VOID*)Object;
+ CmObjectDesc->Count = ObjectCount;
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
+ CmObjectId,
+ CmObjectDesc->Data,
+ CmObjectDesc->Size,
+ CmObjectDesc->Count
+ ));
+ return EFI_SUCCESS;
+}
+
+/** A helper function for returning the Configuration Manager Objects that
+ match the token.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Object Pointer to the Object(s).
+ @param [in] ObjectSize Total size of the Object(s).
+ @param [in] ObjectCount Number of Objects.
+ @param [in] Token A token identifying the object.
+ @param [in] HandlerProc A handler function to search the object
+ referenced by the token.
+ @param [in, out] CmObjectDesc Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObjectRefByToken (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN VOID * Object,
+ IN CONST UINTN ObjectSize,
+ IN CONST UINTN ObjectCount,
+ IN CONST CM_OBJECT_TOKEN Token,
+ IN CONST CM_OBJECT_HANDLER_PROC HandlerProc,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObjectDesc
+ )
+{
+ EFI_STATUS Status;
+ CmObjectDesc->ObjectId = CmObjectId;
+ if (Token == CM_NULL_TOKEN) {
+ CmObjectDesc->Size = ObjectSize;
+ CmObjectDesc->Data = (VOID*)Object;
+ CmObjectDesc->Count = ObjectCount;
+ Status = EFI_SUCCESS;
+ } else {
+ Status = HandlerProc (This, CmObjectId, Token, CmObjectDesc);
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Token = 0x%p, CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
+ (VOID*)Token,
+ CmObjectId,
+ CmObjectDesc->Data,
+ CmObjectDesc->Size,
+ CmObjectDesc->Count
+ ));
+ return Status;
+}
+
+/** Return an ITS identifier array.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token A token for identifying the object
+ @param [out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetItsIdentifierArray (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token,
+ OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ PlatformRepo = This->PlatRepoInfo;
+
+ if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->ItsIdentifierArray) {
+ return EFI_NOT_FOUND;
+ }
+
+ CmObject->ObjectId = CmObjectId;
+ CmObject->Size = sizeof (PlatformRepo->ItsIdentifierArray);
+ CmObject->Data = (VOID*)&PlatformRepo->ItsIdentifierArray;
+ CmObject->Count = ARRAY_SIZE (PlatformRepo->ItsIdentifierArray);
+ return EFI_SUCCESS;
+}
+
+/** Return a device Id mapping array.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token A token for identifying the object
+ @param [out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetDeviceIdMappingArray (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token,
+ OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ PlatformRepo = This->PlatRepoInfo;
+
+ if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->DeviceIdMapping[0]) {
+ return EFI_NOT_FOUND;
+ }
+
+ CmObject->ObjectId = CmObjectId;
+ CmObject->Size = sizeof (CM_ARM_ID_MAPPING);
+ CmObject->Data = (VOID*)Token;
+ CmObject->Count = 1;
+ return EFI_SUCCESS;
+}
+
+/** Function pointer called by the parser to add information.
+
+ Callback function that the parser can use to add new
+ CmObj. This function must copy the CmObj data and not rely on
+ the parser preserving the CmObj memory.
+ This function is responsible of the Token allocation.
+
+ @param [in] ParserHandle A handle to the parser instance.
+ @param [in] Context A pointer to the caller's context provided in
+ HwInfoParserInit ().
+ @param [in] CmObjDesc CM_OBJ_DESCRIPTOR containing the CmObj(s) to add.
+ @param [out] Token If provided and success, contain the token
+ generated for the CmObj.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HwInfoAdd (
+ IN HW_INFO_PARSER_HANDLE ParserHandle,
+ IN VOID * Context,
+ IN CONST CM_OBJ_DESCRIPTOR * CmObjDesc,
+ OUT CM_OBJECT_TOKEN * Token OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+
+ if ((ParserHandle == NULL) ||
+ (Context == NULL) ||
+ (CmObjDesc == NULL)) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ PlatformRepo = (EDKII_PLATFORM_REPOSITORY_INFO*)Context;
+
+#ifndef MDEPKG_NDEBUG
+ // Print the received objects.
+ ParseCmObjDesc (CmObjDesc);
+#endif
+
+ Status = DynPlatRepoAddObject (
+ PlatformRepo->DynamicPlatformRepo,
+ CmObjDesc,
+ Token
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ }
+ return Status;
+}
+
+/** Cleanup the platform configuration repository.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+
+ @retval EFI_SUCCESS Success
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+CleanupPlatformRepository (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This
+ )
+{
+ EFI_STATUS Status;
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+
+ if (This == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ PlatformRepo = This->PlatRepoInfo;
+
+ // Shutdown the dynamic repo and free all objects.
+ Status = DynamicPlatRepoShutdown (PlatformRepo->DynamicPlatformRepo);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Shutdown parser.
+ Status = HwInfoParserShutdown (PlatformRepo->FdtParserHandle);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ }
+ return Status;
+}
+
+/** Initialize the platform configuration repository.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+
+ @retval EFI_SUCCESS Success
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_OUT_OF_RESOURCES An allocation has failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InitializePlatformRepository (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This
+ )
+{
+ EFI_STATUS Status;
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+ VOID * Hob;
+
+ if (This == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Hob = GetFirstGuidHob (&gFdtHobGuid);
+ if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
+ ASSERT (0);
+ return EFI_NOT_FOUND;
+ }
+
+ PlatformRepo = This->PlatRepoInfo;
+ PlatformRepo->FdtBase = (VOID *)*(UINTN*)GET_GUID_HOB_DATA (Hob);
+
+ // Initialise the dynamic platform repository.
+ Status = DynamicPlatRepoInit (&PlatformRepo->DynamicPlatformRepo);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Initialise the FDT parser
+ Status = HwInfoParserInit (
+ PlatformRepo->FdtBase,
+ PlatformRepo,
+ HwInfoAdd,
+ &PlatformRepo->FdtParserHandle
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ Status = HwInfoParse (PlatformRepo->FdtParserHandle);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ Status = DynamicPlatRepoFinalise (PlatformRepo->DynamicPlatformRepo);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ return EFI_SUCCESS;
+
+error_handler:
+ CleanupPlatformRepository (This);
+ return Status;
+}
+
+/** Return a standard namespace object.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token An optional token identifying the object. If
+ unused this must be CM_NULL_TOKEN.
+ @param [in, out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetStandardNameSpaceObject (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EFI_STATUS Status;
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+ UINTN AcpiTableCount;
+ CM_OBJ_DESCRIPTOR CmObjDesc;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (This != NULL);
+ ASSERT (CmObject != NULL);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = EFI_NOT_FOUND;
+ PlatformRepo = This->PlatRepoInfo;
+
+ switch (GET_CM_OBJECT_ID (CmObjectId)) {
+ case EStdObjCfgMgrInfo:
+ Status = HandleCmObject (
+ CmObjectId,
+ &PlatformRepo->CmInfo,
+ sizeof (PlatformRepo->CmInfo),
+ 1,
+ CmObject
+ );
+ break;
+
+ case EStdObjAcpiTableList:
+ AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
+
+ // Get Pci config space information.
+ Status = DynamicPlatRepoGetObject (
+ PlatformRepo->DynamicPlatformRepo,
+ CREATE_CM_ARM_OBJECT_ID (EArmObjPciConfigSpaceInfo),
+ CM_NULL_TOKEN,
+ &CmObjDesc
+ );
+ if (Status == EFI_NOT_FOUND) {
+ // The last 3 tables are for PCIe. If PCIe information is not
+ // present, Kvmtool was launched without the PCIe option.
+ // Therefore, reduce the table count by 3.
+ AcpiTableCount -= 3;
+ } else if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Get the Gic version.
+ Status = DynamicPlatRepoGetObject (
+ PlatformRepo->DynamicPlatformRepo,
+ CREATE_CM_ARM_OBJECT_ID (EArmObjGicDInfo),
+ CM_NULL_TOKEN,
+ &CmObjDesc
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+ if (((CM_ARM_GICD_INFO*)CmObjDesc.Data)->GicVersion < 3) {
+ // IORT is only required for GicV3/4
+ AcpiTableCount -= 1;
+ }
+
+ Status = HandleCmObject (
+ CmObjectId,
+ PlatformRepo->CmAcpiTableList,
+ (sizeof (PlatformRepo->CmAcpiTableList[0]) * AcpiTableCount),
+ AcpiTableCount,
+ CmObject
+ );
+ break;
+
+ default: {
+ Status = EFI_NOT_FOUND;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Object 0x%x. Status = %r\n",
+ CmObjectId,
+ Status
+ ));
+ break;
+ }
+ }
+
+ return Status;
+}
+
+/** Return an ARM namespace object.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token An optional token identifying the object. If
+ unused this must be CM_NULL_TOKEN.
+ @param [in, out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetArmNameSpaceObject (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EFI_STATUS Status;
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (This != NULL);
+ ASSERT (CmObject != NULL);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = EFI_NOT_FOUND;
+ PlatformRepo = This->PlatRepoInfo;
+
+ // First check among the static objects.
+ switch (GET_CM_OBJECT_ID (CmObjectId)) {
+ case EArmObjPowerManagementProfileInfo:
+ Status = HandleCmObject (
+ CmObjectId,
+ &PlatformRepo->PmProfileInfo,
+ sizeof (PlatformRepo->PmProfileInfo),
+ 1,
+ CmObject
+ );
+ break;
+
+ case EArmObjItsGroup:
+ Status = HandleCmObject (
+ CmObjectId,
+ &PlatformRepo->ItsGroupInfo,
+ sizeof (PlatformRepo->ItsGroupInfo),
+ 1,
+ CmObject
+ );
+ break;
+
+ case EArmObjGicItsIdentifierArray:
+ Status = HandleCmObjectRefByToken (
+ This,
+ CmObjectId,
+ PlatformRepo->ItsIdentifierArray,
+ sizeof (PlatformRepo->ItsIdentifierArray),
+ ARRAY_SIZE (PlatformRepo->ItsIdentifierArray),
+ Token,
+ GetItsIdentifierArray,
+ CmObject
+ );
+ break;
+
+ case EArmObjRootComplex:
+ Status = HandleCmObject (
+ CmObjectId,
+ &PlatformRepo->RootComplexInfo,
+ sizeof (PlatformRepo->RootComplexInfo),
+ 1,
+ CmObject
+ );
+ break;
+
+ case EArmObjIdMappingArray:
+ Status = HandleCmObjectRefByToken (
+ This,
+ CmObjectId,
+ PlatformRepo->DeviceIdMapping,
+ sizeof (PlatformRepo->DeviceIdMapping),
+ ARRAY_SIZE (PlatformRepo->DeviceIdMapping),
+ Token,
+ GetDeviceIdMappingArray,
+ CmObject
+ );
+ break;
+
+ default:
+ // No match found among the static objects.
+ // Check the dynamic objects.
+ Status = DynamicPlatRepoGetObject (
+ PlatformRepo->DynamicPlatformRepo,
+ CmObjectId,
+ Token,
+ CmObject
+ );
+ break;
+ } // switch
+
+ if (Status == EFI_NOT_FOUND) {
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: Object 0x%x. Status = %r\n",
+ CmObjectId,
+ Status
+ ));
+ } else {
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ return Status;
+}
+
+/** Return an OEM namespace object.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token An optional token identifying the object. If
+ unused this must be CM_NULL_TOKEN.
+ @param [in, out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetOemNameSpaceObject (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (This != NULL);
+ ASSERT (CmObject != NULL);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ switch (GET_CM_OBJECT_ID (CmObjectId)) {
+ default: {
+ Status = EFI_NOT_FOUND;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Object 0x%x. Status = %r\n",
+ CmObjectId,
+ Status
+ ));
+ break;
+ }
+ }
+
+ return Status;
+}
+
+/** The GetObject function defines the interface implemented by the
+ Configuration Manager Protocol for returning the Configuration
+ Manager Objects.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token An optional token identifying the object. If
+ unused this must be CM_NULL_TOKEN.
+ @param [in, out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+ArmKvmtoolPlatformGetObject (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EFI_STATUS Status;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (This != NULL);
+ ASSERT (CmObject != NULL);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ switch (GET_CM_NAMESPACE_ID (CmObjectId)) {
+ case EObjNameSpaceStandard:
+ Status = GetStandardNameSpaceObject (This, CmObjectId, Token, CmObject);
+ break;
+ case EObjNameSpaceArm:
+ Status = GetArmNameSpaceObject (This, CmObjectId, Token, CmObject);
+ break;
+ case EObjNameSpaceOem:
+ Status = GetOemNameSpaceObject (This, CmObjectId, Token, CmObject);
+ break;
+ default: {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Unknown Namespace Object = 0x%x. Status = %r\n",
+ CmObjectId,
+ Status
+ ));
+ break;
+ }
+ }
+
+ return Status;
+}
+
+/** The SetObject function defines the interface implemented by the
+ Configuration Manager Protocol for updating the Configuration
+ Manager Objects.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token An optional token identifying the object. If
+ unused this must be CM_NULL_TOKEN.
+ @param [in] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the Object.
+
+ @retval EFI_UNSUPPORTED This operation is not supported.
+**/
+EFI_STATUS
+EFIAPI
+ArmKvmtoolPlatformSetObject (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
+ IN CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/** A structure describing the configuration manager protocol interface.
+*/
+STATIC
+CONST
+EDKII_CONFIGURATION_MANAGER_PROTOCOL KvmtoolPlatformConfigManagerProtocol = {
+ CREATE_REVISION(1,0),
+ ArmKvmtoolPlatformGetObject,
+ ArmKvmtoolPlatformSetObject,
+ &KvmtoolPlatRepositoryInfo
+};
+
+/**
+ Entrypoint of Configuration Manager Dxe.
+
+ @param ImageHandle
+ @param SystemTable
+
+ @return EFI_SUCCESS
+ @return EFI_LOAD_ERROR
+ @return EFI_OUT_OF_RESOURCES
+**/
+EFI_STATUS
+EFIAPI
+ConfigurationManagerDxeInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ if (PcdGetBool (PcdForceNoAcpi)) {
+ // Use DT and not ACPI.
+ return EFI_SUCCESS;
+ }
+
+ Status = gBS->InstallProtocolInterface (
+ &ImageHandle,
+ &gEdkiiConfigurationManagerProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ (VOID*)&KvmtoolPlatformConfigManagerProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to get Install Configuration Manager Protocol." \
+ " Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ Status = InitializePlatformRepository (
+ &KvmtoolPlatformConfigManagerProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to initialize the Platform Configuration Repository." \
+ " Status = %r\n",
+ Status
+ ));
+ }
+
+error_handler:
+ return Status;
+}
+
+/**
+ Unload function for this image.
+
+ @param ImageHandle Handle for the image of this driver.
+
+ @retval EFI_SUCCESS Driver unloaded successfully.
+ @return other Driver can not unloaded.
+**/
+EFI_STATUS
+EFIAPI
+ConfigurationManagerDxeUnloadImage (
+ IN EFI_HANDLE ImageHandle
+ )
+{
+ return CleanupPlatformRepository (&KvmtoolPlatformConfigManagerProtocol);
+}
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
new file mode 100644
index 000000000000..94cfca3b7671
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
@@ -0,0 +1,94 @@
+/** @file
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#ifndef CONFIGURATION_MANAGER_H_
+#define CONFIGURATION_MANAGER_H_
+
+/** C array containing the compiled AML template.
+ This symbol is defined in the auto generated C file
+ containing the AML bytecode array.
+*/
+extern CHAR8 dsdt_aml_code[];
+
+/** The configuration manager version.
+*/
+#define CONFIGURATION_MANAGER_REVISION CREATE_REVISION (1, 0)
+
+/** The OEM ID
+*/
+#define CFG_MGR_OEM_ID { 'A', 'R', 'M', 'L', 'T', 'D' }
+
+/** A function that prepares Configuration Manager Objects for returning.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Configuration Manager Object ID.
+ @param [in] Token A token for identifying the object.
+ @param [out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN Token,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ );
+
+/** A helper macro for mapping a reference token.
+*/
+#define REFERENCE_TOKEN(Field) \
+ (CM_OBJECT_TOKEN)((UINT8*)&KvmtoolPlatRepositoryInfo + \
+ OFFSET_OF (EDKII_PLATFORM_REPOSITORY_INFO, Field))
+
+/** The number of ACPI tables to install
+*/
+#define PLAT_ACPI_TABLE_COUNT 10
+
+/** A structure describing the platform configuration
+ manager repository information
+*/
+typedef struct PlatformRepositoryInfo {
+ /// Configuration Manager Information.
+ CM_STD_OBJ_CONFIGURATION_MANAGER_INFO CmInfo;
+
+ /// List of ACPI tables
+ CM_STD_OBJ_ACPI_TABLE_INFO CmAcpiTableList[PLAT_ACPI_TABLE_COUNT];
+
+ /// Power management profile information
+ CM_ARM_POWER_MANAGEMENT_PROFILE_INFO PmProfileInfo;
+
+ /// ITS Group node
+ CM_ARM_ITS_GROUP_NODE ItsGroupInfo;
+
+ /// ITS Identifier array
+ CM_ARM_ITS_IDENTIFIER ItsIdentifierArray[1];
+
+ /// PCI Root complex node
+ CM_ARM_ROOT_COMPLEX_NODE RootComplexInfo;
+
+ /// Array of DeviceID mapping
+ CM_ARM_ID_MAPPING DeviceIdMapping[1];
+
+ /// Dynamic platform repository.
+ /// CmObj created by parsing the Kvmtool device tree are stored here.
+ DYNAMIC_PLATFORM_REPOSITORY_INFO * DynamicPlatformRepo;
+
+ /// Base address of the FDT.
+ VOID * FdtBase;
+
+ /// A handle to the FDT HwInfoParser.
+ HW_INFO_PARSER_HANDLE FdtParserHandle;
+} EDKII_PLATFORM_REPOSITORY_INFO;
+
+#endif // CONFIGURATION_MANAGER_H_
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
new file mode 100644
index 000000000000..9f0bf72fce2d
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
@@ -0,0 +1,58 @@
+## @file
+# Configuration Manager Dxe
+#
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = ConfigurationManagerDxe
+ FILE_GUID = 3C80D366-510C-4154-BB3A-E12439AD337C
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = ConfigurationManagerDxeInitialize
+ UNLOAD_IMAGE = ConfigurationManagerDxeUnloadImage
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = ARM AARCH64
+#
+
+[Sources]
+ AslTables/Dsdt.asl
+ ConfigurationManager.c
+ ConfigurationManager.h
+ ConfigurationManagerDxe.inf
+
+[Packages]
+ ArmVirtPkg/ArmVirtPkg.dec
+ DynamicTablesPkg/DynamicTablesPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ DynamicPlatRepoLib
+ HobLib
+ HwInfoParserLib
+ PrintLib
+ TableHelperLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ UefiRuntimeServicesTableLib
+
+[Protocols]
+ gEdkiiConfigurationManagerProtocolGuid
+
+[Guids]
+ gFdtHobGuid
+
+[FixedPcd]
+
+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+
+[Depex]
+ TRUE
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
` (2 preceding siblings ...)
2021-06-23 14:06 ` [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager " PierreGondois
@ 2021-06-23 14:06 ` PierreGondois
2021-06-24 12:50 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg PierreGondois
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Sami Mujawar <sami.mujawar@arm.com>
A Configuration Manager that uses the Dynamic Tables framework
to generate ACPI tables for Kvmtool Guests has been provided.
This Configuration Manager uses the FdtHwInfoParser module to
parse the Kvmtool Device Tree and generate the required
Configuration Manager objects for generating the ACPI tables.
Therefore, enable ACPI table generation for Kvmtool.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
ArmVirtPkg/ArmVirtKvmTool.dsc | 15 +++++++++++++--
ArmVirtPkg/ArmVirtKvmTool.fdf | 11 +++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 920880796ac2..b02324312f18 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -28,6 +28,7 @@ [Defines]
FLASH_DEFINITION = ArmVirtPkg/ArmVirtKvmTool.fdf
!include ArmVirtPkg/ArmVirt.dsc.inc
+!include DynamicTablesPkg/DynamicTables.dsc.inc
!include MdePkg/MdeLibs.dsc.inc
@@ -144,6 +145,11 @@ [PcdsFixedAtBuild.common]
#
gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
+ #
+ # ACPI Table Version
+ #
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
+
[PcdsPatchableInModule.common]
#
# This will be overridden in the code
@@ -198,8 +204,8 @@ [PcdsDynamicDefault.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
- ## Force DTB
- gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
+ ## Set default option to ACPI
+ gArmVirtTokenSpaceGuid.PcdForceNoAcpi|FALSE
# Setup Flash storage variables
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
@@ -356,3 +362,8 @@ [Components.common]
}
OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
OvmfPkg/Virtio10Dxe/Virtio10.inf
+ #
+ # ACPI Support
+ #
+ MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
index 076155199905..5ba4c579f050 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.fdf
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -204,6 +204,17 @@ [FV.FvMain]
INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
INF OvmfPkg/Virtio10Dxe/Virtio10.inf
+ #
+ # ACPI Support
+ #
+ INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ #
+ # Dynamic Table fdf
+ #
+ !include DynamicTablesPkg/DynamicTables.fdf.inc
+
+ INF ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
+
#
# TianoCore logo (splash screen)
#
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
` (3 preceding siblings ...)
2021-06-23 14:06 ` [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool PierreGondois
@ 2021-06-23 14:06 ` PierreGondois
2021-06-24 12:59 ` [edk2-devel] " Laszlo Ersek
2021-06-24 11:51 ` [edk2-devel] [PATCH v1 0/5] Add ACPI support for Kvmtool Laszlo Ersek
2021-06-24 13:02 ` Laszlo Ersek
6 siblings, 1 reply; 15+ messages in thread
From: PierreGondois @ 2021-06-23 14:06 UTC (permalink / raw)
To: devel, Sami Mujawar, Laszlo Ersek
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
From: Sami Mujawar <sami.mujawar@arm.com>
Acpiview is a command line tool allowing to display, dump, or
check installed ACPI tables. Add the tool to ArmVirt platforms.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index d9abadbe708c..269ac4990a6c 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+# Copyright (c) 2011-2021, Arm Limited. All rights reserved.
# Copyright (c) 2014, Linaro Limited. All rights reserved.
# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
# Copyright (c) Microsoft Corporation.
@@ -398,6 +398,7 @@ [Components.common]
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+ NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions
2021-06-23 14:06 ` [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions PierreGondois
@ 2021-06-24 11:11 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 11:11 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> The cpsell tool checks for unknown words in the upstream CI.
> Add some new words to the list of exceptions.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
> index 5f427e57233e..e3f30d69e89a 100644
> --- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
> +++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
> @@ -47,6 +47,7 @@
> "MdePkg/MdePkg.dec",
> "MdeModulePkg/MdeModulePkg.dec",
> "ArmVirtPkg/ArmVirtPkg.dec",
> + "DynamicTablesPkg/DynamicTablesPkg.dec",
> "NetworkPkg/NetworkPkg.dec",
> "ArmPkg/ArmPkg.dec",
> "OvmfPkg/OvmfPkg.dec",
> @@ -97,6 +98,8 @@
> "AuditOnly": False, # Fails right now with over 270 errors
> "IgnoreFiles": [], # use gitignore syntax to ignore errors in matching files
> "ExtendWords": [
> + "armltd",
> + "ssdts",
> "setjump",
> "plong",
> "lparam",
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware
2021-06-23 14:06 ` [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware PierreGondois
@ 2021-06-24 11:13 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 11:13 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Sami Mujawar <sami.mujawar@arm.com>
>
> Most ACPI tables for Kvmtool firmware are dynamically
> generated. The AML code is also generated at runtime
> for most components in appropriate SSDTs.
>
> Although there may not be much to describe in the DSDT,
> the DSDT table is mandatory.
>
> Therefore, add an empty stub for DSDT.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> .../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
>
> diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
> new file mode 100644
> index 000000000000..8467d1ede4ec
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
> @@ -0,0 +1,19 @@
> +/** @file
> + Differentiated System Description Table Fields (DSDT)
> +
> + Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> + Scope (_SB) {
> + // Most ACPI tables for Kvmtool firmware are
> + // dynamically generated. The AML code is also
> + // generated at runtime for most components in
> + // appropriate SSDTs.
> + // Although there may not be much to describe
> + // in the DSDT, the DSDT table is mandatory.
> + // Therefore, add an empty stub for DSDT.
> + } // Scope (_SB)
> +}
>
Please insert empty // lines at the top and bottom of the comment block,
to stick more closely with the edk2 coding style.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/5] Add ACPI support for Kvmtool
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
` (4 preceding siblings ...)
2021-06-23 14:06 ` [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg PierreGondois
@ 2021-06-24 11:51 ` Laszlo Ersek
2021-06-24 13:02 ` Laszlo Ersek
6 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 11:51 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> Kvmtool dynamically generates a device tree describing the platform
> to boot on. Using the patch-sets listed below, the DynamicTables
> framework generates ACPI tables describing a similar platform.
>
> This patch-set:
> - adds a ConfigurationManager allowing to generate ACPI tables
> for Kvmtool
> - adds the acpiview command line utility to the ArmVirtPkg
> - update ArmVirtPkg.ci.yaml to add new words and use the
> DynamicTablesPkg
>
> This patch sets also set the default platform description format
> to ACPI instead of the device tree (c.f.: PcdForceNoAcpi is set
> to FALSE).
>
> The changes can be seen at: https://github.com/PierreARM/edk2/tree/1456_Add_ACPI_support_for_Kvmtool_v1
> The results of the CI can be seen at: https://github.com/tianocore/edk2/pull/1753
>
> This patch-set is dependent over the following patch-sets:
> [PATCH v1 00/10] Various DynamicTablesPkg modifications
> https://edk2.groups.io/g/devel/message/76929
> and:
> [PATCH v1 00/13] Create a SSDT CPU topology generator
> https://edk2.groups.io/g/devel/message/76941
> and:
> [PATCH v1 0/7] Create a SSDT PCIe generator
> https://edk2.groups.io/g/devel/message/76958
> and:
> [PATCH v1 00/14] Implement a FdtHwInfoParserLib
> https://edk2.groups.io/g/devel/message/76967
> and:
> [PATCH v1 0/5] Add DynamicPlatRepoLib
> https://edk2.groups.io/g/devel/message/76984
Not sure if you want just one BZ for all of these subfeatures, or one BZ
per subfeature, but we definitely need at least one BZ for this series.
Please update the commit messages accordingly.
Thanks
Laszlo
>
> Pierre Gondois (1):
> ArmVirtPkg: Add cspell exceptions
>
> Sami Mujawar (4):
> ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware
> ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
> ArmVirtPkg: Enable ACPI support for Kvmtool
> ArmVirtPkg: Enable Acpiview for ArmVirtPkg
>
> ArmVirtPkg/ArmVirt.dsc.inc | 3 +-
> ArmVirtPkg/ArmVirtKvmTool.dsc | 18 +-
> ArmVirtPkg/ArmVirtKvmTool.fdf | 11 +
> ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 +
> .../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl | 19 +
> .../KvmtoolCfgMgrDxe/ConfigurationManager.c | 948 ++++++++++++++++++
> .../KvmtoolCfgMgrDxe/ConfigurationManager.h | 94 ++
> .../ConfigurationManagerDxe.inf | 58 ++
> 8 files changed, 1151 insertions(+), 3 deletions(-)
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
2021-06-23 14:06 ` [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager " PierreGondois
@ 2021-06-24 12:30 ` Laszlo Ersek
2021-06-24 12:35 ` Laszlo Ersek
1 sibling, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 12:30 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Sami Mujawar <sami.mujawar@arm.com>
>
> Add Configuration Manager to enable ACPI tables for Kvmtool
> firmware. The Configuration Manager for Kvmtool uses the DT
> Hardware Information Parser module (FdtHwInfoParser) to parse
> the DT provided by Kvmtool. The FdtHwInfoParser parses the DT
> and invokes the callback function HW_INFO_ADD_OBJECT to add
> the Configuration Manager objects to the Platform Information
> repository.
>
> The information for some Configuration Manager objects may not
> be available in the DT. Such objects are initialised locally
> by the Configuration Manager.
>
> Support for the following ACPI tables is provided:
> - DBG2
> - DSDT (Empty stub)
> - FADT
> - GTDT
> - MADT
> - SPCR
> - SSDT (Cpu Hierarchy)
> - SSDT (Pcie bus)
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> ArmVirtPkg/ArmVirtKvmTool.dsc | 3 +
> .../KvmtoolCfgMgrDxe/ConfigurationManager.c | 948 ++++++++++++++++++
> .../KvmtoolCfgMgrDxe/ConfigurationManager.h | 94 ++
> .../ConfigurationManagerDxe.inf | 58 ++
> 4 files changed, 1103 insertions(+)
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
> create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 3bd1cc72a1eb..920880796ac2 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -71,6 +71,9 @@ [LibraryClasses.common]
> PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>
> + HwInfoParserLib|DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
> + DynamicPlatRepoLib|DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> +
> [LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
> PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
(1) Please move at least the line
ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
from the next patch into this patch.
The reason is that without that line in the DSC file, you can't build
the new driver, right after this patch is applied, even with the "-m"
option of the "build" utility. And if you cannot build the driver, then
adding the library class resolutions *here* is useless.
... Alternatively, please move the lib class resolutions to the next
patch (and don't touch the DSC file in this patch).
> diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
> new file mode 100644
> index 000000000000..07b8b403dd4a
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
> @@ -0,0 +1,948 @@
> +/** @file
> + Configuration Manager Dxe
> +
> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Glossary:
> + - Cm or CM - Configuration Manager
> + - Obj or OBJ - Object
> +**/
> +
> +#include <IndustryStandard/DebugPort2Table.h>
> +#include <IndustryStandard/IoRemappingTable.h>
> +#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DynamicPlatRepoLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/HwInfoParserLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/TableHelperLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/AcpiTable.h>
> +#include <Protocol/ConfigurationManagerProtocol.h>
> +
> +#include "ConfigurationManager.h"
> +
> +/** The platform configuration repository information.
> +*/
(2) This comment style is only used on functions. For other declarations
/ definitions, please use
//
// blah
//
> +STATIC
> +EDKII_PLATFORM_REPOSITORY_INFO KvmtoolPlatRepositoryInfo = {
> + /// Configuration Manager information
(3) "///" is sometimes used in edk2 indeed, but here it is inconsistent
with the other "//" usages
(4) please add leading and trailing empty comment lines (elsewhere too)
(5) all module global variables must start with a lower-case "m". Such
as "mKvmtoolPlatRepositoryInfo".
> + { CONFIGURATION_MANAGER_REVISION, CFG_MGR_OEM_ID },
> +
> + // ACPI Table List
> + {
> + // FADT Table
> + {
> + EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
> + NULL
> + },
> + // GTDT Table
> + {
> + EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdGtdt),
> + NULL
> + },
> + // MADT Table
> + {
> + EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
> + NULL
> + },
> + // SPCR Table
> + {
> + EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> + EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSpcr),
> + NULL
> + },
> + // DSDT Table
> + {
> + EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> + 0, // Unused
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDsdt),
> + (EFI_ACPI_DESCRIPTION_HEADER*)dsdt_aml_code
> + },
> + // SSDT Cpu Hierarchy Table
> + {
> + EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> + 0, // Unused
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtCpuTopology),
> + NULL
> + },
> + // DBG2 Table
> + {
> + EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> + EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDbg2),
> + NULL
> + },
> + // PCI MCFG Table
> + {
> + EFI_ACPI_6_3_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMcfg),
> + NULL
> + },
> + // SSDT table describing the PCI root complex
> + {
> + EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> + 0, // Unused
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtPciExpress),
> + NULL
> + },
> + // IORT Table
> + {
> + EFI_ACPI_6_3_IO_REMAPPING_TABLE_SIGNATURE,
> + EFI_ACPI_IO_REMAPPING_TABLE_REVISION,
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdIort),
> + NULL
> + },
> + },
> +
> + // Power management profile information
> + { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Profile
> +
> + // ITS group node
> + {
> + // Reference token for this Iort node
> + REFERENCE_TOKEN (ItsGroupInfo),
> + // The number of ITS identifiers in the ITS node.
> + 1,
> + // Reference token for the ITS identifier array
> + REFERENCE_TOKEN (ItsIdentifierArray)
> + },
> + // ITS identifier array
> + {
> + {
> + // The ITS Identifier
> + 0
> + }
> + },
> +
> + // Root Complex node info
> + {
> + // Reference token for this Iort node
> + REFERENCE_TOKEN (RootComplexInfo),
> + // Number of ID mappings
> + 1,
> + // Reference token for the ID mapping array
> + REFERENCE_TOKEN (DeviceIdMapping[0]),
> +
> + // Memory access properties : Cache coherent attributes
> + EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
> + // Memory access properties : Allocation hints
> + 0,
> + // Memory access properties : Memory access flags
> + 0,
> + // ATS attributes
> + EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED,
> + // PCI segment number
> + 0
> + },
> +
> + // Array of Device ID mappings
> + {
> + /* RootComplex -> ITS Group
> + */
> + // Device ID mapping for Root complex node
> + {
> + // Input base
> + 0x0,
> + // Number of input IDs
> + 0x0000FFFF,
> + // Output Base
> + 0x0,
> + // Output reference
> + REFERENCE_TOKEN (ItsGroupInfo),
> + // Flags
> + 0
> + },
> + },
> +};
> +
> +/** A helper function for returning the Configuration Manager Objects.
(6) I think we tend to keep the "/**" comment starter alone on the line,
and the actual function description starts at the next line.
I suggest updating all the other comment blocks, too.
> +
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Object Pointer to the Object(s).
> + @param [in] ObjectSize Total size of the Object(s).
> + @param [in] ObjectCount Number of Objects.
> + @param [in, out] CmObjectDesc Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HandleCmObject (
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN VOID * Object,
> + IN CONST UINTN ObjectSize,
> + IN CONST UINTN ObjectCount,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObjectDesc
> + )
> +{
> + CmObjectDesc->ObjectId = CmObjectId;
> + CmObjectDesc->Size = ObjectSize;
> + CmObjectDesc->Data = (VOID*)Object;
(7) This cast is useless. Please update the rest of the code too, if
necessary.
> + CmObjectDesc->Count = ObjectCount;
> + DEBUG ((
> + DEBUG_INFO,
> + "INFO: CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
> + CmObjectId,
> + CmObjectDesc->Data,
> + CmObjectDesc->Size,
> + CmObjectDesc->Count
> + ));
(8) Please audit the format specifications in all of your DEBUGs.
First, wherever you introduce the CM_OBJECT_ID typedef, please also
#define a new format string macro for printing objects of type
CM_OBJECT_ID. Then, in DEBUG format strings, please use that typedef,
rather than an open-coded %x. "%x" may be proper (I can't tell), but it
should be hidden behind a macro.
#define FMT_CM_OBJECT_ID "%x"
DEBUG ((DEBUG_INFO, "blah " FMT_CM_OBJECT_ID " blah\n", CmObjectId));
Second, *assuming* "CmObjectDesc->Size" has type UINTN, the right way to
print it is not with %d, but (a) casting the value to UINT64, (b)
printing the converted value with %Lu. Same for "CmObjectDesc->Count".
> + return EFI_SUCCESS;
> +}
> +
> +/** A helper function for returning the Configuration Manager Objects that
> + match the token.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Object Pointer to the Object(s).
> + @param [in] ObjectSize Total size of the Object(s).
> + @param [in] ObjectCount Number of Objects.
> + @param [in] Token A token identifying the object.
> + @param [in] HandlerProc A handler function to search the object
> + referenced by the token.
> + @param [in, out] CmObjectDesc Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HandleCmObjectRefByToken (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN VOID * Object,
> + IN CONST UINTN ObjectSize,
> + IN CONST UINTN ObjectCount,
> + IN CONST CM_OBJECT_TOKEN Token,
> + IN CONST CM_OBJECT_HANDLER_PROC HandlerProc,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObjectDesc
> + )
> +{
> + EFI_STATUS Status;
> + CmObjectDesc->ObjectId = CmObjectId;
> + if (Token == CM_NULL_TOKEN) {
> + CmObjectDesc->Size = ObjectSize;
> + CmObjectDesc->Data = (VOID*)Object;
> + CmObjectDesc->Count = ObjectCount;
> + Status = EFI_SUCCESS;
> + } else {
> + Status = HandlerProc (This, CmObjectId, Token, CmObjectDesc);
> + }
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "INFO: Token = 0x%p, CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
> + (VOID*)Token,
> + CmObjectId,
> + CmObjectDesc->Data,
> + CmObjectDesc->Size,
> + CmObjectDesc->Count
> + ));
> + return Status;
> +}
> +
> +/** Return an ITS identifier array.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token A token for identifying the object
> + @param [out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetItsIdentifierArray (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token,
> + OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> +
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (0);
(9) should be ASSERT (FALSE) -- please update all instances.
Better yet: in GetStandardNameSpaceObject(), you use ASSERT()
expressions that make a lot more sense, when logged. I suggest replacing
all the ASSERT (0) calls with that (more useful) pattern.
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + PlatformRepo = This->PlatRepoInfo;
> +
> + if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->ItsIdentifierArray) {
> + return EFI_NOT_FOUND;
> + }
> +
> + CmObject->ObjectId = CmObjectId;
> + CmObject->Size = sizeof (PlatformRepo->ItsIdentifierArray);
> + CmObject->Data = (VOID*)&PlatformRepo->ItsIdentifierArray;
> + CmObject->Count = ARRAY_SIZE (PlatformRepo->ItsIdentifierArray);
> + return EFI_SUCCESS;
> +}
> +
> +/** Return a device Id mapping array.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token A token for identifying the object
> + @param [out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetDeviceIdMappingArray (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token,
> + OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> +
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + PlatformRepo = This->PlatRepoInfo;
> +
> + if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->DeviceIdMapping[0]) {
> + return EFI_NOT_FOUND;
> + }
> +
> + CmObject->ObjectId = CmObjectId;
> + CmObject->Size = sizeof (CM_ARM_ID_MAPPING);
> + CmObject->Data = (VOID*)Token;
> + CmObject->Count = 1;
> + return EFI_SUCCESS;
> +}
> +
> +/** Function pointer called by the parser to add information.
> +
> + Callback function that the parser can use to add new
> + CmObj. This function must copy the CmObj data and not rely on
> + the parser preserving the CmObj memory.
> + This function is responsible of the Token allocation.
> +
> + @param [in] ParserHandle A handle to the parser instance.
> + @param [in] Context A pointer to the caller's context provided in
> + HwInfoParserInit ().
> + @param [in] CmObjDesc CM_OBJ_DESCRIPTOR containing the CmObj(s) to add.
> + @param [out] Token If provided and success, contain the token
> + generated for the CmObj.
> +
> + @retval EFI_SUCCESS The function completed successfully.
> + @retval EFI_INVALID_PARAMETER Invalid parameter.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HwInfoAdd (
> + IN HW_INFO_PARSER_HANDLE ParserHandle,
> + IN VOID * Context,
> + IN CONST CM_OBJ_DESCRIPTOR * CmObjDesc,
> + OUT CM_OBJECT_TOKEN * Token OPTIONAL
> + )
> +{
> + EFI_STATUS Status;
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> +
> + if ((ParserHandle == NULL) ||
> + (Context == NULL) ||
> + (CmObjDesc == NULL)) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + PlatformRepo = (EDKII_PLATFORM_REPOSITORY_INFO*)Context;
> +
> +#ifndef MDEPKG_NDEBUG
> + // Print the received objects.
> + ParseCmObjDesc (CmObjDesc);
> +#endif
(10) Please use the DEBUG_CODE() or DEBUG_CODE_BEGIN() /
DEBUG_CODE_END() macros here.
> +
> + Status = DynPlatRepoAddObject (
> + PlatformRepo->DynamicPlatformRepo,
> + CmObjDesc,
> + Token
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + }
(11) Please use the dedicated macro ASSERT_EFI_ERROR().
(There are other instances of the same, please update those as well.)
> + return Status;
> +}
> +
> +/** Cleanup the platform configuration repository.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> +
> + @retval EFI_SUCCESS Success
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +CleanupPlatformRepository (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This
> + )
> +{
> + EFI_STATUS Status;
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> +
> + if (This == NULL) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + PlatformRepo = This->PlatRepoInfo;
> +
> + // Shutdown the dynamic repo and free all objects.
> + Status = DynamicPlatRepoShutdown (PlatformRepo->DynamicPlatformRepo);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // Shutdown parser.
> + Status = HwInfoParserShutdown (PlatformRepo->FdtParserHandle);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + }
> + return Status;
> +}
> +
> +/** Initialize the platform configuration repository.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> +
> + @retval EFI_SUCCESS Success
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_OUT_OF_RESOURCES An allocation has failed.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InitializePlatformRepository (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This
> + )
> +{
> + EFI_STATUS Status;
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> + VOID * Hob;
> +
> + if (This == NULL) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Hob = GetFirstGuidHob (&gFdtHobGuid);
> + if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
> + ASSERT (0);
> + return EFI_NOT_FOUND;
> + }
> +
> + PlatformRepo = This->PlatRepoInfo;
> + PlatformRepo->FdtBase = (VOID *)*(UINTN*)GET_GUID_HOB_DATA (Hob);
> +
> + // Initialise the dynamic platform repository.
> + Status = DynamicPlatRepoInit (&PlatformRepo->DynamicPlatformRepo);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // Initialise the FDT parser
> + Status = HwInfoParserInit (
> + PlatformRepo->FdtBase,
> + PlatformRepo,
> + HwInfoAdd,
> + &PlatformRepo->FdtParserHandle
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + goto error_handler;
(12) This label should be named ErrorHandler or just Error.
(Applies to ConfigurationManagerDxeInitialize() as well.)
> + }
> +
> + Status = HwInfoParse (PlatformRepo->FdtParserHandle);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + goto error_handler;
> + }
> +
> + Status = DynamicPlatRepoFinalise (PlatformRepo->DynamicPlatformRepo);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + goto error_handler;
> + }
> +
> + return EFI_SUCCESS;
> +
> +error_handler:
> + CleanupPlatformRepository (This);
> + return Status;
> +}
> +
> +/** Return a standard namespace object.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token An optional token identifying the object. If
> + unused this must be CM_NULL_TOKEN.
> + @param [in, out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetStandardNameSpaceObject (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EFI_STATUS Status;
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> + UINTN AcpiTableCount;
> + CM_OBJ_DESCRIPTOR CmObjDesc;
> +
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (This != NULL);
> + ASSERT (CmObject != NULL);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Status = EFI_NOT_FOUND;
> + PlatformRepo = This->PlatRepoInfo;
> +
> + switch (GET_CM_OBJECT_ID (CmObjectId)) {
> + case EStdObjCfgMgrInfo:
> + Status = HandleCmObject (
> + CmObjectId,
> + &PlatformRepo->CmInfo,
> + sizeof (PlatformRepo->CmInfo),
> + 1,
> + CmObject
> + );
> + break;
> +
> + case EStdObjAcpiTableList:
> + AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
> +
> + // Get Pci config space information.
> + Status = DynamicPlatRepoGetObject (
> + PlatformRepo->DynamicPlatformRepo,
> + CREATE_CM_ARM_OBJECT_ID (EArmObjPciConfigSpaceInfo),
> + CM_NULL_TOKEN,
> + &CmObjDesc
> + );
> + if (Status == EFI_NOT_FOUND) {
> + // The last 3 tables are for PCIe. If PCIe information is not
> + // present, Kvmtool was launched without the PCIe option.
> + // Therefore, reduce the table count by 3.
> + AcpiTableCount -= 3;
> + } else if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // Get the Gic version.
> + Status = DynamicPlatRepoGetObject (
> + PlatformRepo->DynamicPlatformRepo,
> + CREATE_CM_ARM_OBJECT_ID (EArmObjGicDInfo),
> + CM_NULL_TOKEN,
> + &CmObjDesc
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> + if (((CM_ARM_GICD_INFO*)CmObjDesc.Data)->GicVersion < 3) {
> + // IORT is only required for GicV3/4
> + AcpiTableCount -= 1;
> + }
> +
> + Status = HandleCmObject (
> + CmObjectId,
> + PlatformRepo->CmAcpiTableList,
> + (sizeof (PlatformRepo->CmAcpiTableList[0]) * AcpiTableCount),
> + AcpiTableCount,
> + CmObject
> + );
> + break;
> +
> + default: {
(13) Please drop the brace.
(applies to ArmKvmtoolPlatformGetObject() as well)
> + Status = EFI_NOT_FOUND;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: Object 0x%x. Status = %r\n",
> + CmObjectId,
> + Status
> + ));
> + break;
> + }
> + }
> +
> + return Status;
> +}
> +
> +/** Return an ARM namespace object.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token An optional token identifying the object. If
> + unused this must be CM_NULL_TOKEN.
> + @param [in, out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetArmNameSpaceObject (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EFI_STATUS Status;
> + EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> +
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (This != NULL);
> + ASSERT (CmObject != NULL);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Status = EFI_NOT_FOUND;
> + PlatformRepo = This->PlatRepoInfo;
> +
> + // First check among the static objects.
> + switch (GET_CM_OBJECT_ID (CmObjectId)) {
> + case EArmObjPowerManagementProfileInfo:
> + Status = HandleCmObject (
> + CmObjectId,
> + &PlatformRepo->PmProfileInfo,
> + sizeof (PlatformRepo->PmProfileInfo),
> + 1,
> + CmObject
> + );
> + break;
> +
> + case EArmObjItsGroup:
> + Status = HandleCmObject (
> + CmObjectId,
> + &PlatformRepo->ItsGroupInfo,
> + sizeof (PlatformRepo->ItsGroupInfo),
> + 1,
> + CmObject
> + );
> + break;
> +
> + case EArmObjGicItsIdentifierArray:
> + Status = HandleCmObjectRefByToken (
> + This,
> + CmObjectId,
> + PlatformRepo->ItsIdentifierArray,
> + sizeof (PlatformRepo->ItsIdentifierArray),
> + ARRAY_SIZE (PlatformRepo->ItsIdentifierArray),
> + Token,
> + GetItsIdentifierArray,
> + CmObject
> + );
> + break;
> +
> + case EArmObjRootComplex:
> + Status = HandleCmObject (
> + CmObjectId,
> + &PlatformRepo->RootComplexInfo,
> + sizeof (PlatformRepo->RootComplexInfo),
> + 1,
> + CmObject
> + );
> + break;
> +
> + case EArmObjIdMappingArray:
> + Status = HandleCmObjectRefByToken (
> + This,
> + CmObjectId,
> + PlatformRepo->DeviceIdMapping,
> + sizeof (PlatformRepo->DeviceIdMapping),
> + ARRAY_SIZE (PlatformRepo->DeviceIdMapping),
> + Token,
> + GetDeviceIdMappingArray,
> + CmObject
> + );
> + break;
> +
> + default:
> + // No match found among the static objects.
> + // Check the dynamic objects.
> + Status = DynamicPlatRepoGetObject (
> + PlatformRepo->DynamicPlatformRepo,
> + CmObjectId,
> + Token,
> + CmObject
> + );
> + break;
> + } // switch
> +
> + if (Status == EFI_NOT_FOUND) {
> + DEBUG ((
> + DEBUG_INFO,
> + "INFO: Object 0x%x. Status = %r\n",
> + CmObjectId,
> + Status
> + ));
> + } else {
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> + return Status;
> +}
> +
> +/** Return an OEM namespace object.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token An optional token identifying the object. If
> + unused this must be CM_NULL_TOKEN.
> + @param [in, out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetOemNameSpaceObject (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = EFI_SUCCESS;
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (This != NULL);
> + ASSERT (CmObject != NULL);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + switch (GET_CM_OBJECT_ID (CmObjectId)) {
> + default: {
> + Status = EFI_NOT_FOUND;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: Object 0x%x. Status = %r\n",
> + CmObjectId,
> + Status
> + ));
> + break;
> + }
> + }
> +
> + return Status;
> +}
> +
> +/** The GetObject function defines the interface implemented by the
> + Configuration Manager Protocol for returning the Configuration
> + Manager Objects.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token An optional token identifying the object. If
> + unused this must be CM_NULL_TOKEN.
> + @param [in, out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmKvmtoolPlatformGetObject (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + EFI_STATUS Status;
> +
> + if ((This == NULL) || (CmObject == NULL)) {
> + ASSERT (This != NULL);
> + ASSERT (CmObject != NULL);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + switch (GET_CM_NAMESPACE_ID (CmObjectId)) {
> + case EObjNameSpaceStandard:
> + Status = GetStandardNameSpaceObject (This, CmObjectId, Token, CmObject);
> + break;
> + case EObjNameSpaceArm:
> + Status = GetArmNameSpaceObject (This, CmObjectId, Token, CmObject);
> + break;
> + case EObjNameSpaceOem:
> + Status = GetOemNameSpaceObject (This, CmObjectId, Token, CmObject);
> + break;
> + default: {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: Unknown Namespace Object = 0x%x. Status = %r\n",
> + CmObjectId,
> + Status
> + ));
> + break;
> + }
> + }
> +
> + return Status;
> +}
> +
> +/** The SetObject function defines the interface implemented by the
> + Configuration Manager Protocol for updating the Configuration
> + Manager Objects.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token An optional token identifying the object. If
> + unused this must be CM_NULL_TOKEN.
> + @param [in] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the Object.
> +
> + @retval EFI_UNSUPPORTED This operation is not supported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmKvmtoolPlatformSetObject (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token OPTIONAL,
> + IN CM_OBJ_DESCRIPTOR * CONST CmObject
> + )
> +{
> + return EFI_UNSUPPORTED;
> +}
> +
> +/** A structure describing the configuration manager protocol interface.
> +*/
> +STATIC
> +CONST
> +EDKII_CONFIGURATION_MANAGER_PROTOCOL KvmtoolPlatformConfigManagerProtocol = {
> + CREATE_REVISION(1,0),
> + ArmKvmtoolPlatformGetObject,
> + ArmKvmtoolPlatformSetObject,
> + &KvmtoolPlatRepositoryInfo
> +};
> +
> +/**
> + Entrypoint of Configuration Manager Dxe.
> +
> + @param ImageHandle
> + @param SystemTable
> +
> + @return EFI_SUCCESS
> + @return EFI_LOAD_ERROR
> + @return EFI_OUT_OF_RESOURCES
(14) These should be @retval, not @return.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ConfigurationManagerDxeInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE * SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + if (PcdGetBool (PcdForceNoAcpi)) {
> + // Use DT and not ACPI.
> + return EFI_SUCCESS;
> + }
(15) This is wrong.
KvmtoolPlatformDxe already (correctly) installs either
"gEdkiiPlatformHasAcpiGuid" or "gEdkiiPlatformHasDeviceTreeGuid".
"PcdForceNoAcpi" must not be used outside of that driver.
In this driver, the DEPEX section of the INF file should list
"gEdkiiPlatformHasAcpiGuid". If ACPI is disabled, this driver should not
be dispatched at all.
> +
> + Status = gBS->InstallProtocolInterface (
> + &ImageHandle,
> + &gEdkiiConfigurationManagerProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + (VOID*)&KvmtoolPlatformConfigManagerProtocol
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: Failed to get Install Configuration Manager Protocol." \
> + " Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + Status = InitializePlatformRepository (
> + &KvmtoolPlatformConfigManagerProtocol
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: Failed to initialize the Platform Configuration Repository." \
> + " Status = %r\n",
> + Status
> + ));
> + }
> +
> +error_handler:
> + return Status;
> +}
(16) I've paid basically zero attention to your error handlers, but this
is so obviously wrong that it catches my eye. If
InitializePlatformRepository() fails and so you exit with an error, the
driver will be unloaded, and your installed protocol interface will be a
dangling one.
> +
> +/**
> + Unload function for this image.
> +
> + @param ImageHandle Handle for the image of this driver.
> +
> + @retval EFI_SUCCESS Driver unloaded successfully.
> + @return other Driver can not unloaded.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ConfigurationManagerDxeUnloadImage (
> + IN EFI_HANDLE ImageHandle
> + )
> +{
> + return CleanupPlatformRepository (&KvmtoolPlatformConfigManagerProtocol);
> +}
> diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
> new file mode 100644
> index 000000000000..94cfca3b7671
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
> @@ -0,0 +1,94 @@
> +/** @file
> +
> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Glossary:
> + - Cm or CM - Configuration Manager
> + - Obj or OBJ - Object
> +**/
> +
> +#ifndef CONFIGURATION_MANAGER_H_
> +#define CONFIGURATION_MANAGER_H_
> +
> +/** C array containing the compiled AML template.
> + This symbol is defined in the auto generated C file
> + containing the AML bytecode array.
> +*/
(17) Please apply all the comment observations I made for the C file to
the header file.
> +extern CHAR8 dsdt_aml_code[];
> +
> +/** The configuration manager version.
> +*/
> +#define CONFIGURATION_MANAGER_REVISION CREATE_REVISION (1, 0)
> +
> +/** The OEM ID
> +*/
> +#define CFG_MGR_OEM_ID { 'A', 'R', 'M', 'L', 'T', 'D' }
> +
> +/** A function that prepares Configuration Manager Objects for returning.
> +
> + @param [in] This Pointer to the Configuration Manager Protocol.
> + @param [in] CmObjectId The Configuration Manager Object ID.
> + @param [in] Token A token for identifying the object.
> + @param [out] CmObject Pointer to the Configuration Manager Object
> + descriptor describing the requested Object.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object information is not found.
> +**/
> +typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
> + IN CONST CM_OBJECT_ID CmObjectId,
> + IN CONST CM_OBJECT_TOKEN Token,
> + IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
> + );
> +
> +/** A helper macro for mapping a reference token.
> +*/
> +#define REFERENCE_TOKEN(Field) \
> + (CM_OBJECT_TOKEN)((UINT8*)&KvmtoolPlatRepositoryInfo + \
> + OFFSET_OF (EDKII_PLATFORM_REPOSITORY_INFO, Field))
> +
> +/** The number of ACPI tables to install
> +*/
> +#define PLAT_ACPI_TABLE_COUNT 10
> +
> +/** A structure describing the platform configuration
> + manager repository information
> +*/
> +typedef struct PlatformRepositoryInfo {
(18) the "PlatformRepositoryInfo" struct tag is not spelled
idiomatically. On the other hand, it's also not used anywhere. I suggest
simply dropping it.
> + /// Configuration Manager Information.
> + CM_STD_OBJ_CONFIGURATION_MANAGER_INFO CmInfo;
> +
> + /// List of ACPI tables
> + CM_STD_OBJ_ACPI_TABLE_INFO CmAcpiTableList[PLAT_ACPI_TABLE_COUNT];
> +
> + /// Power management profile information
> + CM_ARM_POWER_MANAGEMENT_PROFILE_INFO PmProfileInfo;
> +
> + /// ITS Group node
> + CM_ARM_ITS_GROUP_NODE ItsGroupInfo;
> +
> + /// ITS Identifier array
> + CM_ARM_ITS_IDENTIFIER ItsIdentifierArray[1];
> +
> + /// PCI Root complex node
> + CM_ARM_ROOT_COMPLEX_NODE RootComplexInfo;
> +
> + /// Array of DeviceID mapping
> + CM_ARM_ID_MAPPING DeviceIdMapping[1];
> +
> + /// Dynamic platform repository.
> + /// CmObj created by parsing the Kvmtool device tree are stored here.
> + DYNAMIC_PLATFORM_REPOSITORY_INFO * DynamicPlatformRepo;
> +
> + /// Base address of the FDT.
> + VOID * FdtBase;
> +
> + /// A handle to the FDT HwInfoParser.
> + HW_INFO_PARSER_HANDLE FdtParserHandle;
> +} EDKII_PLATFORM_REPOSITORY_INFO;
> +
> +#endif // CONFIGURATION_MANAGER_H_
> diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
> new file mode 100644
> index 000000000000..9f0bf72fce2d
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
> @@ -0,0 +1,58 @@
> +## @file
> +# Configuration Manager Dxe
> +#
> +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001B
> + BASE_NAME = ConfigurationManagerDxe
> + FILE_GUID = 3C80D366-510C-4154-BB3A-E12439AD337C
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = ConfigurationManagerDxeInitialize
> + UNLOAD_IMAGE = ConfigurationManagerDxeUnloadImage
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = ARM AARCH64
> +#
> +
> +[Sources]
> + AslTables/Dsdt.asl
> + ConfigurationManager.c
> + ConfigurationManager.h
> + ConfigurationManagerDxe.inf
> +
> +[Packages]
> + ArmVirtPkg/ArmVirtPkg.dec
> + DynamicTablesPkg/DynamicTablesPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + DynamicPlatRepoLib
> + HobLib
> + HwInfoParserLib
> + PrintLib
> + TableHelperLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> + UefiRuntimeServicesTableLib
> +
> +[Protocols]
> + gEdkiiConfigurationManagerProtocolGuid
> +
> +[Guids]
> + gFdtHobGuid
> +
> +[FixedPcd]
(19) please drop this empty section.
> +
> +[Pcd]
> + gArmVirtTokenSpaceGuid.PcdForceNoAcpi
> +
> +[Depex]
> + TRUE
>
I can't see myself reviewing this again (even at this level), so I'll
trust you on all the updates.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
2021-06-23 14:06 ` [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager " PierreGondois
2021-06-24 12:30 ` [edk2-devel] " Laszlo Ersek
@ 2021-06-24 12:35 ` Laszlo Ersek
1 sibling, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 12:35 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
> new file mode 100644
> index 000000000000..9f0bf72fce2d
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = ARM AARCH64
> +#
(20) ACPI is undefined for ARM, therefore VALID_ARCHITECTURES should
only list AARCH64.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool
2021-06-23 14:06 ` [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool PierreGondois
@ 2021-06-24 12:50 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 12:50 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Sami Mujawar <sami.mujawar@arm.com>
>
> A Configuration Manager that uses the Dynamic Tables framework
> to generate ACPI tables for Kvmtool Guests has been provided.
> This Configuration Manager uses the FdtHwInfoParser module to
> parse the Kvmtool Device Tree and generate the required
> Configuration Manager objects for generating the ACPI tables.
>
> Therefore, enable ACPI table generation for Kvmtool.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> ArmVirtPkg/ArmVirtKvmTool.dsc | 15 +++++++++++++--
> ArmVirtPkg/ArmVirtKvmTool.fdf | 11 +++++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 920880796ac2..b02324312f18 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -28,6 +28,7 @@ [Defines]
> FLASH_DEFINITION = ArmVirtPkg/ArmVirtKvmTool.fdf
>
> !include ArmVirtPkg/ArmVirt.dsc.inc
> +!include DynamicTablesPkg/DynamicTables.dsc.inc
(1) This doesn't seem right. In fact, ARM (not AARCH64) support claimed in "DynamicTablesPkg/DynamicTablesPkg.dsc" seems bogus in the first place; to my understanding, ACPI is not defined for 32-bit ARM.
More precisely, this !include directive is OK, but "DynamicTablesPkg/DynamicTables.dsc.inc" file should not provide a
[Components.common]
section, but a
[Components.AARCH64]
section. Refer to "ArmVirtPkg/ArmVirt.dsc.inc" please:
[Components.AARCH64]
#
# ACPI Support
#
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf {
<LibraryClasses>
NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
}
>
> !include MdePkg/MdeLibs.dsc.inc
>
> @@ -144,6 +145,11 @@ [PcdsFixedAtBuild.common]
> #
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>
> + #
> + # ACPI Table Version
> + #
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
> +
> [PcdsPatchableInModule.common]
> #
> # This will be overridden in the code
(2) This hunk is superfluous. Please refer to "MdeModulePkg/MdeModulePkg.dec":
[PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20|UINT32|0x0001004c
If you simply don't list the PCD in your DSC file, you'll get the default value, and the most restrictive access method declared for the PCD in the DEC file (here: fixed-at-build).
> @@ -198,8 +204,8 @@ [PcdsDynamicDefault.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
>
> - ## Force DTB
> - gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
> + ## Set default option to ACPI
> + gArmVirtTokenSpaceGuid.PcdForceNoAcpi|FALSE
>
> # Setup Flash storage variables
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
(3) Same story as (2), please refer to "ArmVirtPkg/ArmVirtPkg.dec":
[PcdsDynamic]
#
# Whether to force disable ACPI, regardless of the fw_cfg settings
# exposed by QEMU
#
gArmVirtTokenSpaceGuid.PcdForceNoAcpi|0x0|BOOLEAN|0x00000003
> @@ -356,3 +362,8 @@ [Components.common]
> }
> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
> OvmfPkg/Virtio10Dxe/Virtio10.inf
> + #
> + # ACPI Support
> + #
> + MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
(4) Superfluous by virtue of including "ArmVirtPkg/ArmVirt.dsc.inc" already.
> + ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
(5) This should be in a [Components.AARCH64] section, not in [Components.common].
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
> index 076155199905..5ba4c579f050 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.fdf
> +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
> @@ -204,6 +204,17 @@ [FV.FvMain]
> INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
> INF OvmfPkg/Virtio10Dxe/Virtio10.inf
>
> + #
> + # ACPI Support
> + #
> + INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> + #
> + # Dynamic Table fdf
> + #
> + !include DynamicTablesPkg/DynamicTables.fdf.inc
> +
> + INF ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
> +
> #
> # TianoCore logo (splash screen)
> #
>
(6) Please do what "ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc" does:
!if $(ARCH) == AARCH64
INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
...
!endif
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
2021-06-23 14:06 ` [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg PierreGondois
@ 2021-06-24 12:59 ` Laszlo Ersek
2021-06-24 13:07 ` Laszlo Ersek
0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 12:59 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> From: Sami Mujawar <sami.mujawar@arm.com>
>
> Acpiview is a command line tool allowing to display, dump, or
> check installed ACPI tables. Add the tool to ArmVirt platforms.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index d9abadbe708c..269ac4990a6c 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -1,5 +1,5 @@
> #
> -# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2021, Arm Limited. All rights reserved.
> # Copyright (c) 2014, Linaro Limited. All rights reserved.
> # Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> # Copyright (c) Microsoft Corporation.
> @@ -398,6 +398,7 @@ [Components.common]
> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> + NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> !if $(NETWORK_IP6_ENABLE) == TRUE
>
I disagree with this patch, as it will cause the Shell binary in all
ArmVirtPkg platforms to include the (rather large) ACPIVIEW command.
ACPIVIEW is super useful for when the tables are (dynamically) generated
by the firmware itself, but that does not apply to the Qemu and Xen
platforms.
Note NETWORK_IP6_ENABLE: UefiShellNetwork2CommandsLib is only hooked
into the shell application if NETWORK_IP6_ENABLE is TRUE.
Please add
DEFINE ACPIVIEW_ENABLE = TRUE
to "ArmVirtPkg/ArmVirtKvmTool.dsc", and in "ArmVirtPkg/ArmVirt.dsc.inc",
include the new command lib conditionally on ACPIVIEW_ENABLE being TRUE.
(Can be in the same patch.)
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/5] Add ACPI support for Kvmtool
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
` (5 preceding siblings ...)
2021-06-24 11:51 ` [edk2-devel] [PATCH v1 0/5] Add ACPI support for Kvmtool Laszlo Ersek
@ 2021-06-24 13:02 ` Laszlo Ersek
6 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 13:02 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/23/21 16:06, PierreGondois wrote:
> Pierre Gondois (1):
> ArmVirtPkg: Add cspell exceptions
>
> Sami Mujawar (4):
> ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware
> ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
> ArmVirtPkg: Enable ACPI support for Kvmtool
> ArmVirtPkg: Enable Acpiview for ArmVirtPkg
The subject lines of Sami's 4 patches should be updated as follows:
ArmVirtPkg/Kvmtool: Add DSDT ACPI table
ArmVirtPkg/Kvmtool: Add Configuration Manager
ArmVirtPkg/Kvmtool: Enable ACPI support
ArmVirtPkg/Kvmtool: Enable Acpiview
(regarding the last patch, I requested in its subthread that ACPIVIEW be
restricted to kvmtool please.)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
2021-06-24 12:59 ` [edk2-devel] " Laszlo Ersek
@ 2021-06-24 13:07 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-06-24 13:07 UTC (permalink / raw)
To: devel, pierre.gondois, Sami Mujawar
Cc: Ard Biesheuvel, Leif Lindholm, Akanksha Jain, Alexandru Elisei
On 06/24/21 14:59, Laszlo Ersek wrote:
> On 06/23/21 16:06, PierreGondois wrote:
>> From: Sami Mujawar <sami.mujawar@arm.com>
>>
>> Acpiview is a command line tool allowing to display, dump, or
>> check installed ACPI tables. Add the tool to ArmVirt platforms.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
>> ---
>> ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index d9abadbe708c..269ac4990a6c 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -1,5 +1,5 @@
>> #
>> -# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
>> +# Copyright (c) 2011-2021, Arm Limited. All rights reserved.
>> # Copyright (c) 2014, Linaro Limited. All rights reserved.
>> # Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
>> # Copyright (c) Microsoft Corporation.
>> @@ -398,6 +398,7 @@ [Components.common]
>> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>> + NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
>> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>> !if $(NETWORK_IP6_ENABLE) == TRUE
>>
>
> I disagree with this patch, as it will cause the Shell binary in all
> ArmVirtPkg platforms to include the (rather large) ACPIVIEW command.
>
> ACPIVIEW is super useful for when the tables are (dynamically) generated
> by the firmware itself, but that does not apply to the Qemu and Xen
> platforms.
>
> Note NETWORK_IP6_ENABLE: UefiShellNetwork2CommandsLib is only hooked
> into the shell application if NETWORK_IP6_ENABLE is TRUE.
>
> Please add
>
> DEFINE ACPIVIEW_ENABLE = TRUE
>
> to "ArmVirtPkg/ArmVirtKvmTool.dsc",
To clarify: please place
DEFINE ACPIVIEW_ENABLE = TRUE
in a new [Defines.AARCH64] section in "ArmVirtPkg/ArmVirtKvmTool.dsc",
not in the existent [Defines] section.
This should happen just before !including "ArmVirtPkg/ArmVirt.dsc.inc".
Thanks
Laszlo
> and in "ArmVirtPkg/ArmVirt.dsc.inc",
> include the new command lib conditionally on ACPIVIEW_ENABLE being TRUE.
> (Can be in the same patch.)
>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
>
>
> Thanks
> Laszlo
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-06-24 13:07 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-23 14:06 [PATCH v1 0/5] Add ACPI support for Kvmtool PierreGondois
2021-06-23 14:06 ` [PATCH v1 1/5] ArmVirtPkg: Add cspell exceptions PierreGondois
2021-06-24 11:11 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 2/5] ArmVirtPkg: Add DSDT ACPI table for Kvmtool firmware PierreGondois
2021-06-24 11:13 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager " PierreGondois
2021-06-24 12:30 ` [edk2-devel] " Laszlo Ersek
2021-06-24 12:35 ` Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 4/5] ArmVirtPkg: Enable ACPI support for Kvmtool PierreGondois
2021-06-24 12:50 ` [edk2-devel] " Laszlo Ersek
2021-06-23 14:06 ` [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg PierreGondois
2021-06-24 12:59 ` [edk2-devel] " Laszlo Ersek
2021-06-24 13:07 ` Laszlo Ersek
2021-06-24 11:51 ` [edk2-devel] [PATCH v1 0/5] Add ACPI support for Kvmtool Laszlo Ersek
2021-06-24 13:02 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox