[edk2-devel] [PATCH v1 02/14] DynamicTablesPkg: FdtHwInfoParser: CM Object descriptor helper

Sami Mujawar sami.mujawar at arm.com
Fri Nov 5 14:27:25 UTC 2021


Hi Pierre,

Thank you for this patch.

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar

On 23/06/2021 01:38 PM, Pierre.Gondois at arm.com wrote:
> From: Pierre Gondois <Pierre.Gondois at arm.com>
>
> FdtHwInfoParserLib is an instance of the HwInfoParser. The
> FdtHwInfoParser parses a platform Device Tree and populates
> the Platform Information repository with Configuration
> Manager objects that describe the platform hardware.
> These Configuration Manager objects are encapsulated in
> Configuration Manager Object Descriptors.
>
> Therefore, add helper functions to create and free the
> Configuration Manager Object descriptors.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois at arm.com>
> Signed-off-by: Sami Mujawar <sami.mujawar at arm.com>
> ---
>   .../FdtHwInfoParserLib/CmObjectDescUtility.c  | 305 ++++++++++++++++++
>   .../FdtHwInfoParserLib/CmObjectDescUtility.h  | 131 ++++++++
>   2 files changed, 436 insertions(+)
>   create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.c
>   create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.h
>
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.c
> new file mode 100644
> index 000000000000..e471217504fe
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.c
> @@ -0,0 +1,305 @@
> +/** @file
> +  Configuration manager Object Descriptor Utility.
> +
> +  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <FdtHwInfoParserInclude.h>
> +#include <ConfigurationManagerObject.h>
> +
> +#include "CmObjectDescUtility.h"
> +
> +/** Create a CM_OBJ_DESCRIPTOR.
> +
> +  @param [in]  ObjectId       CM_OBJECT_ID of the node.
> +  @param [in]  Count          Number of CmObj stored in the
> +                              data field.
> +  @param [in]  Data           Pointer to one or more CmObj objects.
> +                              The content of this Data buffer is copied.
> +  @param [in]  Size           Size of the Data buffer.
> +  @param [out] NewCmObjDesc   The created CM_OBJ_DESCRIPTOR.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CreateCmObjDesc (
> +  IN  CM_OBJECT_ID          ObjectId,
> +  IN  UINT32                Count,
> +  IN  VOID                * Data,
> +  IN  UINT32                Size,
> +  OUT CM_OBJ_DESCRIPTOR  ** NewCmObjDesc
> +  )
> +{
> +  CM_OBJ_DESCRIPTOR   * CmObjDesc;
> +  VOID                * DataBuffer;
> +
> +  if ((Count == 0)      ||
> +      (Data == NULL)    ||
> +      (Size == 0)       ||
> +      (NewCmObjDesc == NULL)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  CmObjDesc = AllocateZeroPool (sizeof (CM_OBJ_DESCRIPTOR));
> +  if (CmObjDesc == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  DataBuffer = AllocateCopyPool (Size, Data);
> +  if (DataBuffer == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
[SAMI] CmObjDescmust be freed here otherwise there would be a memory leak.
> +  }
> +
> +  CmObjDesc->ObjectId = ObjectId;
> +  CmObjDesc->Count = Count;
> +  CmObjDesc->Data = DataBuffer;
> +  CmObjDesc->Size = Size;
> +
> +  *NewCmObjDesc = CmObjDesc;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Free resources allocated for the CM_OBJ_DESCRIPTOR.
> +
> +  @param [in] CmObjDesc           Pointer to the CM_OBJ_DESCRIPTOR.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +FreeCmObjDesc (
> +  IN CM_OBJ_DESCRIPTOR  * CmObjDesc
> +  )
> +{
> +  if (CmObjDesc == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (CmObjDesc->Data != NULL) {
> +    FreePool (CmObjDesc->Data);
> +  }
> +
> +  FreePool (CmObjDesc);
> +  return EFI_SUCCESS;
> +}
> +
> +/** Add a single CmObj to the Configuration Manager.
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  ObjectId          CmObj ObjectId.
> +  @param  [in]  Data              CmObj Data.
> +  @param  [in]  Size              CmObj Size.
> +  @param  [out] Token             If provided and success,
> +                                  token generated for this CmObj.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddSingleCmObj (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE   FdtParserHandle,
> +  IN        CM_OBJECT_ID                ObjectId,
> +  IN        VOID                        *Data,
> +  IN        UINT32                      Size,
> +  OUT       CM_OBJECT_TOKEN             *Token    OPTIONAL
> +  )
> +{
> +  EFI_STATUS          Status;
> +  CM_OBJ_DESCRIPTOR   CmObjDesc;
> +
> +  if ((FdtParserHandle == NULL)             ||
> +      (FdtParserHandle->HwInfoAdd == NULL)  ||
> +      (Data == NULL)                        ||
> +      (Size == 0)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  CmObjDesc.ObjectId = ObjectId;
> +  CmObjDesc.Count = 1;
> +  CmObjDesc.Data = Data;
> +  CmObjDesc.Size = Size;
> +
> +  // Add the CmObj.
> +  // Don't ask for a token.
> +  Status = FdtParserHandle->HwInfoAdd (
> +                              FdtParserHandle,
> +                              FdtParserHandle->Context,
> +                              &CmObjDesc,
> +                              Token
> +                              );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +  }
[SAMI] I thinkASSERT_EFI_ERROR() can be used here.
> +  return Status;
> +}
> +
> +/** Add multiple CmObj to the Configuration Manager.
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  CmObjDesc         CmObjDesc containing multiple CmObj
> +                                  to add.
> +  @param  [in]  TokenCount        If provided, count of entries in the
> +                                  TokenTable.
> +  @param  [out] TokenTable        If provided and success,
> +                                  token generated for these CmObj.
> +                                  Address of an array of CM_OBJECT_TOKEN
> +                                  with the same number of elements as the
> +                                  CmObjDesc.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddMultipleCmObj (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE   FdtParserHandle,
> +  IN  CONST CM_OBJ_DESCRIPTOR           *CmObjDesc,
> +  IN        UINT32                      TokenCount,   OPTIONAL
> +  OUT       CM_OBJECT_TOKEN             *TokenTable   OPTIONAL
> +  )
> +{
> +  EFI_STATUS          Status;
> +  UINT32              Index;
> +  UINT32              Count;
> +  VOID              * Data;
> +  UINT32              Size;
> +  CM_OBJ_DESCRIPTOR   SingleCmObjDesc;
> +
> +  if ((FdtParserHandle == NULL)             ||
> +      (FdtParserHandle->HwInfoAdd == NULL)  ||
> +      (CmObjDesc == NULL)                   ||
> +      (CmObjDesc->Count == 0)               ||
> +      (CmObjDesc->Data == NULL)             ||
> +      (CmObjDesc->Size == 0)){
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Count = CmObjDesc->Count;
> +  Data = CmObjDesc->Data;
> +  Size = CmObjDesc->Size / Count;
> +
> +  SingleCmObjDesc.ObjectId = CmObjDesc->ObjectId;
> +  SingleCmObjDesc.Count = 1;
> +  SingleCmObjDesc.Size = Size;
> +
> +  for (Index = 0; Index < Count; Index++) {
> +    SingleCmObjDesc.Data = Data + Index * Size;
[SAMI] Data needs to be typecasted to UINT8 for the pointer arithmetic 
to work. Also, enclose Index * Size in parenthesis.
> +    // Add the CmObj.
> +    Status = FdtParserHandle->HwInfoAdd (
> +                                FdtParserHandle,
> +                                FdtParserHandle->Context,
> +                                &SingleCmObjDesc,
> +                                (TokenTable != NULL) ?
> +                                  &TokenTable[Index] :
> +                                  NULL
> +                                );
> +    if (EFI_ERROR (Status)) {
> +      ASSERT (0);
> +      return Status;
> +    }
> +  } // for
> +
> +  return Status;
> +}
> +
> +/** Add multiple CmObj to the Configuration Manager.
> +
> +  Get one token referencing a EArmObjCmRef CmObj itself referencing
> +  the input CmObj. In the table below, RefToken is returned.
> +
> +  Token referencing an      Array of tokens             Array of CmObj
> +  array of EArmObjCmRef     referencing each            from the input:
> +  CmObj:                    CmObj from the input:
> +
> +  RefToken         --->     CmObjToken[0]        --->   CmObj[0]
> +                            CmObjToken[1]        --->   CmObj[1]
> +                            CmObjToken[2]        --->   CmObj[2]
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  CmObjDesc         CmObjDesc containing multiple CmObj
> +                                  to add.
> +  @param  [out] Token             If success, token referencing an array
> +                                  of EArmObjCmRef CmObj, themselves
> +                                  referencing the input CmObjs.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddMultipleCmObjWithCmObjRef (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE     FdtParserHandle,
> +  IN  CM_OBJ_DESCRIPTOR                 * CmObjDesc,
> +  OUT CM_OBJECT_TOKEN                   * Token
> +  )
> +{
> +  EFI_STATUS          Status;
> +  CM_OBJ_DESCRIPTOR   CmObjRef;
> +  CM_OBJECT_TOKEN    *TokenTable;
> +  INT32               TokenTableSize;
> +
> +  if ((FdtParserHandle == NULL)             ||
> +      (FdtParserHandle->HwInfoAdd == NULL)  ||
> +      (CmObjDesc == NULL)                   ||
> +      (CmObjDesc->Count == 0)               ||
> +      (CmObjDesc->Data == NULL)             ||
> +      (CmObjDesc->Size == 0)                ||
> +      (Token == NULL)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Allocate a buffer to store the tokens.
> +  TokenTableSize = CmObjDesc->Count * sizeof (CM_OBJECT_TOKEN);
> +  TokenTable = AllocateZeroPool (TokenTableSize);
> +  if (TokenTable == NULL) {
> +    ASSERT (0);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  // Add the input CmObjs.
> +  Status = AddMultipleCmObj (
> +             FdtParserHandle,
> +             CmObjDesc,
> +             CmObjDesc->Count,
> +             TokenTable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    goto exit_handler;
> +  }
> +
> +  CmObjRef.ObjectId = CREATE_CM_ARM_OBJECT_ID (EArmObjCmRef);
> +  CmObjRef.Data = TokenTable;
> +  CmObjRef.Count = CmObjDesc->Count;
> +  CmObjRef.Size = TokenTableSize;
> +
> +  // Add the array of EArmObjCmRef CmObjs.
> +  Status = FdtParserHandle->HwInfoAdd (
> +                              FdtParserHandle,
> +                              FdtParserHandle->Context,
> +                              &CmObjRef,
> +                              Token
> +                              );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +  }
[SAMI] I thinkASSERT_EFI_ERROR() can be used here.
> +
> +exit_handler:
> +  FreePool (TokenTable);
> +  return Status;
> +}
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.h
> new file mode 100644
> index 000000000000..34439c716fb3
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/CmObjectDescUtility.h
> @@ -0,0 +1,131 @@
> +/** @file
> +  Configuration manager Object Descriptor Utility.
> +
> +  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef CM_OBJECT_DESC_UTILITY_H_
> +#define CM_OBJECT_DESC_UTILITY_H_
> +
> +#include <ConfigurationManagerObject.h>
> +
> +#include "FdtHwInfoParser.h"
> +
> +/** Create a CM_OBJ_DESCRIPTOR.
> +
> +  @param [in]  ObjectId       CM_OBJECT_ID of the node.
> +  @param [in]  Count          Number of CmObj stored in the
> +                              data field.
> +  @param [in]  Data           Pointer to one or more CmObj objects.
> +                              The content of this Data buffer is copied.
> +  @param [in]  Size           Size of the Data buffer.
> +  @param [out] NewCmObjDesc   The created CM_OBJ_DESCRIPTOR.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CreateCmObjDesc (
> +  IN  CM_OBJECT_ID          ObjectId,
> +  IN  UINT32                Count,
> +  IN  VOID                * Data,
> +  IN  UINT32                Size,
> +  OUT CM_OBJ_DESCRIPTOR  ** NewCmObjDesc
> +  );
> +
> +/** Free resources allocated for the CM_OBJ_DESCRIPTOR.
> +
> +  @param [in] CmObjDesc           Pointer to the CM_OBJ_DESCRIPTOR.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +FreeCmObjDesc (
> +  IN CM_OBJ_DESCRIPTOR  * CmObjDesc
> +  );
> +
> +/** Add a single CmObj to the Configuration Manager.
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  ObjectId          CmObj ObjectId.
> +  @param  [in]  Data              CmObj Data.
> +  @param  [in]  Size              CmObj Size.
> +  @param  [out] Token             If provided and success,
> +                                  token generated for this CmObj.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddSingleCmObj (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE   FdtParserHandle,
> +  IN        CM_OBJECT_ID                ObjectId,
> +  IN        VOID                        *Data,
> +  IN        UINT32                      Size,
> +  OUT       CM_OBJECT_TOKEN             *Token    OPTIONAL
> +  );
> +
> +/** Add multiple CmObj to the Configuration Manager.
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  CmObjDesc         CmObjDesc containing multiple CmObj
> +                                  to add.
> +  @param  [in]  TokenCount        If provided, count of entries in the
> +                                  TokenTable.
> +  @param  [out] TokenTable        If provided and success,
> +                                  token generated for these CmObj.
> +                                  Address of an array of CM_OBJECT_TOKEN
> +                                  with the same number of elements as the
> +                                  CmObjDesc.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddMultipleCmObj (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE   FdtParserHandle,
> +  IN  CONST CM_OBJ_DESCRIPTOR           *CmObjDesc,
> +  IN        UINT32                      TokenCount,   OPTIONAL
> +  OUT       CM_OBJECT_TOKEN             *TokenTable   OPTIONAL
> +  );
> +
> +/** Add multiple CmObj to the Configuration Manager.
> +
> +  Get one token referencing a EArmObjCmRef CmObj itself referencing
> +  the input CmObj. In the table below, RefToken is returned.
> +
> +  Token referencing an      Array of tokens             Array of CmObj
> +  array of EArmObjCmRef     referencing each            from the input:
> +  CmObj:                    CmObj from the input:
> +
> +  RefToken         --->     CmObjToken[0]        --->   CmObj[0]
> +                            CmObjToken[1]        --->   CmObj[1]
> +                            CmObjToken[2]        --->   CmObj[2]
> +
> +  @param  [in]  FdtParserHandle   A handle to the parser instance.
> +  @param  [in]  CmObjDesc         CmObjDesc containing multiple CmObj
> +                                  to add.
> +  @param  [out] Token             If success, token referencing an array
> +                                  of EArmObjCmRef CmObj, themselves
> +                                  referencing the input CmObjs.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    An allocation has failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AddMultipleCmObjWithCmObjRef (
> +  IN  CONST FDT_HW_INFO_PARSER_HANDLE     FdtParserHandle,
> +  IN  CM_OBJ_DESCRIPTOR                 * CmObjDesc,
> +  OUT CM_OBJECT_TOKEN                   * Token
> +  );
> +
> +#endif // CM_OBJECT_DESC_UTILITY_H_



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83399): https://edk2.groups.io/g/devel/message/83399
Mute This Topic: https://groups.io/mt/83736622/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20211105/14093729/attachment.htm>


More information about the edk2-devel-archive mailing list