From: Swatisri Kantamsetti <swatisrik@nvidia.com>

Added SLIT Generator and supporting files to
generate SLIT ACPI Table

Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
---
 DynamicTablesPkg/DynamicTables.dsc.inc        |   2 +
 DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
 .../Include/ArmNameSpaceObjects.h             |  12 +
 .../Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf     |  29 ++
 .../Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c   | 326 ++++++++++++++++++
 MdePkg/Include/IndustryStandard/Slit.h        |  39 +++
 6 files changed, 409 insertions(+)
 create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
 create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
 create mode 100644 MdePkg/Include/IndustryStandard/Slit.h

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 3d4fa0c4c4..c24fc1d719 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -31,6 +31,7 @@
   DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
   DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
   DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
+  DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
   DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
   DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
 
@@ -56,6 +57,7 @@
       NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
       NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
       NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
+      NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
       NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
 
       # AML Fixup
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index f962dbff57..82d0272031 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -93,6 +93,7 @@ typedef enum StdAcpiTableId {
   EStdAcpiTableIdMcfg,                          ///< MCFG Generator
   EStdAcpiTableIdIort,                          ///< IORT Generator
   EStdAcpiTableIdPptt,                          ///< PPTT Generator
+  EStdAcpiTableIdSlit,                          ///< SLIT Generator
   EStdAcpiTableIdSrat,                          ///< SRAT Generator
   EStdAcpiTableIdSsdtSerialPort,                ///< SSDT Serial-Port Generator
   EStdAcpiTableIdSsdtCmn600,                    ///< SSDT Cmn-600 Generator
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index c66b441d53..aa3f464132 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -65,6 +65,7 @@ typedef enum ArmObjectID {
   EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
   EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
   EArmObjCpcInfo,                      ///< 42 - Continuous Performance Control Info
+  EArmObjSystemLocalityInfo,           ///< 43 - Relative Distance Info
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -1095,6 +1096,17 @@ typedef struct CmArmRmrDescriptor {
 */
 typedef AML_CPC_INFO CM_ARM_CPC_INFO;
 
+/** A structure that describes the System Locality Information
+
+  This information is used to optimize the system performance
+
+  ID: EArmObjSystemLocalityInfo
+*/
+typedef struct CmArmSystemLocalityInfo {
+  UINT64    NumSystemLocalities;
+  UINT8     *Distance;
+} CM_ARM_SYSTEM_LOCALITY_INFO;
+
 #pragma pack()
 
 #endif // ARM_NAMESPACE_OBJECTS_H_
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
new file mode 100644
index 0000000000..ab0ffc9a39
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
@@ -0,0 +1,29 @@
+## @file
+#  SRAT Table Generator
+#
+#  Copyright (c) 2022, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = AcpiSlitLibArm
+  FILE_GUID      = 28e3f485-0c24-4376-8982-8e1febd791bc
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER
+  CONSTRUCTOR    = AcpiSlitLibConstructor
+  DESTRUCTOR     = AcpiSlitLibDestructor
+
+[Sources]
+  SlitGenerator.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
new file mode 100644
index 0000000000..1a232ce95c
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
@@ -0,0 +1,326 @@
+/** @file
+  SLIT Table Generator
+
+  Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - ACPI 6.3 Specification, January 2019
+
+  @par Glossary:
+  - Cm or CM   - Configuration Manager
+  - Obj or OBJ - Object
+**/
+
+
+#include <IndustryStandard/Slit.h>
+#include <Library/AcpiLib.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/AcpiTable.h>
+
+// Module specific include files.
+#include <AcpiTableGenerator.h>
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <Library/TableHelperLib.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+
+/**
+  ARM standard SLIT Generator
+
+  Requirements:
+    The following Configuration Manager Object(s) are used by this Generator:
+    - EArmObjSystemLocalityInfo (Required)
+*/
+
+/**
+  This macro expands to a function that retrieves the Relative Distance
+  information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjSystemLocalityInfo,
+  CM_ARM_SYSTEM_LOCALITY_INFO
+  );
+
+/** Construct the SLIT ACPI table.
+
+  Called by the Dynamic Table Manager, this function invokes the
+  Configuration Manager protocol interface to get the required hardware
+  information for generating the ACPI table.
+
+  If this function allocates any resources then they must be freed
+  in the FreeXXXXTableResources function.
+
+  @param [in]  This           Pointer to the table generator.
+  @param [in]  AcpiTableInfo  Pointer to the ACPI Table Info.
+  @param [in]  CfgMgrProtocol Pointer to the Configuration Manager
+                              Protocol Interface.
+  @param [out] Table          Pointer to the constructed ACPI Table.
+
+  @retval EFI_SUCCESS           Table generated successfully.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object was not found.
+  @retval EFI_BAD_BUFFER_SIZE   The size returned by the Configuration
+                                Manager is less than the Object size for the
+                                requested object.
+  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildSlitTable (
+  IN  CONST ACPI_TABLE_GENERATOR                  *CONST  This,
+  IN  CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  OUT       EFI_ACPI_DESCRIPTION_HEADER          **CONST  Table
+  )
+{
+  EFI_STATUS  Status;
+  UINT32      TableSize;
+  UINT32      SlitObjectCount;
+  UINT32      SlitObjectOffset;
+  UINT64      NumSysLocalities;
+  UINT8       *RelDistInfo;
+
+  CM_ARM_SYSTEM_LOCALITY_INFO                        *SysLocalityInfo;
+  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE     *Slit;
+
+  ASSERT (
+    (This != NULL) &&
+    (AcpiTableInfo != NULL) &&
+    (CfgMgrProtocol != NULL) &&
+    (Table != NULL) &&
+    (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
+    (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
+    );
+
+  if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
+      (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision))
+  {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SLIT: Requested table revision = %d is not supported. "
+      "Supported table revisions: Minimum = %d. Maximum = %d\n",
+      AcpiTableInfo->AcpiTableRevision,
+      This->MinAcpiTableRevision,
+      This->AcpiTableRevision
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Table = NULL;
+
+  Status = GetEArmObjSystemLocalityInfo (
+             CfgMgrProtocol,
+             CM_NULL_TOKEN,
+             &SysLocalityInfo,
+             &SlitObjectCount
+             );
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SLIT: Failed to get Relative Distance Info. Status = %r\n",
+      Status
+      ));
+    goto error_handler;
+  }
+
+  // Calculate the size of the SLIT table
+  TableSize = sizeof (EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE);
+
+  // Number of System Localities
+  NumSysLocalities = SysLocalityInfo->NumSystemLocalities;
+
+  if (SlitObjectCount != 0) {
+    SlitObjectOffset = TableSize;
+    TableSize   += (sizeof (UINT8) * NumSysLocalities * NumSysLocalities);
+  }
+
+  // Allocate the Buffer for SLIT table
+  *Table = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize);
+  if (*Table == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SLIT: Failed to allocate memory for SLIT Table, Size = %d," \
+      " Status = %r\n",
+      TableSize,
+      Status
+      ));
+    goto error_handler;
+  }
+
+  Slit = (EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE *)*Table;
+
+  DEBUG ((
+    DEBUG_INFO,
+    "SLIT: Slit = 0x%p TableSize = 0x%x\n",
+    Slit,
+    TableSize
+    ));
+
+  Status = AddAcpiHeader (
+             CfgMgrProtocol,
+             This,
+             &Slit->Header,
+             AcpiTableInfo,
+             TableSize
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SLIT: Failed to add ACPI header. Status = %r\n",
+      Status
+      ));
+    goto error_handler;
+  }
+
+  // Number of System Localities
+  Slit->NumLocalities = NumSysLocalities;
+
+  // Offset to place the Relative Distance Info
+  RelDistInfo = (UINT8*)((UINT8*)Slit + SlitObjectOffset);
+
+  // Adding Relative distance in SLIT Table
+  for (UINT64 Index = 0; Index < Slit->NumLocalities * Slit->NumLocalities; Index++) {
+    *RelDistInfo  = SysLocalityInfo->Distance[Index];
+    // Next
+    RelDistInfo++;
+  }// while
+
+  return Status;
+
+error_handler:
+
+  if (*Table != NULL) {
+    FreePool (*Table);
+    *Table = NULL;
+  }
+
+  return Status;
+}
+
+/** Free any resources allocated for constructing the SLIT.
+
+  @param [in]      This           Pointer to the table generator.
+  @param [in]      AcpiTableInfo  Pointer to the ACPI Table Info.
+  @param [in]      CfgMgrProtocol Pointer to the Configuration Manager
+                                  Protocol Interface.
+  @param [in, out] Table          Pointer to the ACPI Table.
+
+  @retval EFI_SUCCESS           The resources were freed successfully.
+  @retval EFI_INVALID_PARAMETER The table pointer is NULL or invalid.
+**/
+STATIC
+EFI_STATUS
+FreeSlitTableResources (
+  IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
+  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN OUT        EFI_ACPI_DESCRIPTION_HEADER          **CONST  Table
+  )
+{
+  ASSERT (
+    (This != NULL) &&
+    (AcpiTableInfo != NULL) &&
+    (CfgMgrProtocol != NULL) &&
+    (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
+    (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
+    );
+
+  if ((Table == NULL) || (*Table == NULL)) {
+    DEBUG ((DEBUG_ERROR, "ERROR: SLIT: Invalid Table Pointer\n"));
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  FreePool (*Table);
+  *Table = NULL;
+  return EFI_SUCCESS;
+}
+
+/** The SLIT Table Generator revision.
+*/
+#define SLIT_GENERATOR_REVISION  CREATE_REVISION (1, 0)
+
+/** The interface for the SLIT Table Generator.
+*/
+STATIC
+CONST
+ACPI_TABLE_GENERATOR  SlitGenerator = {
+  // Generator ID
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSlit),
+  // Generator Description
+  L"ACPI.STD.SLIT.GENERATOR",
+  // ACPI Table Signature
+  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE,
+  // ACPI Table Revision supported by this Generator
+  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION,
+  // Minimum supported ACPI Table Revision
+  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION,
+  // Creator ID
+  TABLE_GENERATOR_CREATOR_ID_ARM,
+  // Creator Revision
+  SLIT_GENERATOR_REVISION,
+  // Build Table function
+  BuildSlitTable,
+  // Free Resource function
+  FreeSlitTableResources,
+  // Extended build function not needed
+  NULL,
+  // Extended build function not implemented by the generator.
+  // Hence extended free resource function is not required.
+  NULL
+};
+
+/** Register the Generator with the ACPI 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
+AcpiSlitLibConstructor (
+  IN  EFI_HANDLE        ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = RegisterAcpiTableGenerator (&SlitGenerator);
+  DEBUG ((DEBUG_INFO, "SLIT: Register Generator. Status = %r\n", Status));
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+
+/** Deregister the Generator from the ACPI 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
+AcpiSlitLibDestructor (
+  IN  EFI_HANDLE        ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = DeregisterAcpiTableGenerator (&SlitGenerator);
+  DEBUG ((DEBUG_INFO, "SLIT: Deregister Generator. Status = %r\n", Status));
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
diff --git a/MdePkg/Include/IndustryStandard/Slit.h b/MdePkg/Include/IndustryStandard/Slit.h
new file mode 100644
index 0000000000..3413b4046b
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Slit.h
@@ -0,0 +1,39 @@
+/** @file
+  ACPI System Locality Information Table (SLIT)
+
+  https://developer.arm.com/documentation/ddi0598/latest
+
+  Copyright (c) 2022 NVIDIA ????????
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __SLIT_H__
+#define __SLIT_H__
+
+#pragma pack(1)
+
+///
+/// System Locality Information Table (SLIT)
+///
+typedef struct {
+  EFI_ACPI_DESCRIPTION_HEADER    Header;
+  UINT64                         NumLocalities;
+} EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE;
+
+///
+/// SLIT Revision (as defined in ACPI 6.4 spec.)
+///
+#define EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION  0x01
+
+///
+/// Relative Distance Info Node Structure
+///
+/*
+typedef struct {
+  UINT8         *Distance;
+} EFI_ACPI_6_4_REL_DISTANCE_INFO_STRUCTURE;
+*/
+#pragma pack()
+
+#endif
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94537): https://edk2.groups.io/g/devel/message/94537
Mute This Topic: https://groups.io/mt/94007104/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hello Swatisri,

On 9/29/22 20:18, Name via groups.io wrote:
> From: Swatisri Kantamsetti <swatisrik@nvidia.com>
> 
> Added SLIT Generator and supporting files to
> generate SLIT ACPI Table
> 
> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> ---
>   DynamicTablesPkg/DynamicTables.dsc.inc        |   2 +
>   DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
>   .../Include/ArmNameSpaceObjects.h             |  12 +
>   .../Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf     |  29 ++
>   .../Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c   | 326 ++++++++++++++++++
>   MdePkg/Include/IndustryStandard/Slit.h        |  39 +++
>   6 files changed, 409 insertions(+)
>   create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
>   create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
>   create mode 100644 MdePkg/Include/IndustryStandard/Slit.h
> 
> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
> index 3d4fa0c4c4..c24fc1d719 100644
> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> @@ -31,6 +31,7 @@
>     DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
>     DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
>     DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
> +  DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
>     DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
>     DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
>   
> @@ -56,6 +57,7 @@
>         NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
>         NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
>         NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
> +      NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
>         NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
>   
>         # AML Fixup
> diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> index f962dbff57..82d0272031 100644
> --- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
> +++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> @@ -93,6 +93,7 @@ typedef enum StdAcpiTableId {
>     EStdAcpiTableIdMcfg,                          ///< MCFG Generator
>     EStdAcpiTableIdIort,                          ///< IORT Generator
>     EStdAcpiTableIdPptt,                          ///< PPTT Generator
> +  EStdAcpiTableIdSlit,                          ///< SLIT Generator
>     EStdAcpiTableIdSrat,                          ///< SRAT Generator
>     EStdAcpiTableIdSsdtSerialPort,                ///< SSDT Serial-Port Generator
>     EStdAcpiTableIdSsdtCmn600,                    ///< SSDT Cmn-600 Generator
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index c66b441d53..aa3f464132 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -65,6 +65,7 @@ typedef enum ArmObjectID {
>     EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
>     EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
>     EArmObjCpcInfo,                      ///< 42 - Continuous Performance Control Info
> +  EArmObjSystemLocalityInfo,           ///< 43 - Relative Distance Info
>     EArmObjMax
>   } EARM_OBJECT_ID;
>   
> @@ -1095,6 +1096,17 @@ typedef struct CmArmRmrDescriptor {
>   */
>   typedef AML_CPC_INFO CM_ARM_CPC_INFO;
>   
> +/** A structure that describes the System Locality Information
> +
> +  This information is used to optimize the system performance

This sentence could be replaced by something more desciptive.

> +
> +  ID: EArmObjSystemLocalityInfo
> +*/
> +typedef struct CmArmSystemLocalityInfo {
> +  UINT64    NumSystemLocalities;
> +  UINT8     *Distance;

The fields need comments I think.

> +} CM_ARM_SYSTEM_LOCALITY_INFO;
> +
>   #pragma pack()
>   
>   #endif // ARM_NAMESPACE_OBJECTS_H_
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
> new file mode 100644
> index 0000000000..ab0ffc9a39
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/AcpiSlitLibArm.inf
> @@ -0,0 +1,29 @@
> +## @file
> +#  SRAT Table Generator
> +#
> +#  Copyright (c) 2022, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION    = 0x0001001B
> +  BASE_NAME      = AcpiSlitLibArm
> +  FILE_GUID      = 28e3f485-0c24-4376-8982-8e1febd791bc
> +  VERSION_STRING = 1.0
> +  MODULE_TYPE    = DXE_DRIVER
> +  LIBRARY_CLASS  = NULL|DXE_DRIVER
> +  CONSTRUCTOR    = AcpiSlitLibConstructor
> +  DESTRUCTOR     = AcpiSlitLibDestructor
> +
> +[Sources]
> +  SlitGenerator.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  DynamicTablesPkg/DynamicTablesPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec

This could be sorted in alphabetical order.

> +
> +[LibraryClasses]
> +  BaseLib
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
> new file mode 100644
> index 0000000000..1a232ce95c
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSlitLibArm/SlitGenerator.c
> @@ -0,0 +1,326 @@
> +/** @file
> +  SLIT Table Generator
> +
> +  Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - ACPI 6.3 Specification, January 2019
> +
> +  @par Glossary:
> +  - Cm or CM   - Configuration Manager
> +  - Obj or OBJ - Object
> +**/
> +
> +

NIT: one newline should be enough.

> +#include <IndustryStandard/Slit.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Protocol/AcpiTable.h>
> +
> +// Module specific include files.
> +#include <AcpiTableGenerator.h>
> +#include <ConfigurationManagerObject.h>
> +#include <ConfigurationManagerHelper.h>
> +#include <Library/TableHelperLib.h>
> +#include <Protocol/ConfigurationManagerProtocol.h>
> +
> +/**
> +  ARM standard SLIT Generator
> +
> +  Requirements:
> +    The following Configuration Manager Object(s) are used by this Generator:
> +    - EArmObjSystemLocalityInfo (Required)
> +*/
> +
> +/**
> +  This macro expands to a function that retrieves the Relative Distance

I think it should be 'System Locality' instead. It seems that there are
other places where 'Relative Distance' is used, maybe there has been a
renaming ?

> +  information from the Configuration Manager.
> +*/
> +GET_OBJECT_LIST (
> +  EObjNameSpaceArm,
> +  EArmObjSystemLocalityInfo,
> +  CM_ARM_SYSTEM_LOCALITY_INFO
> +  );
> +
> +/** Construct the SLIT ACPI table.
> +
> +  Called by the Dynamic Table Manager, this function invokes the
> +  Configuration Manager protocol interface to get the required hardware
> +  information for generating the ACPI table.
> +
> +  If this function allocates any resources then they must be freed
> +  in the FreeXXXXTableResources function.
> +
> +  @param [in]  This           Pointer to the table generator.
> +  @param [in]  AcpiTableInfo  Pointer to the ACPI Table Info.
> +  @param [in]  CfgMgrProtocol Pointer to the Configuration Manager
> +                              Protocol Interface.
> +  @param [out] Table          Pointer to the constructed ACPI Table.
> +
> +  @retval EFI_SUCCESS           Table generated successfully.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_NOT_FOUND         The required object was not found.
> +  @retval EFI_BAD_BUFFER_SIZE   The size returned by the Configuration
> +                                Manager is less than the Object size for the
> +                                requested object.
> +  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +BuildSlitTable (
> +  IN  CONST ACPI_TABLE_GENERATOR                  *CONST  This,
> +  IN  CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
> +  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
> +  OUT       EFI_ACPI_DESCRIPTION_HEADER          **CONST  Table
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT32      TableSize;
> +  UINT32      SlitObjectCount;
> +  UINT32      SlitObjectOffset;
> +  UINT64      NumSysLocalities;
> +  UINT8       *RelDistInfo;
> +
> +  CM_ARM_SYSTEM_LOCALITY_INFO                        *SysLocalityInfo;
> +  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE     *Slit;
> +
> +  ASSERT (
> +    (This != NULL) &&
> +    (AcpiTableInfo != NULL) &&
> +    (CfgMgrProtocol != NULL) &&
> +    (Table != NULL) &&
> +    (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
> +    (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
> +    );

I think it would be better to have multiple ASSERT to identify from
where the error came, as:
   ASSERT (This != NULL);
   ASSERT (AcpiTableInfo != NULL);
   ...

> +
> +  if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
> +      (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision))
> +  {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "ERROR: SLIT: Requested table revision = %d is not supported. "
> +      "Supported table revisions: Minimum = %d. Maximum = %d\n",
> +      AcpiTableInfo->AcpiTableRevision,
> +      This->MinAcpiTableRevision,
> +      This->AcpiTableRevision
> +      ));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *Table = NULL;
> +
> +  Status = GetEArmObjSystemLocalityInfo (
> +             CfgMgrProtocol,
> +             CM_NULL_TOKEN,
> +             &SysLocalityInfo,
> +             &SlitObjectCount
> +             );
> +  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "ERROR: SLIT: Failed to get Relative Distance Info. Status = %r\n",
> +      Status
> +      ));
> +    goto error_handler;
> +  }

I think we don't need to free memory here, so no need to go to the
error_handler, we can just:
   return Status;

> +
> +  // Calculate the size of the SLIT table
> +  TableSize = sizeof (EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE);
> +
> +  // Number of System Localities
> +  NumSysLocalities = SysLocalityInfo->NumSystemLocalities;

I think that if (SlitObjectCount == 0), then SysLocalityInfo is pointing
to random memory. However NumSysLocalities is used here and later in the
function.

> +
> +  if (SlitObjectCount != 0) {
> +    SlitObjectOffset = TableSize;
> +    TableSize   += (sizeof (UINT8) * NumSysLocalities * NumSysLocalities);
> +  }
> +
> +  // Allocate the Buffer for SLIT table
> +  *Table = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize);
> +  if (*Table == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "ERROR: SLIT: Failed to allocate memory for SLIT Table, Size = %d," \
> +      " Status = %r\n",
> +      TableSize,
> +      Status
> +      ));
> +    goto error_handler;
> +  }
> +
> +  Slit = (EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE *)*Table;
> +
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "SLIT: Slit = 0x%p TableSize = 0x%x\n",
> +    Slit,
> +    TableSize
> +    ));
> +
> +  Status = AddAcpiHeader (
> +             CfgMgrProtocol,
> +             This,
> +             &Slit->Header,
> +             AcpiTableInfo,
> +             TableSize
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "ERROR: SLIT: Failed to add ACPI header. Status = %r\n",
> +      Status
> +      ));
> +    goto error_handler;
> +  }
> +
> +  // Number of System Localities
> +  Slit->NumLocalities = NumSysLocalities;
> +
> +  // Offset to place the Relative Distance Info
> +  RelDistInfo = (UINT8*)((UINT8*)Slit + SlitObjectOffset);
> +
> +  // Adding Relative distance in SLIT Table
> +  for (UINT64 Index = 0; Index < Slit->NumLocalities * Slit->NumLocalities; Index++) {

I think Slit->NumLocalitie should not be used in this for loop to avoid dereferencing
the pointer and make the multiplication for each loop.

I think a simpler solution would be to use CopyMem.

> +    *RelDistInfo  = SysLocalityInfo->Distance[Index];
> +    // Next
> +    RelDistInfo++;
> +  }// while
> +
> +  return Status;
> +
> +error_handler:
> +

The newline could be removed.

> +  if (*Table != NULL) {
> +    FreePool (*Table);
> +    *Table = NULL;
> +  }
> +
> +  return Status;
> +}
> +
> +/** Free any resources allocated for constructing the SLIT.
> +
> +  @param [in]      This           Pointer to the table generator.
> +  @param [in]      AcpiTableInfo  Pointer to the ACPI Table Info.
> +  @param [in]      CfgMgrProtocol Pointer to the Configuration Manager
> +                                  Protocol Interface.
> +  @param [in, out] Table          Pointer to the ACPI Table.
> +
> +  @retval EFI_SUCCESS           The resources were freed successfully.
> +  @retval EFI_INVALID_PARAMETER The table pointer is NULL or invalid.
> +**/
> +STATIC
> +EFI_STATUS
> +FreeSlitTableResources (
> +  IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
> +  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
> +  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
> +  IN OUT        EFI_ACPI_DESCRIPTION_HEADER          **CONST  Table
> +  )
> +{
> +  ASSERT (
> +    (This != NULL) &&
> +    (AcpiTableInfo != NULL) &&
> +    (CfgMgrProtocol != NULL) &&
> +    (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
> +    (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
> +    );

I think it would be better to have multiple ASSERT to identify from
where the error came, as:
   ASSERT (This != NULL);
   ASSERT (AcpiTableInfo != NULL);
   ...

> +
> +  if ((Table == NULL) || (*Table == NULL)) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: SLIT: Invalid Table Pointer\n"));
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  FreePool (*Table);
> +  *Table = NULL;
> +  return EFI_SUCCESS;
> +}
> +
> +/** The SLIT Table Generator revision.
> +*/
> +#define SLIT_GENERATOR_REVISION  CREATE_REVISION (1, 0)
> +
> +/** The interface for the SLIT Table Generator.
> +*/
> +STATIC
> +CONST
> +ACPI_TABLE_GENERATOR  SlitGenerator = {
> +  // Generator ID
> +  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSlit),
> +  // Generator Description
> +  L"ACPI.STD.SLIT.GENERATOR",
> +  // ACPI Table Signature
> +  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE,
> +  // ACPI Table Revision supported by this Generator
> +  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION,
> +  // Minimum supported ACPI Table Revision
> +  EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION,
> +  // Creator ID
> +  TABLE_GENERATOR_CREATOR_ID_ARM,
> +  // Creator Revision
> +  SLIT_GENERATOR_REVISION,
> +  // Build Table function
> +  BuildSlitTable,
> +  // Free Resource function
> +  FreeSlitTableResources,
> +  // Extended build function not needed
> +  NULL,
> +  // Extended build function not implemented by the generator.
> +  // Hence extended free resource function is not required.
> +  NULL
> +};
> +
> +/** Register the Generator with the ACPI 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
> +AcpiSlitLibConstructor (
> +  IN  EFI_HANDLE        ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = RegisterAcpiTableGenerator (&SlitGenerator);
> +  DEBUG ((DEBUG_INFO, "SLIT: Register Generator. Status = %r\n", Status));
> +  ASSERT_EFI_ERROR (Status);
> +  return Status;
> +}
> +
> +/** Deregister the Generator from the ACPI 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
> +AcpiSlitLibDestructor (
> +  IN  EFI_HANDLE        ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = DeregisterAcpiTableGenerator (&SlitGenerator);
> +  DEBUG ((DEBUG_INFO, "SLIT: Deregister Generator. Status = %r\n", Status));
> +  ASSERT_EFI_ERROR (Status);
> +  return Status;
> +}
> diff --git a/MdePkg/Include/IndustryStandard/Slit.h b/MdePkg/Include/IndustryStandard/Slit.h
> new file mode 100644
> index 0000000000..3413b4046b
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Slit.h

There is already definitions/macros for the SLIT in:
MdePkg/Include/IndustryStandard/Acpi64.h
Cf: EFI_ACPI_6_4_SYSTEM_LOCALITY_DISTANCE_INFORMATION_TABLE_HEADER

I think this new file can be removed.

> @@ -0,0 +1,39 @@
> +/** @file
> +  ACPI System Locality Information Table (SLIT)
> +
> +  https://developer.arm.com/documentation/ddi0598/latest
> +
> +  Copyright (c) 2022 NVIDIA ????????
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef __SLIT_H__
> +#define __SLIT_H__
> +
> +#pragma pack(1)
> +
> +///
> +/// System Locality Information Table (SLIT)
> +///
> +typedef struct {
> +  EFI_ACPI_DESCRIPTION_HEADER    Header;
> +  UINT64                         NumLocalities;
> +} EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE;
> +
> +///
> +/// SLIT Revision (as defined in ACPI 6.4 spec.)
> +///
> +#define EFI_ACPI_6_4_SYSTEM_LOCALITY_INFORMATION_TABLE_REVISION  0x01
> +
> +///
> +/// Relative Distance Info Node Structure
> +///
> +/*
> +typedef struct {
> +  UINT8         *Distance;
> +} EFI_ACPI_6_4_REL_DISTANCE_INFO_STRUCTURE;
> +*/
> +#pragma pack()
> +
> +#endif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95470): https://edk2.groups.io/g/devel/message/95470
Mute This Topic: https://groups.io/mt/94007104/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-