public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Pierre.Gondois@arm.com, devel@edk2.groups.io,
	Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Akanksha Jain <akanksha.jain2@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>, nd <nd@arm.com>
Subject: Re: [PATCH v1 5/5] DynamicTablesPkg: Add DynamicPlatRepo library
Date: Fri, 5 Nov 2021 14:10:13 +0000	[thread overview]
Message-ID: <4164d6f0-a81e-0905-b479-0d65d0c722e3@arm.com> (raw)
In-Reply-To: <20210623133613.29600-6-Pierre.Gondois@arm.com>

[-- Attachment #1: Type: text/plain, Size: 23028 bytes --]

Hi Pierre,

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar

On 23/06/2021 02:36 PM, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> The DynamicPlatRepo library allows to handle dynamically created
> CmObj. The dynamic platform repository can be in the following states:
> 1 - Non-initialised
> 2 - Transient:
>      Possibility to add CmObj to the platform, but not to query them.
> 3 - Finalised:
>      Possibility to query CmObj, but not to add new.
>
> A token is allocated to each CmObj added to the dynamic platform
> repository (except for reference tokens CmObj). This allows to retrieve
> dynamic CmObjs among all CmObj (static CmObj for instance).
>
> This patch add the inf file of the module and the main module
> functionnalities and update the dsc file of the package.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>   DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
>   .../Include/Library/DynamicPlatRepoLib.h      |   4 +-
>   .../DynamicPlatRepoLib/DynamicPlatRepo.c      | 518 ++++++++++++++++++
>   .../DynamicPlatRepoInternal.h                 |  78 +++
>   .../DynamicPlatRepoLib/DynamicPlatRepoLib.inf |  33 ++
>   5 files changed, 633 insertions(+), 1 deletion(-)
>   create mode 100644 DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepo.c
>   create mode 100644 DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoInternal.h
>   create mode 100644 DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
>
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
> index fd7345891cf1..432c958cf8b2 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
> @@ -43,6 +43,7 @@ [Components.common]
>     DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>     DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>     DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
> +  DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
>   
>   [BuildOptions]
>     *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> diff --git a/DynamicTablesPkg/Include/Library/DynamicPlatRepoLib.h b/DynamicTablesPkg/Include/Library/DynamicPlatRepoLib.h
> index 8e4665bf5c59..4a76b30399b4 100644
> --- a/DynamicTablesPkg/Include/Library/DynamicPlatRepoLib.h
> +++ b/DynamicTablesPkg/Include/Library/DynamicPlatRepoLib.h
> @@ -13,6 +13,8 @@
>   #ifndef DYNAMIC_PLAT_REPO_H_
>   #define DYNAMIC_PLAT_REPO_H_
>   
> +#include <Protocol/ConfigurationManagerProtocol.h>
> +
[SAMI] I think the changes in this file can be merged with patch 1/5.
>   /** A structure describing the platform configuration
>       manager repository information
>   */
> @@ -109,4 +111,4 @@ DynamicPlatRepoShutdown (
>     IN  DYNAMIC_PLATFORM_REPOSITORY_INFO * DynPlatRepo
>     );
>   
> -#endif DYNAMIC_PLAT_REPO_H_
> +#endif // DYNAMIC_PLAT_REPO_H_
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepo.c b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepo.c
> new file mode 100644
> index 000000000000..f26f8ad32bc8
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepo.c
> @@ -0,0 +1,518 @@
> +/** @file
> +  Dynamic Platform Info Repository
> +
> +  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 <Protocol/ConfigurationManagerProtocol.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#include "CmObjectTokenFixer.h"
> +#include "DynamicPlatRepoInternal.h"
> +#include "TokenGenerator.h"
> +
> +/** Allocate a CM_OBJ_NODE.
> +
> +  @param [in]  CmObjDesc  CmObj to wrap in a node.
> +                          All the fields of the CmObj (Data field included),
> +                          are copied.
> +  @param [in]  Token      Token to assign to this CmObj/node.
> +  @param [out] ObjNode    Allocated ObjNode.
> +
> +  @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
> +AllocCmObjNode (
> +  IN  CONST CM_OBJ_DESCRIPTOR   * CmObjDesc,
> +  IN        CM_OBJECT_TOKEN       Token,
> +  OUT       CM_OBJ_NODE        ** ObjNode
> +  )
> +{
> +  CM_OBJ_NODE       *Node;
> +  CM_OBJ_DESCRIPTOR *Desc;
> +
> +  if ((CmObjDesc == NULL) || (ObjNode == NULL)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Node = AllocateZeroPool (sizeof (CM_OBJ_NODE));
> +  if (Node == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  // Initialise the list head.
> +  InitializeListHead (&Node->Link);
> +  Node->Token = Token;
> +  Desc = &Node->CmObjDesc;
> +  Desc->ObjectId = CmObjDesc->ObjectId;
> +  Desc->Size = CmObjDesc->Size;
> +  Desc->Count = CmObjDesc->Count;
> +
> +  // Allocate and copy the CmObject Data.
> +  Desc->Data = AllocateCopyPool (CmObjDesc->Size, CmObjDesc->Data);
> +  if (Desc->Data == NULL) {
> +    FreePool (Node);
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  *ObjNode = Node;
> +  return EFI_SUCCESS;
> +}
> +
> +/** Free a CM_OBJ_NODE.
> +
> +  @param [in]  ObjNode    ObjNode to free.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +FreeCmObjNode (
> +  IN  CM_OBJ_NODE   * ObjNode
> +  )
> +{
> +  CM_OBJ_DESCRIPTOR   *Desc;
> +
> +  if (ObjNode == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Unlink Node
> +  RemoveEntryList (&ObjNode->Link);
> +
> +  Desc = &ObjNode->CmObjDesc;
> +  if (Desc->Data != NULL) {
> +    FreePool (Desc->Data);
> +  }
> +
> +  FreePool (ObjNode);
> +  return EFI_SUCCESS;
> +}
> +
> +/** Add an object to the dynamic platform repository.
> +
> +  @param [in]  This       This dynamic platform repository.
> +  @param [in]  CmObjDesc  CmObj to add. The data is copied.
> +  @param [out] Token      If not NULL, token allocated to this CmObj.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_OUT_OF_RESOURCES  An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DynPlatRepoAddObject (
> +  IN        DYNAMIC_PLATFORM_REPOSITORY_INFO    * This,
> +  IN  CONST CM_OBJ_DESCRIPTOR                   * CmObjDesc,
> +  OUT       CM_OBJECT_TOKEN                     * Token OPTIONAL
> +  )
> +{
> +  EFI_STATUS              Status;
> +  CM_OBJ_NODE             *ObjNode;
> +  CM_OBJECT_ID            ArmNamespaceObjId;
> +  CM_OBJECT_TOKEN         NewToken;
> +
> +  // The dynamic repository must be able to receive objects.
> +  if ((This == NULL)      ||
> +      (CmObjDesc == NULL) ||
> +      (This->RepoState != DynRepoTransient)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Check the CmObjDesc:
> +  //  - only Arm objects are supported for now.
> +  //  - only EArmObjCmRef objects can be added as arrays.
> +  ArmNamespaceObjId = GET_CM_OBJECT_ID (CmObjDesc->ObjectId);
> +  if ((CmObjDesc->Size == 0)              ||
> +      (CmObjDesc->Count == 0)             ||
> +      (ArmNamespaceObjId >= EArmObjMax)   ||
> +      ((CmObjDesc->Count > 1)  && (ArmNamespaceObjId != EArmObjCmRef))  ||
> +      (GET_CM_NAMESPACE_ID (CmObjDesc->ObjectId) != EObjNameSpaceArm)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Generate a token.
> +  NewToken = GenerateToken ();
> +
> +  // Create an ObjNode.
> +  Status = AllocCmObjNode (CmObjDesc, NewToken, &ObjNode);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // Fixup self-token if necessary.
> +  Status = FixupCmObjectSelfToken (&ObjNode->CmObjDesc, NewToken);
> +  if (EFI_ERROR (Status)) {
> +    FreeCmObjNode (ObjNode);
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // Add to link list.
> +  InsertTailList (&This->ArmCmObjList[ArmNamespaceObjId], &ObjNode->Link);
> +  This->ObjectCount += 1;
> +
> +  if (Token != NULL) {
> +    *Token = NewToken;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +/** Group lists of CmObjNode from the ArmNameSpace to one array.
> +
> +  @param [in]  This         This dynamic platform repository.
> +  @param [in]  ArmObjIndex  Index in EARM_OBJECT_ID
> +                            (must be < EArmObjMax).
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_BUFFER_TOO_SMALL    Buffer too small.
> +  @retval EFI_OUT_OF_RESOURCES  An allocation has failed.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GroupCmObjNodes (
> +  IN  DYNAMIC_PLATFORM_REPOSITORY_INFO    * This,
> +  IN  UINT32                                ArmObjIndex
> +  )
> +{
> +  EFI_STATUS          Status;
> +  UINTN               Count;
> +  UINTN               Size;
> +  UINT32              CmObjId;
> +  UINT8               *GroupedData;
> +  UINT8               *Data;
> +  CM_OBJ_DESCRIPTOR   *CmObjDesc;
> +  LIST_ENTRY          *ListHead;
> +  LIST_ENTRY          *Link;
> +
> +  if ((This == NULL)  ||
> +      (ArmObjIndex >= EArmObjMax)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Count = 0;
> +  Size = 0;
> +  CmObjId = CREATE_CM_ARM_OBJECT_ID (ArmObjIndex);
> +  ListHead = &This->ArmCmObjList[ArmObjIndex];
> +  Link = GetFirstNode (ListHead);
> +
> +  // Compute the total count and size of the CmObj in the list.
> +  while (Link != ListHead) {
> +    CmObjDesc = &((CM_OBJ_NODE*)Link)->CmObjDesc;
> +
> +    if (CmObjDesc->ObjectId != CmObjId) {
> +      ASSERT (0);
> +      return EFI_INVALID_PARAMETER;
> +    }
> +
> +    if ((CmObjDesc->Count != 1) && (ArmObjIndex != EArmObjCmRef)){
> +      // We expect each descriptor to contain an individual object.
> +      // EArmObjCmRef objects are counted as groups, so +1 as well.
> +      ASSERT (0);
> +      return EFI_INVALID_PARAMETER;
> +    }
> +
> +    Count++;
> +    Size += CmObjDesc->Size;
> +
> +    // Next Link
> +    Link = GetNextNode (ListHead, Link);
> +  } // while
> +
> +  if (Count == 0) {
> +    // No objects found.
> +    return EFI_SUCCESS;
> +  }
> +
> +  GroupedData = AllocateZeroPool (Size);
> +  if (GroupedData == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  // Copy the Object Data and add to the TokenMapper.
> +  Data = GroupedData;
> +  Link = GetFirstNode (ListHead);
> +  while (Link != ListHead) {
> +    CmObjDesc = &((CM_OBJ_NODE*)Link)->CmObjDesc;
> +    CopyMem (Data, CmObjDesc->Data, CmObjDesc->Size);
> +
> +    // Add the object to the Token Mapper.
> +    // Note: The CmObject Data field of objects in the Token Mapper point
> +    //       to the memory in the GroupedData array.
> +    Status = TokenMapperAddObject (
> +               &This->TokenMapper,
> +               ((CM_OBJ_NODE*)Link)->Token,
> +               CmObjDesc->ObjectId,
> +               CmObjDesc->Size,
> +               Data
> +               );
> +    if (EFI_ERROR (Status)) {
> +      FreePool (GroupedData);
> +      return Status;
> +    }
> +
> +    Data += CmObjDesc->Size;
> +    Link = GetNextNode (ListHead, Link);
> +  } // while
> +
> +  CmObjDesc = &This->ArmCmObjArray[ArmObjIndex];
> +  CmObjDesc->ObjectId = CmObjId;
> +  CmObjDesc->Size = Size;
> +  CmObjDesc->Count = Count;
> +  CmObjDesc->Data = GroupedData;
> +
> +  return Status;
> +}
> +
> +/** Finalise the dynamic repository.
> +
> +  Finalising means:
> +   - Preventing any further objects from being added.
> +   - Allowing to get objects from the dynamic repository
> +     (not possible before a call to this function).
> +
> +  @param [in]  This       This dynamic platform repository.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_ALREADY_STARTED   Instance already initialised.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_BUFFER_TOO_SMALL  Buffer too small.
> +  @retval EFI_OUT_OF_RESOURCES  An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DynamicPlatRepoFinalise (
> +  IN  DYNAMIC_PLATFORM_REPOSITORY_INFO      * This
> +  )
> +{
> +  EFI_STATUS    Status;
> +  UINTN         ArmObjIndex;
> +
> +  if ((This == NULL)  ||
> +      (This->RepoState != DynRepoTransient)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Prevent any further objects from being added.
> +  This->RepoState = DynRepoFinalized;
> +
> +  // Initialise the token mapper.
> +  Status = TokenMapperInitialise (&This->TokenMapper, This->ObjectCount);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // For each CM_OBJECT_ID:
> +  //  - Convert the list of nodes to an array
> +  //    (the array is wrapped in a CmObjDesc).
> +  //  - Add the Token/CmObj binding to the token mapper.
> +  for (ArmObjIndex = 0; ArmObjIndex < EArmObjMax; ArmObjIndex++) {
> +      Status = GroupCmObjNodes (This, ArmObjIndex);
> +      if (EFI_ERROR (Status)) {
> +        ASSERT (0);
> +        // Free the TokenMapper.
> +        // Ignore the returned Status since we already failed.
> +        TokenMapperShutdown (&This->TokenMapper);
> +        return Status;
> +      }
> +  } // for
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Get a CmObj from the dynamic repository.
> +
> +  @param [in]      This        Pointer to the Dynamic Platform Repository.
> +  @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] CmObjDesc   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
> +DynamicPlatRepoGetObject (
> +  IN      DYNAMIC_PLATFORM_REPOSITORY_INFO    * This,
> +  IN      CM_OBJECT_ID                          CmObjectId,
> +  IN      CM_OBJECT_TOKEN                       Token OPTIONAL,
> +  IN  OUT CM_OBJ_DESCRIPTOR                   * CmObjDesc
> +  )
> +{
> +  EFI_STATUS          Status;
> +  CM_OBJ_DESCRIPTOR   *Desc;
> +  CM_OBJECT_ID        ArmNamespaceObjId;
> +
> +  if ((This == NULL)      ||
> +      (CmObjDesc == NULL) ||
> +      (This->RepoState != DynRepoFinalized)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  ArmNamespaceObjId = GET_CM_OBJECT_ID (CmObjectId);
> +  if (ArmNamespaceObjId >= EArmObjMax) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Token != CM_NULL_TOKEN) {
> +    // Search in the Token Mapper and return the object.
> +    Status = TokenMapperGetObject (
> +               &This->TokenMapper,
> +               Token,
> +               CmObjectId,
> +               CmObjDesc
> +               );
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +
> +  if (ArmNamespaceObjId == EArmObjCmRef) {
> +    // EArmObjCmRef object must be requested using a valid token.
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Desc = &This->ArmCmObjArray[ArmNamespaceObjId];
> +
> +  // Nothing here.
> +  if (Desc->Count == 0) {
> +    return EFI_NOT_FOUND;
> +  } else {
> +    // Return the full array.
> +    CmObjDesc->ObjectId = Desc->ObjectId;
> +    CmObjDesc->Size = Desc->Size;
> +    CmObjDesc->Data = Desc->Data;
> +    CmObjDesc->Count = Desc->Count;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Initialize the dynamic platform repository.
> +
> +  @param [out]  DynPlatRepo   If success, contains the initialised dynamic
> +                              platform repository.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_OUT_OF_RESOURCES  An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DynamicPlatRepoInit (
> +  OUT DYNAMIC_PLATFORM_REPOSITORY_INFO   ** DynPlatRepo
> +  )
> +{
> +  UINTN                              Index;
> +  DYNAMIC_PLATFORM_REPOSITORY_INFO * Repo;
> +
> +  if (DynPlatRepo == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Repo = AllocateZeroPool (sizeof (DYNAMIC_PLATFORM_REPOSITORY_INFO));
> +  if (Repo == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  // Initialise the CmObject List.
> +  for (Index = 0; Index < EArmObjMax; Index++) {
> +    InitializeListHead (&Repo->ArmCmObjList[Index]);
> +  }
> +
> +  Repo->ObjectCount = 0;
> +  Repo->RepoState = DynRepoTransient;
> +
> +  *DynPlatRepo = Repo;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Shutdown the dynamic platform repository.
> +
> +  Free all the memory allocated for the dynamic platform repository.
> +
> +  @param [in]  DynPlatRepo    The dynamic platform repository.
> +
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_SUCCESS           Success.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DynamicPlatRepoShutdown (
> +  IN  DYNAMIC_PLATFORM_REPOSITORY_INFO * DynPlatRepo
> +  )
> +{
> +  EFI_STATUS            Status;
> +  UINT32                Index;
> +  LIST_ENTRY          * ListHead;
> +  CM_OBJ_DESCRIPTOR   * CmObjDesc;
> +  VOID                * Data;
> +
> +  if (DynPlatRepo == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Free the list of objects.
> +  for (Index = 0; Index < EArmObjMax; Index++) {
> +    // Free all the nodes with this object Id.
> +    ListHead = &DynPlatRepo->ArmCmObjList[Index];
> +    while (!IsListEmpty (ListHead)) {
> +      FreeCmObjNode ((CM_OBJ_NODE*)GetFirstNode (ListHead));
> +    } // while
> +  } // for
> +
> +  // Free the arrays.
> +  CmObjDesc = DynPlatRepo->ArmCmObjArray;
> +  for (Index = 0; Index < EArmObjMax; Index++) {
> +    Data = CmObjDesc[Index].Data;
> +    if (Data != NULL) {
> +      FreePool (Data);
> +    }
> +  } // for
> +
> +  // Free the TokenMapper
> +  Status = TokenMapperShutdown (&DynPlatRepo->TokenMapper);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
[SAMI] In DynamicPlatRepoFinalise() if an error occurs the Token Mapper 
would have been shutdown freeing all the memory.  So I think 
inTokenMapperShutdown(), the TokenDescArrayshould be set to NULL after 
freeing, andMaxTokenDescCount should be set to 0.
Can you check, please?
[/SAMI]
> +  }
> +
> +  FreePool (DynPlatRepo);
> +  return Status;
> +}
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoInternal.h b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoInternal.h
> new file mode 100644
> index 000000000000..d03fa2b7dcec
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoInternal.h
> @@ -0,0 +1,78 @@
> +/** @file
> +  Dynamic Platform Info Repository Internal
> +
> +  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 DYNAMIC_PLAT_REPO_INTERNAL_H_
> +#define DYNAMIC_PLAT_REPO_INTERNAL_H_
> +
> +#include "TokenMapper.h"
> +
> +#pragma pack(1)
> +
> +/** CmObj node.
> +
> +  This is a node wrapper around the CM_OBJ_DESCRIPTOR structure.
> +  It also allows to bind a token to the CM_OBJ_DESCRIPTOR.
> +*/
> +typedef struct CmObjectNode {
> +  /// This must be the first field in this structure.
> +  LIST_ENTRY          Link;
> +
> +  /// Token associated with the CmObjDesc.
> +  CM_OBJECT_TOKEN     Token;
> +
> +  /// CmObjDesc wrapped.
> +  /// Note: the CM_OBJ_DESCRIPTOR.Data field is allocated and copied.
> +  CM_OBJ_DESCRIPTOR   CmObjDesc;
> +} CM_OBJ_NODE;
> +
> +/** Dynamic repository states.
> +
> +  The states must progress as:
> +  UnInitialised -> Transient -> Finalized
> +*/
> +typedef enum DynRepoState {
> +  DynRepoUnInitialised, ///< Un-Initialised state
> +  DynRepoTransient,     ///< Transient state - CmObjects can be added.
> +  DynRepoFinalized,     ///< Repo Locked - No further CmObjects can be added.
> +                        ///< Getting objects is now possible.
> +  DynRepoMax            ///< Max value.
> +} EDYNAMIC_REPO_STATE;
> +
> +/** A structure describing the platform configuration
> +    manager repository information
> +*/
> +typedef struct DynamicPlatformRepositoryInfo {
> +  /// Repo state machine.
> +  EDYNAMIC_REPO_STATE   RepoState;
> +
> +  /// Count of all the objects added to the Dynamic Platform Repo
> +  /// during the Transient state.
> +  UINTN                 ObjectCount;
> +
> +  /// Link lists of CmObj from the ArmNameSpace
> +  /// that are added in the Transient state.
> +  LIST_ENTRY            ArmCmObjList[EArmObjMax];
> +
> +  /// Structure Members used in Finalized state.
> +  /// An array of CmObj Descriptors from the ArmNameSpace
> +  /// This array is populated when the Repo is finalized.
> +  CM_OBJ_DESCRIPTOR     ArmCmObjArray[EArmObjMax];
> +
> +  /// A token mapper for the objects in the ArmNamespaceObjectArray
> +  /// The Token mapper is populated when the Repo is finalized in
> +  /// a call to DynamicPlatRepoFinalise ().
> +  TOKEN_MAPPER          TokenMapper;
> +} DYNAMIC_PLATFORM_REPOSITORY_INFO;
> +
> +#pragma pack()
> +
> +#endif // DYNAMIC_PLAT_REPO_INTERNAL_H_
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> new file mode 100644
> index 000000000000..9a3cc87fd91d
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +#  Dynamic Platform Repository
> +#
> +#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION    = 0x0001001B
> +  BASE_NAME      = DynamicPlatRepoLib
> +  FILE_GUID      = 836D253D-3144-4A89-9BEE-BC55AFDC814E
> +  VERSION_STRING = 1.0
> +  MODULE_TYPE    = DXE_DRIVER
> +  LIBRARY_CLASS  = DynamicPlatRepoLib
> +
> +[Sources]
> +  CmObjectTokenFixer.c
> +  CmObjectTokenFixer.h
> +  DynamicPlatRepo.c
> +  DynamicPlatRepoInternal.h
> +  TokenGenerator.c
> +  TokenGenerator.h
> +  TokenMapper.c
> +  TokenMapper.h
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  DynamicTablesPkg/DynamicTablesPkg.dec
> +
> +[LibraryClasses]
> +  AcpiHelperLib
> +  BaseLib


[-- Attachment #2: Type: text/html, Size: 23222 bytes --]

      reply	other threads:[~2021-11-05 14:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23 13:36 [PATCH v1 0/5] Add DynamicPlatRepoLib PierreGondois
2021-06-23 13:36 ` [PATCH v1 1/5] DynamicTablesPkg: Definition for DynamicPlatRepoLib interface PierreGondois
2021-06-23 13:36 ` [PATCH v1 2/5] DynamicTablesPkg: DynamicPlatRepo: Add TokenGenerator PierreGondois
2021-06-23 13:36 ` [PATCH v1 3/5] DynamicTablesPkg: DynamicPlatRepo: Add TokenFixer PierreGondois
2021-06-23 13:36 ` [PATCH v1 4/5] DynamicTablesPkg: DynamicPlatRepo: Add TokenMapper PierreGondois
2021-11-05 13:31   ` Sami Mujawar
2021-06-23 13:36 ` [PATCH v1 5/5] DynamicTablesPkg: Add DynamicPlatRepo library PierreGondois
2021-11-05 14:10   ` Sami Mujawar [this message]

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=4164d6f0-a81e-0905-b479-0d65d0c722e3@arm.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