* [PATCH] DynamicTablesPkg:ACPI: SLIT Generator files
@ 2022-09-29 18:18 Name
2022-10-21 9:02 ` [edk2-devel] " PierreGondois
0 siblings, 1 reply; 2+ messages in thread
From: Name @ 2022-09-29 18:18 UTC (permalink / raw)
To: devel, Sami.Mujawar, Alexei.Fedorov; +Cc: Swatisri Kantamsetti
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [edk2-devel] [PATCH] DynamicTablesPkg:ACPI: SLIT Generator files
2022-09-29 18:18 [PATCH] DynamicTablesPkg:ACPI: SLIT Generator files Name
@ 2022-10-21 9:02 ` PierreGondois
0 siblings, 0 replies; 2+ messages in thread
From: PierreGondois @ 2022-10-21 9:02 UTC (permalink / raw)
To: devel, username, Sami.Mujawar, Alexei.Fedorov; +Cc: Swatisri Kantamsetti
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-21 9:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-29 18:18 [PATCH] DynamicTablesPkg:ACPI: SLIT Generator files Name
2022-10-21 9:02 ` [edk2-devel] " PierreGondois
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox