public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, pierre.gondois@arm.com,
	Sami Mujawar <sami.mujawar@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Leif Lindholm <leif@nuviainc.com>,
	Akanksha Jain <akanksha.jain2@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 3/5] ArmVirtPkg: Add Configuration Manager for Kvmtool firmware
Date: Thu, 24 Jun 2021 14:30:47 +0200	[thread overview]
Message-ID: <14795d6e-a784-5aaf-e697-e60464f0dc5e@redhat.com> (raw)
In-Reply-To: <20210623140640.16754-4-Pierre.Gondois@arm.com>

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


  reply	other threads:[~2021-06-24 12:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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   ` Laszlo Ersek [this message]
2021-06-24 12:35   ` [edk2-devel] " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14795d6e-a784-5aaf-e697-e60464f0dc5e@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox