Re: [edk2-devel] [PATCH v3 5/5] DynamicTablesPkg: Smbios Physical Memory Array (Type 16)

["Jose Marinho" <jose.marinho@arm.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Girish
> Mahadevan via groups.io
> Sent: Wednesday, July 26, 2023 12:38 AM
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: gmahadevan@nvidia.com; Alexei Fedorov <Alexei.Fedorov@arm.com>;
> Pierre Gondois <Pierre.Gondois@arm.com>; Jeff Brasen (jbrasen@nvidia.com)
> <jbrasen@nvidia.com>; ashishsingha@nvidia.com; nramirez@nvidia.com
> Subject: [edk2-devel] [PATCH v3 5/5] DynamicTablesPkg: Smbios Physical
> Memory Array (Type 16)
> 
> Add the Generator library for SMBIOS Table Type 16 - Physical Memory Array.
> 
> Signed-off-by: Girish Mahadevan <gmahadevan@nvidia.com>
> Reviewed-by: Jeff Brasen <jbrasen@nvidia.com>
> Reviewed-by: Nick Ramirez <nramirez@nvidia.com>
> ---
>  .../SmbiosTableBuilder.c                      |   3 -
>  .../Include/SmbiosNameSpaceObjects.h          |  19 +
>  .../SmbiosType16Lib/SmbiosType16Generator.c   | 361 ++++++++++++++++++
>  .../SmbiosType16Lib/SmbiosType16Lib.inf       |  35 ++
>  4 files changed, 415 insertions(+), 3 deletions(-)  create mode 100644
> DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Generator
> .c
>  create mode 100644
> DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Lib.inf
> 
> diff --git
> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c
> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c
> index bfddaac3aa..5feafcac66 100644
> ---
> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder.c
> +++
> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/SmbiosTableBuilder
> +++ .c
> @@ -312,7 +312,6 @@ exit_handler:
>      }
>    }
> 
> -  DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
>    return Status;

nit: these debug statements were added in the commit with title
" DynamicTablesPkg: Split the ACPI and SMBIOS table generators"
Should these be removed there?

Jose

>  }
> 
> @@ -432,7 +431,6 @@ BuildAndInstallSmbiosTable (
>        ));
>    }
> 
> -  DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
>    return Status;
>  }
> 
> @@ -516,7 +514,6 @@ ProcessSmbiosTables (
>        ));
>    }
> 
> -  DEBUG ((DEBUG_ERROR, "%a: Returning %r\n", __FUNCTION__, Status));
>    return Status;
>  }
> 
> diff --git a/DynamicTablesPkg/Include/SmbiosNameSpaceObjects.h
> b/DynamicTablesPkg/Include/SmbiosNameSpaceObjects.h
> index e44d703fac..760b082021 100644
> --- a/DynamicTablesPkg/Include/SmbiosNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/SmbiosNameSpaceObjects.h
> @@ -107,6 +107,25 @@ typedef struct {
>    SMBIOS_TABLE_GENERATOR_ID    GeneratorId;
>  } CONTAINED_CM_OBJECTS;
> 
> +/** A structure that describes the Physical Memory Array.
> +
> +  SMBIOS Specification v3.6.0 Type 16
> +
> +  ID: ESmbiosObjPhysicalMemoryArray
> +**/
> +typedef struct CmSmbiosPhysicalMemoryArray {
> +  UINT8              Use;
> +  UINT8              Location;
> +  UINT16             MemoryErrorCorrection;
> +  UINT16             MemoryErrorInformationHandle;
> +  UINT16             NumberOfMemoryDevices;
> +  UINT8              MemoryErrorCorrectionType;
> +  UINT64             Size;
> +  UINT8              NumMemDevices;
> +  CM_OBJECT_TOKEN    MemoryErrInfoToken;
> +  CM_OBJECT_TOKEN    PhysMemArrayToken;
> +} CM_SMBIOS_PHYSICAL_MEMORY_ARRAY;
> +
>  #pragma pack()
> 
>  #endif // SMBIOS_NAMESPACE_OBJECTS_H_
> diff --git
> a/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Generat
> or.c
> b/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Generat
> or.c
> new file mode 100644
> index 0000000000..dad7588ed5
> --- /dev/null
> +++
> b/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Genera
> +++ tor.c
> @@ -0,0 +1,361 @@
> +/** @file
> +  SMBIOS Type16 Table Generator.
> +
> +  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/MemoryAllocationLib.h> #include
> +<Library/UefiBootServicesTableLib.h>
> +#include <Library/SmbiosStringTableLib.h>
> +
> +// Module specific include files.
> +#include <ConfigurationManagerObject.h> #include
> +<ConfigurationManagerHelper.h> #include
> +<Protocol/ConfigurationManagerProtocol.h>
> +#include <Protocol/DynamicTableFactoryProtocol.h>
> +#include <Protocol/Smbios.h>
> +#include <IndustryStandard/SmBios.h>
> +
> +/** This macro expands to a function that retrieves the Memory Device
> +    information from the Configuration Manager.
> +*/
> +GET_OBJECT_LIST (
> +  EObjNameSpaceSmbios,
> +  ESmbiosObjPhysicalMemoryArray,
> +  CM_SMBIOS_PHYSICAL_MEMORY_ARRAY
> +  )
> +
> +#define EXTENDED_SIZE_THRESHOLD  (SIZE_2TB)
> +
> +/**
> + * Free any resources allocated when installing SMBIOS Type16 table.
> + *
> + * @param [in]  This                 Pointer to the SMBIOS table generator.
> + * @param [in]  TableFactoryProtocol Pointer to the SMBIOS Table Factory
> +                                      Protocol interface.
> +
> + * @param [in]  SmbiosTableInfo      Pointer to the SMBIOS table
> information.
> + * @param [in]  CfgMgrProtocol       Pointer to the Configuration Manager
> +                                     Protocol interface.
> + * @param [in] Table                 Pointer to the SMBIOS table.
> + * @param [in] CmObjectToken         Pointer to the CM ObjectToken Array.
> + * @param [in] TableCount            Number of SMBIOS tables.
> +
> + * @retval EFI_SUCCESS            Table generated successfully.
> + * @retval EFI_BAD_BUFFER_SIZE    The size returned by the Configuration
> +                                  Manager is less than the Object size for
> +                                  the requested object.
> + * @retval EFI_INVALID_PARAMETER  A parameter is invalid.
> + * @retval EFI_NOT_FOUND          Could not find information.
> + * @retval EFI_OUT_OF_RESOURCES   Could not allocate memory.
> + * @retval EFI_UNSUPPORTED        Unsupported configuration.
> +**/
> +STATIC
> +EFI_STATUS
> +FreeSmbiosType16TableEx (
> +  IN      CONST SMBIOS_TABLE_GENERATOR                    *CONST   This,
> +  IN      CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL      *CONST
> TableFactoryProtocol,
> +  IN      CONST CM_STD_OBJ_SMBIOS_TABLE_INFO              *CONST
> SmbiosTableInfo,
> +  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL      *CONST
> CfgMgrProtocol,
> +  IN      SMBIOS_STRUCTURE                               ***CONST  Table,
> +  IN      CM_OBJECT_TOKEN                                          **CmObjectToken,
> +  IN      CONST UINTN                                              TableCount
> +  )
> +{
> +  UINTN             Index;
> +  SMBIOS_STRUCTURE  **TableList;
> +
> +  TableList = *Table;
> +  for (Index = 0; Index < TableCount; Index++) {
> +    if (TableList[Index] != NULL) {
> +      FreePool (TableList[Index]);
> +    }
> +  }
> +
> +  if (*CmObjectToken != NULL) {
> +    FreePool (*CmObjectToken);
> +  }
> +
> +  if (TableList != NULL) {
> +    FreePool (TableList);
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> + *  Update the Size encoding for Type 16.
> + *
> + *  @param [in]     SizeBytes    Size of the Memory device.
> + *  @param [out]    SmbiosRecord SMBIOS record to update.
> +**/
> +STATIC
> +VOID
> +UpdateSmbiosType16Size (
> +  IN     UINT64               SizeBytes,
> +  IN OUT SMBIOS_TABLE_TYPE16  *SmbiosRecord
> +  )
> +{
> +  UINT64  SizeKb = SizeBytes / SIZE_1KB;
> +
> +  if (SizeBytes < EXTENDED_SIZE_THRESHOLD) {
> +    SmbiosRecord->MaximumCapacity = SizeKb;
> +  } else {
> +    SmbiosRecord->MaximumCapacity         = 0x80000000;
> +    SmbiosRecord->ExtendedMaximumCapacity = SizeKb;
> +  }
> +}
> +
> +/**
> + *  Add the SMBIOS table handle reference to the Error Tables.
> + *
> + *  @param [in]     TableFactoryProtocol   Pointer to the SMBIOS Table
> Factory.
> + *  @param [in]     CmObjToken             CM Token to lookup..
> + *  @param [out]    SmbiosRecord           SMBIOS record to update.
> +**/
> +STATIC
> +VOID
> +AddMemErrDeviceHandle (
> +  IN  CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL   *CONST
> TableFactoryProtocol,
> +  IN  CM_OBJECT_TOKEN                                      CmObjToken,
> +  OUT SMBIOS_TABLE_TYPE16                                  *SmbiosRecord
> +  )
> +{
> +  EFI_SMBIOS_HANDLE  Handle;
> +  SMBIOS_HANDLE_MAP  *HandleMap;
> +
> +  HandleMap = TableFactoryProtocol->GetSmbiosHandle (CmObjToken);  if
> + (HandleMap == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a:Failed to get SMBIOS Handle\n",
> __FUNCTION__));
> +    Handle = 0xFFFF;
> +  } else {
> +    Handle = HandleMap->SmbiosTblHandle;  }
> +
> +  SmbiosRecord->MemoryErrorInformationHandle = Handle; }
> +
> +/** Construct SMBIOS Type16 Table describing memory devices.
> +
> +  If this function allocates any resources then they must be freed  in
> + the FreeXXXXTableResources function.
> +
> + * @param [in]  This                 Pointer to the SMBIOS table generator.
> + * @param [in]  TableFactoryProtocol Pointer to the SMBIOS Table Factory
> + *                                   Protocol interface.
> + * @param [in]  SmbiosTableInfo      Pointer to the SMBIOS table
> information.
> + * @param [in]  CfgMgrProtocol       Pointer to the Configuration Manager
> + *                                   Protocol interface.
> + * @param [out] Table                Pointer to the SMBIOS table.
> + * @param [out] CmObjectToken        Pointer to the CM Object Token Array.
> + * @param [out] TableCount           Number of tables installed.
> +
> + * @retval EFI_SUCCESS            Table generated successfully.
> + * @retval EFI_BAD_BUFFER_SIZE    The size returned by the Configuration
> + *                                Manager is less than the Object size for
> + *                                the requested object.
> + * @retval EFI_INVALID_PARAMETER  A parameter is invalid.
> + * @retval EFI_NOT_FOUND          Could not find information.
> + * @retval EFI_OUT_OF_RESOURCES   Could not allocate memory.
> + * @retval EFI_UNSUPPORTED        Unsupported configuration.
> +**/
> +STATIC
> +EFI_STATUS
> +BuildSmbiosType16TableEx (
> +  IN  CONST SMBIOS_TABLE_GENERATOR                         *This,
> +  IN  CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL   *CONST
> TableFactoryProtocol,
> +  IN        CM_STD_OBJ_SMBIOS_TABLE_INFO           *CONST  SmbiosTableInfo,
> +  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL   *CONST
> CfgMgrProtocol,
> +  OUT       SMBIOS_STRUCTURE                               ***Table,
> +  OUT       CM_OBJECT_TOKEN                                **CmObjectToken,
> +  OUT       UINTN                                  *CONST  TableCount
> +  )
> +{
> +  EFI_STATUS                       Status;
> +  SMBIOS_STRUCTURE                 **TableList;
> +  SMBIOS_TABLE_TYPE16              *SmbiosRecord;
> +  CM_OBJECT_TOKEN                  *CmObjectList;
> +  CM_SMBIOS_PHYSICAL_MEMORY_ARRAY  *PhysMemoryArray;
> +  UINT32                           NumObj;
> +  UINTN                            Index;
> +
> +  ASSERT (This != NULL);
> +  ASSERT (SmbiosTableInfo != NULL);
> +  ASSERT (CfgMgrProtocol != NULL);
> +  ASSERT (Table != NULL);
> +  ASSERT (TableCount != NULL);
> +  ASSERT (SmbiosTableInfo->TableGeneratorId == This->GeneratorID);
> +
> +  if ((This == NULL) || (SmbiosTableInfo == NULL) || (CfgMgrProtocol == NULL)
> ||
> +      (Table == NULL) || (TableCount == NULL) ||
> +      (SmbiosTableInfo->TableGeneratorId != This->GeneratorID))  {
> +    DEBUG ((DEBUG_ERROR, "%a:Invalid Paramater\n ", __FUNCTION__));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *Table = NULL;
> +  Status = GetESmbiosObjPhysicalMemoryArray (
> +             CfgMgrProtocol,
> +             CM_NULL_TOKEN,
> +             &PhysMemoryArray,
> +             &NumObj
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to get Memory Devices CM Object %r\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  TableList = (SMBIOS_STRUCTURE **)AllocateZeroPool (sizeof
> + (SMBIOS_STRUCTURE *) * NumObj);  if (TableList == NULL) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to alloc memory for %u devices table\n",
> +      __FUNCTION__,
> +      NumObj
> +      ));
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto exitBuildSmbiosType16Table;
> +  }
> +
> +  CmObjectList = (CM_OBJECT_TOKEN *)AllocateZeroPool (sizeof
> + (CM_OBJECT_TOKEN *) * NumObj);  if (CmObjectList == NULL) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to alloc memory for %u CM Objects.\n",
> +      __FUNCTION__,
> +      NumObj
> +      ));
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto exitBuildSmbiosType16Table;
> +  }
> +
> +  for (Index = 0; Index < NumObj; Index++) {
> +    /**
> +     * Per Spec each structure is terminated by a double-NULL if there are no
> +     * strings.
> +     */
> +    SmbiosRecord = (SMBIOS_TABLE_TYPE16 *)AllocateZeroPool (sizeof
> (SMBIOS_TABLE_TYPE16) + 1 + 1);
> +    if (SmbiosRecord == NULL) {
> +      Status = EFI_OUT_OF_RESOURCES;
> +      goto exitBuildSmbiosType16Table;
> +    }
> +
> +    UpdateSmbiosType16Size (PhysMemoryArray->Size, SmbiosRecord);
> +    SmbiosRecord->Location              = PhysMemoryArray[Index].Location;
> +    SmbiosRecord->Use                   = PhysMemoryArray[Index].Use;
> +    SmbiosRecord->MemoryErrorCorrection =
> PhysMemoryArray[Index].MemoryErrorCorrectionType;
> +    SmbiosRecord->NumberOfMemoryDevices = PhysMemoryArray-
> >NumMemDevices;
> +    if (PhysMemoryArray[Index].MemoryErrInfoToken != CM_NULL_TOKEN) {
> +      AddMemErrDeviceHandle (
> +        TableFactoryProtocol,
> +        PhysMemoryArray[Index].MemoryErrInfoToken,
> +        SmbiosRecord
> +        );
> +    } else {
> +      SmbiosRecord->MemoryErrorInformationHandle = 0xFFFF;
> +    }
> +
> +    // Setup the header
> +    SmbiosRecord->Hdr.Type   =
> EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY;
> +    SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE16);
> +
> +    TableList[Index]    = (SMBIOS_STRUCTURE *)SmbiosRecord;
> +    CmObjectList[Index] = PhysMemoryArray[Index].PhysMemArrayToken;
> +  }
> +
> +  *Table         = TableList;
> +  *CmObjectToken = CmObjectList;
> +  *TableCount    = NumObj;
> +exitBuildSmbiosType16Table:
> +  return Status;
> +}
> +
> +/** The interface for the SMBIOS Type16 Table Generator.
> +*/
> +STATIC
> +CONST
> +SMBIOS_TABLE_GENERATOR  SmbiosType16Generator = {
> +  // Generator ID
> +  CREATE_STD_SMBIOS_TABLE_GEN_ID (EStdSmbiosTableIdType16),
> +  // Generator Description
> +  L"SMBIOS.TYPE16.GENERATOR",
> +  // SMBIOS Table Type
> +  EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY,
> +  NULL,
> +  NULL,
> +  // Build table function.
> +  BuildSmbiosType16TableEx,
> +  // Free function.
> +  FreeSmbiosType16TableEx,
> +};
> +
> +/** Register the Generator with the SMBIOS Table Factory.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  SystemTable  Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS           The Generator is registered.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_ALREADY_STARTED   The Generator for the Table ID
> +                                is already registered.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmbiosType16LibConstructor (
> +  IN  EFI_HANDLE        ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = RegisterSmbiosTableGenerator (&SmbiosType16Generator);
> + DEBUG ((
> +    DEBUG_INFO,
> +    "SMBIOS Type 16: Register Generator. Status = %r\n",
> +    Status
> +    ));
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
> +/** Deregister the Generator from the SMBIOS Table Factory.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  SystemTable  Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS           The Generator is deregistered.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_NOT_FOUND         The Generator is not registered.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmbiosType16LibDestructor (
> +  IN  EFI_HANDLE        ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = DeregisterSmbiosTableGenerator (&SmbiosType16Generator);
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "SMBIOS Type16: Deregister Generator. Status = %r\n",
> +    Status
> +    ));
> +  ASSERT_EFI_ERROR (Status);
> +  return Status;
> +}
> diff --git
> a/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Lib.inf
> b/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Lib.inf
> new file mode 100644
> index 0000000000..5bb40db21c
> --- /dev/null
> +++
> b/DynamicTablesPkg/Library/Smbios/SmbiosType16Lib/SmbiosType16Lib.in
> +++ f
> @@ -0,0 +1,35 @@
> +## @file
> +# SMBIOS Type17 Table Generator
> +#
> +#  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +#  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR> # #
> +SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> +  INF_VERSION    = 0x0001001B
> +  BASE_NAME      = SmbiosType16LibArm
> +  FILE_GUID      = a256956a-de0b-4aa7-8eac-5ce13bbfbb1f
> +  VERSION_STRING = 1.0
> +  MODULE_TYPE    = DXE_DRIVER
> +  LIBRARY_CLASS  = NULL|DXE_DRIVER
> +  CONSTRUCTOR    = SmbiosType16LibConstructor
> +  DESTRUCTOR     = SmbiosType16LibDestructor
> +
> +[Sources]
> +  SmbiosType16Generator.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  DynamicTablesPkg/DynamicTablesPkg.dec
> +
> +[Protocols]
> +  gEfiSmbiosProtocolGuid                        # PROTOCOL ALWAYS_CONSUMED
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> --
> 2.17.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108021): https://edk2.groups.io/g/devel/message/108021
Mute This Topic: https://groups.io/mt/100361564/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-